Duplicate definition of type for rustc::mir::Constant #56137

Activity

1. oli-obk commented on Nov 21, 2018

   

   oli-obk

   on Nov 21, 2018

   Contributor

   More actions

   These types aren't always equal. e.g. the lifetimes often differ, and I think around impl trait some inconsistencies can happen, too. we should probably document this much better and/or figure out whether we need the duplication
2. oli-obk

   added

   A-MIRArea: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.htmlArea: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html

   on Nov 21, 2018
3. eddyb

   self-assigned this

   on Aug 12, 2019
4. eddyb commented on Aug 12, 2019

   

   eddyb

   on Aug 12, 2019

   Last edited by eddyb

   Contributor

   More actions

   Looks like #61041 is blocked on fixing this.

   EDIT: to expand on that, this example:

   trait Zero {
       type Repr;
       const ZERO: Self::Repr;
   }
   
   impl Zero for char {
       type Repr = u32;
       const ZERO: u32 = 0;
   }
   
   const ONE: u32 = char::ZERO + 1;

   Has this for the char::ZERO in the MIR of ONE:

   // mir::Constant
   // + span: src/lib.rs:11:18: 11:28
   // + ty: u32
   // + literal: Const { ty: <Self as Zero>::Repr, val: Unevaluated(DefId(0:14 ~ playground[5f8c]::Zero[0]::ZERO[0]), [char]) }

   Note the <Self as Zero>::Repr in literal.ty, which is clearly wrong for ONE (appears to be unsubstituted), while ty is correct.
5. RalfJung commented on Aug 12, 2019

   

   RalfJung

   on Aug 12, 2019 · Hidden as off-topic

   show commentMore actions
6. RalfJung commented on Aug 12, 2019

   

   RalfJung

   on Aug 12, 2019 · Hidden as off-topic

   show commentMore actions
7. eddyb commented on Aug 12, 2019

   

   eddyb

   on Aug 12, 2019

   Contributor

   More actions

   @RalfJung All of that is fine. ty::Const is "unusual" because it's one of the 3 "entities" (Ty and ty::Region have the same treatment).

   I'm already fixing this. MIR construction is just wrong and likely has been since forever.
   MIRI kept papering over this issue by not using the ty field of ty::Const.
8. eddyb commented on Aug 12, 2019

   

   eddyb

   on Aug 12, 2019

   Contributor

   More actions

   Huh, looks like the bug is pretty recent: 5cd2806#diff-c599d3d6879ad974960a24abd4e4bdb6R953
   It was introduced in #59178, after this issue - had we removed the duplication earlier, the bug would've been prevented, heh.
9. Centril

   added 5 commits that reference this issue on Aug 14, 2019

   Rollup merge of rust-lang#63495 - eddyb:mir-constant-ty, r=oli-obk

   ...

   This issue will close once commit baab604 is merged into the 'master' branch.

   baab604

   Rollup merge of rust-lang#63495 - eddyb:mir-constant-ty, r=oli-obk

   ...

   This issue will close once commit 47490ff is merged into the 'master' branch.

   47490ff

   Rollup merge of rust-lang#63495 - eddyb:mir-constant-ty, r=oli-obk

   ...

   This issue will close once commit 6a11bf7 is merged into the 'master' branch.

   6a11bf7

   Rollup merge of rust-lang#63495 - eddyb:mir-constant-ty, r=oli-obk

   ...

   This issue will close once commit ddeaf59 is merged into the 'master' branch.

   ddeaf59

   Rollup merge of rust-lang#63495 - eddyb:mir-constant-ty, r=oli-obk

   ...

   This issue will close once commit 7ff5b38 is merged into the 'master' branch.

   7ff5b38
10. Centril

    added a commit that references this issue on Aug 16, 2019

    Rollup merge of rust-lang#63495 - eddyb:mir-constant-ty, r=oli-obk

    ...

    This issue will close once commit db3bae0 is merged into the 'master' branch.

    db3bae0
11. bors

    closed this as completedin db3bae0on Aug 16, 2019