Auto merge of #66677 - wesleywiser:fix_const_prop_alloc_id_ice, r=oli-obk

[const prop] Fix "alloc id without corresponding allocation" ICE

r? @oli-obk

10 files changed, 78 insertions, 36 deletions

diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs
index 52c72de7579..f82af62c5f3 100644
--- a/src/librustc/mir/interpret/value.rs
+++ b/src/librustc/mir/interpret/value.rs
@@ -458,7 +458,7 @@ impl<Tag> From<Pointer<Tag>> for Scalar<Tag> {
     }
 }
 
-#[derive(Clone, Copy, Eq, PartialEq, RustcEncodable, RustcDecodable, HashStable)]
+#[derive(Clone, Copy, Eq, PartialEq, RustcEncodable, RustcDecodable, HashStable, Hash)]
 pub enum ScalarMaybeUndef<Tag = (), Id = AllocId> {
     Scalar(Scalar<Tag, Id>),
     Undef,
diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs
index 68bb0a3e435..630f3f60344 100644
--- a/src/librustc_mir/interpret/intern.rs
+++ b/src/librustc_mir/interpret/intern.rs
@@ -3,22 +3,34 @@
 //! After a const evaluation has computed a value, before we destroy the const evaluator's session
 //! memory, we need to extract all memory allocations to the global memory pool so they stay around.
 
-use rustc::ty::{Ty, self};
-use rustc::mir::interpret::{InterpResult, ErrorHandled};
-use rustc::hir;
 use super::validity::RefTracking;
-use rustc_data_structures::fx::FxHashSet;
+use rustc::hir;
+use rustc::mir::interpret::{ErrorHandled, InterpResult};
+use rustc::ty::{self, Ty};
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 
 use syntax::ast::Mutability;
 
 use super::{
-    ValueVisitor, MemoryKind, AllocId, MPlaceTy, Scalar,
+    AllocId, Allocation, InterpCx, Machine, MemoryKind, MPlaceTy, Scalar, ValueVisitor,
 };
-use crate::const_eval::{CompileTimeInterpreter, CompileTimeEvalContext};
 
-struct InternVisitor<'rt, 'mir, 'tcx> {
+pub trait CompileTimeMachine<'mir, 'tcx> =
+    Machine<
+        'mir,
+        'tcx,
+        MemoryKinds = !,
+        PointerTag = (),
+        ExtraFnVal = !,
+        FrameExtra = (),
+        MemoryExtra = (),
+        AllocExtra = (),
+        MemoryMap = FxHashMap<AllocId, (MemoryKind<!>, Allocation)>,
+    >;
+
+struct InternVisitor<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> {
     /// The ectx from which we intern.
-    ecx: &'rt mut CompileTimeEvalContext<'mir, 'tcx>,
+    ecx: &'rt mut InterpCx<'mir, 'tcx, M>,
     /// Previously encountered safe references.
     ref_tracking: &'rt mut RefTracking<(MPlaceTy<'tcx>, Mutability, InternMode)>,
     /// A list of all encountered allocations. After type-based interning, we traverse this list to
@@ -58,18 +70,15 @@ struct IsStaticOrFn;
 /// `immutable` things might become mutable if `ty` is not frozen.
 /// `ty` can be `None` if there is no potential interior mutability
 /// to account for (e.g. for vtables).
-fn intern_shallow<'rt, 'mir, 'tcx>(
-    ecx: &'rt mut CompileTimeEvalContext<'mir, 'tcx>,
+fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>(
+    ecx: &'rt mut InterpCx<'mir, 'tcx, M>,
     leftover_allocations: &'rt mut FxHashSet<AllocId>,
     mode: InternMode,
     alloc_id: AllocId,
     mutability: Mutability,
     ty: Option<Ty<'tcx>>,
 ) -> InterpResult<'tcx, Option<IsStaticOrFn>> {
-    trace!(
-        "InternVisitor::intern {:?} with {:?}",
-        alloc_id, mutability,
-    );
+    trace!("InternVisitor::intern {:?} with {:?}", alloc_id, mutability,);
     // remove allocation
     let tcx = ecx.tcx;
     let (kind, mut alloc) = match ecx.memory.alloc_map.remove(&alloc_id) {
@@ -130,7 +139,7 @@ fn intern_shallow<'rt, 'mir, 'tcx>(
     Ok(None)
 }
 
-impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> {
+impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> InternVisitor<'rt, 'mir, 'tcx, M> {
     fn intern_shallow(
         &mut self,
         alloc_id: AllocId,
@@ -148,15 +157,15 @@ impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> {
     }
 }
 
-impl<'rt, 'mir, 'tcx>
-    ValueVisitor<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>
+impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>
+    ValueVisitor<'mir, 'tcx, M>
 for
-    InternVisitor<'rt, 'mir, 'tcx>
+    InternVisitor<'rt, 'mir, 'tcx, M>
 {
     type V = MPlaceTy<'tcx>;
 
     #[inline(always)]
-    fn ecx(&self) -> &CompileTimeEvalContext<'mir, 'tcx> {
+    fn ecx(&self) -> &InterpCx<'mir, 'tcx, M> {
         &self.ecx
     }
 
@@ -265,8 +274,8 @@ for
     }
 }
 
-pub fn intern_const_alloc_recursive(
-    ecx: &mut CompileTimeEvalContext<'mir, 'tcx>,
+pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
+    ecx: &mut InterpCx<'mir, 'tcx, M>,
     // The `mutability` of the place, ignoring the type.
     place_mut: Option<hir::Mutability>,
     ret: MPlaceTy<'tcx>,
diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index 4f03b132090..8dd5807f7cf 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -27,7 +27,7 @@ use rustc_macros::HashStable;
 /// operations and fat pointers. This idea was taken from rustc's codegen.
 /// In particular, thanks to `ScalarPair`, arithmetic operations and casts can be entirely
 /// defined on `Immediate`, and do not have to work with a `Place`.
-#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable, Hash)]
 pub enum Immediate<Tag=(), Id=AllocId> {
     Scalar(ScalarMaybeUndef<Tag, Id>),
     ScalarPair(ScalarMaybeUndef<Tag, Id>, ScalarMaybeUndef<Tag, Id>),
@@ -104,7 +104,7 @@ impl<'tcx, Tag> ::std::ops::Deref for ImmTy<'tcx, Tag> {
 /// An `Operand` is the result of computing a `mir::Operand`. It can be immediate,
 /// or still in memory. The latter is an optimization, to delay reading that chunk of
 /// memory and to avoid having to store arbitrary-sized data here.
-#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable, Hash)]
 pub enum Operand<Tag=(), Id=AllocId> {
     Immediate(Immediate<Tag, Id>),
     Indirect(MemPlace<Tag, Id>),
@@ -134,7 +134,7 @@ impl<Tag> Operand<Tag> {
     }
 }
 
-#[derive(Copy, Clone, Debug, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
 pub struct OpTy<'tcx, Tag=()> {
     op: Operand<Tag>, // Keep this private; it helps enforce invariants.
     pub layout: TyLayout<'tcx>,
diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs
index f2707969517..cca76700ed1 100644
--- a/src/librustc_mir/lib.rs
+++ b/src/librustc_mir/lib.rs
@@ -28,6 +28,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment!
 #![feature(range_is_empty)]
 #![feature(stmt_expr_attributes)]
 #![feature(bool_to_option)]
+#![feature(trait_alias)]
 
 #![recursion_limit="256"]
 
diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs
index 1114694d2e3..6f0b960cab1 100644
--- a/src/librustc_mir/transform/const_prop.rs
+++ b/src/librustc_mir/transform/const_prop.rs
@@ -30,7 +30,7 @@ use crate::interpret::{
     self, InterpCx, ScalarMaybeUndef, Immediate, OpTy,
     StackPopCleanup, LocalValue, LocalState, AllocId, Frame,
     Allocation, MemoryKind, ImmTy, Pointer, Memory, PlaceTy,
-    Operand as InterpOperand,
+    Operand as InterpOperand, intern_const_alloc_recursive,
 };
 use crate::const_eval::error_to_const_error;
 use crate::transform::{MirPass, MirSource};
@@ -647,9 +647,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
     }
 
     fn should_const_prop(&mut self, op: OpTy<'tcx>) -> bool {
-        if self.tcx.sess.opts.debugging_opts.mir_opt_level >= 2 {
-            return true;
-        } else if self.tcx.sess.opts.debugging_opts.mir_opt_level == 0 {
+        let mir_opt_level = self.tcx.sess.opts.debugging_opts.mir_opt_level;
+
+        if mir_opt_level == 0 {
             return false;
         }
 
@@ -659,6 +659,14 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
             interpret::Operand::Immediate(Immediate::ScalarPair(ScalarMaybeUndef::Scalar(l),
                                                                 ScalarMaybeUndef::Scalar(r))) =>
                 l.is_bits() && r.is_bits(),
+            interpret::Operand::Indirect(_) if mir_opt_level >= 2 => {
+                intern_const_alloc_recursive(
+                    &mut self.ecx,
+                    None,
+                    op.assert_mem_place()
+                ).expect("failed to intern alloc");
+                true
+            },
             _ => false
         }
     }
diff --git a/src/test/mir-opt/const_prop/const_prop_fails_gracefully.rs b/src/test/mir-opt/const_prop/const_prop_fails_gracefully.rs
index 34d4a534ad9..3f82b81a47d 100644
--- a/src/test/mir-opt/const_prop/const_prop_fails_gracefully.rs
+++ b/src/test/mir-opt/const_prop/const_prop_fails_gracefully.rs
@@ -23,9 +23,9 @@ fn main() {
 // START rustc.main.ConstProp.after.mir
 //  bb0: {
 //      ...
-//      _4 = const Scalar(AllocId(1).0x0) : &i32;
-//      _3 = const Scalar(AllocId(1).0x0) : &i32;
-//      _2 = const Value(Scalar(AllocId(1).0x0)) : *const i32;
+//      _4 = const main::FOO;
+//      _3 = _4;
+//      _2 = move _3 as *const i32 (Misc);
 //      ...
 //      _1 = move _2 as usize (Misc);
 //      ...
diff --git a/src/test/mir-opt/const_prop/ref_deref.rs b/src/test/mir-opt/const_prop/ref_deref.rs
index 2d04822c0e7..d45ffdc8775 100644
--- a/src/test/mir-opt/const_prop/ref_deref.rs
+++ b/src/test/mir-opt/const_prop/ref_deref.rs
@@ -14,7 +14,7 @@ fn main() {
 // START rustc.main.ConstProp.after.mir
 // bb0: {
 //     ...
-//     _2 = const Scalar(AllocId(0).0x0) : &i32;
+//     _2 = &(promoted[0]: i32);
 //     _1 = const 4i32;
 //     ...
 // }
diff --git a/src/test/mir-opt/const_prop/reify_fn_ptr.rs b/src/test/mir-opt/const_prop/reify_fn_ptr.rs
index ad7f195676a..4d6fe905b0c 100644
--- a/src/test/mir-opt/const_prop/reify_fn_ptr.rs
+++ b/src/test/mir-opt/const_prop/reify_fn_ptr.rs
@@ -16,7 +16,7 @@ fn main() {
 // START rustc.main.ConstProp.after.mir
 //  bb0: {
 //      ...
-//      _3 = const main;
+//      _3 = const main as fn() (Pointer(ReifyFnPointer));
 //      _2 = move _3 as usize (Misc);
 //      ...
 //      _1 = move _2 as *const fn() (Misc);
diff --git a/src/test/mir-opt/const_prop/slice_len.rs b/src/test/mir-opt/const_prop/slice_len.rs
index 05595ce147c..d6ff76b34b9 100644
--- a/src/test/mir-opt/const_prop/slice_len.rs
+++ b/src/test/mir-opt/const_prop/slice_len.rs
@@ -24,8 +24,8 @@ fn main() {
 // START rustc.main.ConstProp.after.mir
 //  bb0: {
 //      ...
-//      _4 = const Scalar(AllocId(0).0x0) : &[u32; 3];
-//      _3 = const Scalar(AllocId(0).0x0) : &[u32; 3];
+//      _4 = &(promoted[0]: [u32; 3]);
+//      _3 = _4;
 //      _2 = move _3 as &[u32] (Pointer(Unsize));
 //      ...
 //      _6 = const 1usize;
diff --git a/src/test/ui/consts/issue-66345.rs b/src/test/ui/consts/issue-66345.rs
new file mode 100644
index 00000000000..7d0de73007c
--- /dev/null
+++ b/src/test/ui/consts/issue-66345.rs
@@ -0,0 +1,24 @@
+// run-pass
+// compile-flags: -Z mir-opt-level=3
+
+// Checks that the compiler does not ICE when passing references to field of by-value struct
+// with -Z mir-opt-level=3
+
+fn do_nothing(_: &()) {}
+
+pub struct Foo {
+    bar: (),
+}
+
+pub fn by_value_1(foo: Foo) {
+    do_nothing(&foo.bar);
+}
+
+pub fn by_value_2<T>(foo: Foo) {
+    do_nothing(&foo.bar);
+}
+
+fn main() {
+    by_value_1(Foo { bar: () });
+    by_value_2::<()>(Foo { bar: () });
+}