From 7cfb05fd23b5ecc6a13f4629844ff81ac758497c Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Wed, 30 Jan 2019 14:16:18 +0100
Subject: [PATCH 01/11] Merge `locals` and `local_layouts` fields

---
 src/librustc_mir/interpret/eval_context.rs | 70 ++++++++++++----------
 src/librustc_mir/interpret/mod.rs          |  2 +-
 src/librustc_mir/interpret/snapshot.rs     | 35 ++++++++---
 3 files changed, 68 insertions(+), 39 deletions(-)

diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index 34443bb353e0e..d2cabde986345 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -76,8 +76,7 @@ pub struct Frame<'mir, 'tcx: 'mir, Tag=(), Extra=()> {
     /// The locals are stored as `Option<Value>`s.
     /// `None` represents a local that is currently dead, while a live local
     /// can either directly contain `Scalar` or refer to some part of an `Allocation`.
-    pub locals: IndexVec<mir::Local, LocalValue<Tag>>,
-    pub local_layouts: IndexVec<mir::Local, Cell<Option<TyLayout<'tcx>>>>,
+    pub locals: IndexVec<mir::Local, LocalValue<'tcx, Tag>>,
 
     ////////////////////////////////////////////////////////////////////////////////
     // Current position within the function
@@ -106,9 +105,17 @@ pub enum StackPopCleanup {
     None { cleanup: bool },
 }
 
-// State of a local variable
+/// State of a local variable including a memoized layout
+#[derive(Clone, PartialEq, Eq)]
+pub struct LocalValue<'tcx, Tag=(), Id=AllocId> {
+    pub state: LocalState<Tag, Id>,
+    /// Don't modify if `Some`, this is only used to prevent computing the layout twice
+    pub layout: Cell<Option<TyLayout<'tcx>>>,
+}
+
+/// State of a local variable
 #[derive(Copy, Clone, PartialEq, Eq, Hash)]
-pub enum LocalValue<Tag=(), Id=AllocId> {
+pub enum LocalState<Tag=(), Id=AllocId> {
     Dead,
     // Mostly for convenience, we re-use the `Operand` type here.
     // This is an optimization over just always having a pointer here;
@@ -117,18 +124,18 @@ pub enum LocalValue<Tag=(), Id=AllocId> {
     Live(Operand<Tag, Id>),
 }
 
-impl<'tcx, Tag> LocalValue<Tag> {
+impl<'tcx, Tag> LocalValue<'tcx, Tag> {
     pub fn access(&self) -> EvalResult<'tcx, &Operand<Tag>> {
-        match self {
-            LocalValue::Dead => err!(DeadLocal),
-            LocalValue::Live(ref val) => Ok(val),
+        match self.state {
+            LocalState::Dead => err!(DeadLocal),
+            LocalState::Live(ref val) => Ok(val),
         }
     }
 
     pub fn access_mut(&mut self) -> EvalResult<'tcx, &mut Operand<Tag>> {
-        match self {
-            LocalValue::Dead => err!(DeadLocal),
-            LocalValue::Live(ref mut val) => Ok(val),
+        match self.state {
+            LocalState::Dead => err!(DeadLocal),
+            LocalState::Live(ref mut val) => Ok(val),
         }
     }
 }
@@ -312,7 +319,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
         frame: &Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>,
         local: mir::Local
     ) -> EvalResult<'tcx, TyLayout<'tcx>> {
-        let cell = &frame.local_layouts[local];
+        let cell = &frame.locals[local].layout;
         if cell.get().is_none() {
             let local_ty = frame.mir.local_decls[local].ty;
             let local_ty = self.monomorphize_with_substs(local_ty, frame.instance.substs);
@@ -454,7 +461,6 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
             // empty local array, we fill it in below, after we are inside the stack frame and
             // all methods actually know about the frame
             locals: IndexVec::new(),
-            local_layouts: IndexVec::from_elem_n(Default::default(), mir.local_decls.len()),
             span,
             instance,
             stmt: 0,
@@ -464,14 +470,18 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
         // don't allocate at all for trivial constants
         if mir.local_decls.len() > 1 {
             // We put some marker immediate into the locals that we later want to initialize.
-            // This can be anything except for LocalValue::Dead -- because *that* is the
+            // This can be anything except for LocalState::Dead -- because *that* is the
             // value we use for things that we know are initially dead.
-            let dummy =
-                LocalValue::Live(Operand::Immediate(Immediate::Scalar(ScalarMaybeUndef::Undef)));
+            let dummy = LocalValue {
+                state: LocalState::Live(Operand::Immediate(Immediate::Scalar(
+                    ScalarMaybeUndef::Undef,
+                ))),
+                layout: Cell::new(None),
+            };
             let mut locals = IndexVec::from_elem(dummy, &mir.local_decls);
             // Return place is handled specially by the `eval_place` functions, and the
             // entry in `locals` should never be used. Make it dead, to be sure.
-            locals[mir::RETURN_PLACE] = LocalValue::Dead;
+            locals[mir::RETURN_PLACE].state = LocalState::Dead;
             // Now mark those locals as dead that we do not want to initialize
             match self.tcx.describe_def(instance.def_id()) {
                 // statics and constants don't have `Storage*` statements, no need to look for them
@@ -484,7 +494,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
                             match stmt.kind {
                                 StorageLive(local) |
                                 StorageDead(local) => {
-                                    locals[local] = LocalValue::Dead;
+                                    locals[local].state = LocalState::Dead;
                                 }
                                 _ => {}
                             }
@@ -494,13 +504,13 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
             }
             // Finally, properly initialize all those that still have the dummy value
             for (idx, local) in locals.iter_enumerated_mut() {
-                match *local {
-                    LocalValue::Live(_) => {
+                match local.state {
+                    LocalState::Live(_) => {
                         // This needs to be peoperly initialized.
                         let layout = self.layout_of_local(self.frame(), idx)?;
-                        *local = LocalValue::Live(self.uninit_operand(layout)?);
+                        local.state = LocalState::Live(self.uninit_operand(layout)?);
                     }
-                    LocalValue::Dead => {
+                    LocalState::Dead => {
                         // Nothing to do
                     }
                 }
@@ -543,7 +553,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
         }
         // Deallocate all locals that are backed by an allocation.
         for local in frame.locals {
-            self.deallocate_local(local)?;
+            self.deallocate_local(local.state)?;
         }
         // Validate the return value. Do this after deallocating so that we catch dangling
         // references.
@@ -587,31 +597,31 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
     pub fn storage_live(
         &mut self,
         local: mir::Local
-    ) -> EvalResult<'tcx, LocalValue<M::PointerTag>> {
+    ) -> EvalResult<'tcx, LocalState<M::PointerTag>> {
         assert!(local != mir::RETURN_PLACE, "Cannot make return place live");
         trace!("{:?} is now live", local);
 
         let layout = self.layout_of_local(self.frame(), local)?;
-        let init = LocalValue::Live(self.uninit_operand(layout)?);
+        let init = LocalState::Live(self.uninit_operand(layout)?);
         // StorageLive *always* kills the value that's currently stored
-        Ok(mem::replace(&mut self.frame_mut().locals[local], init))
+        Ok(mem::replace(&mut self.frame_mut().locals[local].state, init))
     }
 
     /// Returns the old value of the local.
     /// Remember to deallocate that!
-    pub fn storage_dead(&mut self, local: mir::Local) -> LocalValue<M::PointerTag> {
+    pub fn storage_dead(&mut self, local: mir::Local) -> LocalState<M::PointerTag> {
         assert!(local != mir::RETURN_PLACE, "Cannot make return place dead");
         trace!("{:?} is now dead", local);
 
-        mem::replace(&mut self.frame_mut().locals[local], LocalValue::Dead)
+        mem::replace(&mut self.frame_mut().locals[local].state, LocalState::Dead)
     }
 
     pub(super) fn deallocate_local(
         &mut self,
-        local: LocalValue<M::PointerTag>,
+        local: LocalState<M::PointerTag>,
     ) -> EvalResult<'tcx> {
         // FIXME: should we tell the user that there was a local which was never written to?
-        if let LocalValue::Live(Operand::Indirect(MemPlace { ptr, .. })) = local {
+        if let LocalState::Live(Operand::Indirect(MemPlace { ptr, .. })) = local {
             trace!("deallocating local");
             let ptr = ptr.to_ptr()?;
             self.memory.dump_alloc(ptr.alloc_id);
diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs
index e3ab90a602040..6ee7d3309f464 100644
--- a/src/librustc_mir/interpret/mod.rs
+++ b/src/librustc_mir/interpret/mod.rs
@@ -18,7 +18,7 @@ mod visitor;
 pub use rustc::mir::interpret::*; // have all the `interpret` symbols in one place: here
 
 pub use self::eval_context::{
-    EvalContext, Frame, StackPopCleanup, LocalValue,
+    EvalContext, Frame, StackPopCleanup, LocalValue, LocalState,
 };
 
 pub use self::place::{Place, PlaceTy, MemPlace, MPlaceTy};
diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs
index 53105266b3928..0b5dc9446921a 100644
--- a/src/librustc_mir/interpret/snapshot.rs
+++ b/src/librustc_mir/interpret/snapshot.rs
@@ -7,7 +7,7 @@
 
 use std::hash::{Hash, Hasher};
 
-use rustc::ich::StableHashingContextProvider;
+use rustc::ich::{StableHashingContextProvider, StableHashingContext};
 use rustc::mir;
 use rustc::mir::interpret::{
     AllocId, Pointer, Scalar,
@@ -19,12 +19,12 @@ use rustc::ty::{self, TyCtxt};
 use rustc::ty::layout::Align;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::indexed_vec::IndexVec;
-use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
+use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableHasherResult};
 use syntax::ast::Mutability;
 use syntax::source_map::Span;
 
 use super::eval_context::{LocalValue, StackPopCleanup};
-use super::{Frame, Memory, Operand, MemPlace, Place, Immediate, ScalarMaybeUndef};
+use super::{Frame, Memory, Operand, MemPlace, Place, Immediate, ScalarMaybeUndef, LocalState};
 use const_eval::CompileTimeInterpreter;
 
 #[derive(Default)]
@@ -250,11 +250,11 @@ impl_snapshot_for!(enum Operand {
     Indirect(m),
 });
 
-impl_stable_hash_for!(enum ::interpret::LocalValue {
+impl_stable_hash_for!(enum ::interpret::LocalState {
     Dead,
     Live(x),
 });
-impl_snapshot_for!(enum LocalValue {
+impl_snapshot_for!(enum LocalState {
     Live(v),
     Dead,
 });
@@ -309,7 +309,7 @@ struct FrameSnapshot<'a, 'tcx: 'a> {
     span: &'a Span,
     return_to_block: &'a StackPopCleanup,
     return_place: Option<Place<(), AllocIdSnapshot<'a>>>,
-    locals: IndexVec<mir::Local, LocalValue<(), AllocIdSnapshot<'a>>>,
+    locals: IndexVec<mir::Local, LocalState<(), AllocIdSnapshot<'a>>>,
     block: &'a mir::BasicBlock,
     stmt: usize,
 }
@@ -321,7 +321,6 @@ impl_stable_hash_for!(impl<'mir, 'tcx: 'mir> for struct Frame<'mir, 'tcx> {
     return_to_block,
     return_place -> (return_place.as_ref().map(|r| &**r)),
     locals,
-    local_layouts -> _,
     block,
     stmt,
     extra,
@@ -340,7 +339,6 @@ impl<'a, 'mir, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a Frame<'mir, 'tcx>
             return_to_block,
             return_place,
             locals,
-            local_layouts: _,
             block,
             stmt,
             extra: _,
@@ -358,6 +356,27 @@ impl<'a, 'mir, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a Frame<'mir, 'tcx>
     }
 }
 
+impl<'a, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a LocalValue<'tcx>
+    where Ctx: SnapshotContext<'a>,
+{
+    type Item = LocalState<(), AllocIdSnapshot<'a>>;
+
+    fn snapshot(&self, ctx: &'a Ctx) -> Self::Item {
+        self.state.snapshot(ctx)
+    }
+}
+
+
+impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for LocalValue<'gcx> {
+    fn hash_stable<W: StableHasherResult>(
+        &self,
+        hcx: &mut StableHashingContext<'a>,
+        hasher: &mut StableHasher<W>,
+    ) {
+        self.state.hash_stable(hcx, hasher);
+    }
+}
+
 impl<'a, 'b, 'mir, 'tcx: 'a+'mir> SnapshotContext<'b>
     for Memory<'a, 'mir, 'tcx, CompileTimeInterpreter<'a, 'mir, 'tcx>>
 {

From bc528d93ec27758af5e61c21c3e05d8da077a9ce Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Wed, 30 Jan 2019 14:55:31 +0100
Subject: [PATCH 02/11] Allow `layout_of_local` to also use cached layouts

---
 src/librustc_mir/interpret/eval_context.rs | 11 +++++++----
 src/librustc_mir/interpret/operand.rs      | 12 +++++++-----
 src/librustc_mir/interpret/place.rs        |  6 +++---
 src/librustc_mir/interpret/terminator.rs   |  4 ++--
 4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index d2cabde986345..d890e2fbe46da 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -317,13 +317,16 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
     pub fn layout_of_local(
         &self,
         frame: &Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>,
-        local: mir::Local
+        local: mir::Local,
+        layout: Option<TyLayout<'tcx>>,
     ) -> EvalResult<'tcx, TyLayout<'tcx>> {
         let cell = &frame.locals[local].layout;
         if cell.get().is_none() {
+            let layout = ::interpret::operand::from_known_layout(layout, || {
             let local_ty = frame.mir.local_decls[local].ty;
             let local_ty = self.monomorphize_with_substs(local_ty, frame.instance.substs);
-            let layout = self.layout_of(local_ty)?;
+                self.layout_of(local_ty)
+            })?;
             cell.set(Some(layout));
         }
 
@@ -507,7 +510,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
                 match local.state {
                     LocalState::Live(_) => {
                         // This needs to be peoperly initialized.
-                        let layout = self.layout_of_local(self.frame(), idx)?;
+                        let layout = self.layout_of_local(self.frame(), idx, None)?;
                         local.state = LocalState::Live(self.uninit_operand(layout)?);
                     }
                     LocalState::Dead => {
@@ -601,7 +604,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
         assert!(local != mir::RETURN_PLACE, "Cannot make return place live");
         trace!("{:?} is now live", local);
 
-        let layout = self.layout_of_local(self.frame(), local)?;
+        let layout = self.layout_of_local(self.frame(), local, None)?;
         let init = LocalState::Live(self.uninit_operand(layout)?);
         // StorageLive *always* kills the value that's currently stored
         Ok(mem::replace(&mut self.frame_mut().locals[local].state, init))
diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index e4bee24c88cfa..a7e92d5679409 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -227,7 +227,7 @@ impl<'tcx, Tag> OpTy<'tcx, Tag>
 // Use the existing layout if given (but sanity check in debug mode),
 // or compute the layout.
 #[inline(always)]
-fn from_known_layout<'tcx>(
+pub(super) fn from_known_layout<'tcx>(
     layout: Option<TyLayout<'tcx>>,
     compute: impl FnOnce() -> EvalResult<'tcx, TyLayout<'tcx>>
 ) -> EvalResult<'tcx, TyLayout<'tcx>> {
@@ -461,10 +461,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         &self,
         frame: &super::Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>,
         local: mir::Local,
+        layout: Option<TyLayout<'tcx>>,
     ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> {
         assert_ne!(local, mir::RETURN_PLACE);
         let op = *frame.locals[local].access()?;
-        let layout = self.layout_of_local(frame, local)?;
+        let layout = self.layout_of_local(frame, local, layout)?;
         Ok(OpTy { op, layout })
     }
 
@@ -473,14 +474,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
     fn eval_place_to_op(
         &self,
         mir_place: &mir::Place<'tcx>,
+        layout: Option<TyLayout<'tcx>>,
     ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> {
         use rustc::mir::Place::*;
         let op = match *mir_place {
             Local(mir::RETURN_PLACE) => return err!(ReadFromReturnPointer),
-            Local(local) => self.access_local(self.frame(), local)?,
+            Local(local) => self.access_local(self.frame(), local, layout)?,
 
             Projection(ref proj) => {
-                let op = self.eval_place_to_op(&proj.base)?;
+                let op = self.eval_place_to_op(&proj.base, None)?;
                 self.operand_projection(op, &proj.elem)?
             }
 
@@ -504,7 +506,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
             // FIXME: do some more logic on `move` to invalidate the old location
             Copy(ref place) |
             Move(ref place) =>
-                self.eval_place_to_op(place)?,
+                self.eval_place_to_op(place, layout)?,
 
             Constant(ref constant) => {
                 let layout = from_known_layout(layout, || {
diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs
index f3a948a6ca3e7..ffb8ec899a07d 100644
--- a/src/librustc_mir/interpret/place.rs
+++ b/src/librustc_mir/interpret/place.rs
@@ -624,7 +624,7 @@ where
                     // their layout on return.
                     PlaceTy {
                         place: *return_place,
-                        layout: self.layout_of_local(self.frame(), mir::RETURN_PLACE)?,
+                        layout: self.layout_of_local(self.frame(), mir::RETURN_PLACE, None)?,
                     },
                 None => return err!(InvalidNullPointerUsage),
             },
@@ -633,7 +633,7 @@ where
                     frame: self.cur_frame(),
                     local,
                 },
-                layout: self.layout_of_local(self.frame(), local)?,
+                layout: self.layout_of_local(self.frame(), local, None)?,
             },
 
             Projection(ref proj) => {
@@ -901,7 +901,7 @@ where
                         // We need the layout of the local.  We can NOT use the layout we got,
                         // that might e.g., be an inner field of a struct with `Scalar` layout,
                         // that has different alignment than the outer field.
-                        let local_layout = self.layout_of_local(&self.stack[frame], local)?;
+                        let local_layout = self.layout_of_local(&self.stack[frame], local, None)?;
                         let ptr = self.allocate(local_layout, MemoryKind::Stack);
                         // We don't have to validate as we can assume the local
                         // was already valid for its type.
diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs
index 951e9fabe5932..7e823524c180c 100644
--- a/src/librustc_mir/interpret/terminator.rs
+++ b/src/librustc_mir/interpret/terminator.rs
@@ -309,7 +309,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                         mir.spread_arg,
                         mir.args_iter()
                             .map(|local|
-                                (local, self.layout_of_local(self.frame(), local).unwrap().ty)
+                                (local, self.layout_of_local(self.frame(), local, None).unwrap().ty)
                             )
                             .collect::<Vec<_>>()
                     );
@@ -383,7 +383,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                         }
                     } else {
                         let callee_layout =
-                            self.layout_of_local(self.frame(), mir::RETURN_PLACE)?;
+                            self.layout_of_local(self.frame(), mir::RETURN_PLACE, None)?;
                         if !callee_layout.abi.is_uninhabited() {
                             return err!(FunctionRetMismatch(
                                 self.tcx.types.never, callee_layout.ty

From 154c54c875fe8892cacc2d8bc0275ecc27b4baaf Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Wed, 30 Jan 2019 15:01:42 +0100
Subject: [PATCH 03/11] Make priroda happy again

---
 src/librustc_mir/interpret/operand.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index a7e92d5679409..37e421c2e7339 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -457,7 +457,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
     }
 
     /// This is used by [priroda](https://github.com/oli-obk/priroda) to get an OpTy from a local
-    fn access_local(
+    pub fn access_local(
         &self,
         frame: &super::Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>,
         local: mir::Local,

From 4165c890ed573cc801a27483935ccdbc01fcd75b Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Wed, 30 Jan 2019 15:05:50 +0100
Subject: [PATCH 04/11] Can't use `layout_of_local` for the frame currently
 being created

---
 src/librustc_mir/interpret/eval_context.rs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index d890e2fbe46da..bb4f5c131d628 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -509,9 +509,10 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
             for (idx, local) in locals.iter_enumerated_mut() {
                 match local.state {
                     LocalState::Live(_) => {
-                        // This needs to be peoperly initialized.
-                        let layout = self.layout_of_local(self.frame(), idx, None)?;
+                        // This needs to be properly initialized.
+                        let layout = self.layout_of(mir.local_decls[idx].ty)?;
                         local.state = LocalState::Live(self.uninit_operand(layout)?);
+                        local.layout = Cell::new(Some(layout));
                     }
                     LocalState::Dead => {
                         // Nothing to do

From ab708f5c6f7077ccf0b30baeb9395d372f5dbcce Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Wed, 30 Jan 2019 15:08:59 +0100
Subject: [PATCH 05/11] The return place's layout is only used once per frame,
 so caching doesn't help

---
 src/librustc_mir/interpret/place.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs
index ffb8ec899a07d..ba1960300a85c 100644
--- a/src/librustc_mir/interpret/place.rs
+++ b/src/librustc_mir/interpret/place.rs
@@ -624,7 +624,7 @@ where
                     // their layout on return.
                     PlaceTy {
                         place: *return_place,
-                        layout: self.layout_of_local(self.frame(), mir::RETURN_PLACE, None)?,
+                        layout: self.layout_of(self.frame().mir.return_ty())?,
                     },
                 None => return err!(InvalidNullPointerUsage),
             },

From 7017927aaf5ba14cec6c28b1079a93abfc715985 Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Wed, 30 Jan 2019 15:24:41 +0100
Subject: [PATCH 06/11] Indent fixup

---
 src/librustc_mir/interpret/eval_context.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index bb4f5c131d628..3382288b99a25 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -323,8 +323,8 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
         let cell = &frame.locals[local].layout;
         if cell.get().is_none() {
             let layout = ::interpret::operand::from_known_layout(layout, || {
-            let local_ty = frame.mir.local_decls[local].ty;
-            let local_ty = self.monomorphize_with_substs(local_ty, frame.instance.substs);
+                let local_ty = frame.mir.local_decls[local].ty;
+                let local_ty = self.monomorphize_with_substs(local_ty, frame.instance.substs);
                 self.layout_of(local_ty)
             })?;
             cell.set(Some(layout));

From 4e0af1fee19c2ff056e20a55464c233dc30e5c92 Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Wed, 30 Jan 2019 15:42:00 +0100
Subject: [PATCH 07/11] Monomorphize types when not going through
 `layout_of_local`

---
 src/librustc_mir/interpret/eval_context.rs | 3 ++-
 src/librustc_mir/interpret/place.rs        | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index 3382288b99a25..4bf84cb465f77 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -510,7 +510,8 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
                 match local.state {
                     LocalState::Live(_) => {
                         // This needs to be properly initialized.
-                        let layout = self.layout_of(mir.local_decls[idx].ty)?;
+                        let ty = self.monomorphize(mir.local_decls[idx].ty)?;
+                        let layout = self.layout_of(ty)?;
                         local.state = LocalState::Live(self.uninit_operand(layout)?);
                         local.layout = Cell::new(Some(layout));
                     }
diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs
index ba1960300a85c..9ca7f9d8e27ff 100644
--- a/src/librustc_mir/interpret/place.rs
+++ b/src/librustc_mir/interpret/place.rs
@@ -624,7 +624,7 @@ where
                     // their layout on return.
                     PlaceTy {
                         place: *return_place,
-                        layout: self.layout_of(self.frame().mir.return_ty())?,
+                        layout: self.layout_of(self.monomorphize(self.frame().mir.return_ty())?)?,
                     },
                 None => return err!(InvalidNullPointerUsage),
             },

From 5aa713e1c39b0e84ae9c96f99514f4981d3cea30 Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Wed, 30 Jan 2019 15:51:20 +0100
Subject: [PATCH 08/11] Eliminate an unwrap

---
 src/librustc_mir/interpret/eval_context.rs | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index 4bf84cb465f77..4e66d21214b29 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -320,17 +320,18 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
         local: mir::Local,
         layout: Option<TyLayout<'tcx>>,
     ) -> EvalResult<'tcx, TyLayout<'tcx>> {
-        let cell = &frame.locals[local].layout;
-        if cell.get().is_none() {
-            let layout = ::interpret::operand::from_known_layout(layout, || {
-                let local_ty = frame.mir.local_decls[local].ty;
-                let local_ty = self.monomorphize_with_substs(local_ty, frame.instance.substs);
-                self.layout_of(local_ty)
-            })?;
-            cell.set(Some(layout));
+        match frame.locals[local].layout.get() {
+            None => {
+                let layout = ::interpret::operand::from_known_layout(layout, || {
+                    let local_ty = frame.mir.local_decls[local].ty;
+                    let local_ty = self.monomorphize_with_substs(local_ty, frame.instance.substs);
+                    self.layout_of(local_ty)
+                })?;
+                frame.locals[local].layout.set(Some(layout));
+                Ok(layout)
+            }
+            Some(layout) => Ok(layout),
         }
-
-        Ok(cell.get().unwrap())
     }
 
     pub fn str_to_immediate(&mut self, s: &str) -> EvalResult<'tcx, Immediate<M::PointerTag>> {

From a7a5cb620feab14defb633af4ebf8d0671c22441 Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Wed, 30 Jan 2019 17:50:46 +0100
Subject: [PATCH 09/11] Prefer macro over manual implementation

---
 src/librustc_mir/interpret/snapshot.rs | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs
index 0b5dc9446921a..c9fe673c847d1 100644
--- a/src/librustc_mir/interpret/snapshot.rs
+++ b/src/librustc_mir/interpret/snapshot.rs
@@ -7,7 +7,7 @@
 
 use std::hash::{Hash, Hasher};
 
-use rustc::ich::{StableHashingContextProvider, StableHashingContext};
+use rustc::ich::StableHashingContextProvider;
 use rustc::mir;
 use rustc::mir::interpret::{
     AllocId, Pointer, Scalar,
@@ -19,7 +19,7 @@ use rustc::ty::{self, TyCtxt};
 use rustc::ty::layout::Align;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::indexed_vec::IndexVec;
-use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableHasherResult};
+use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use syntax::ast::Mutability;
 use syntax::source_map::Span;
 
@@ -366,16 +366,10 @@ impl<'a, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a LocalValue<'tcx>
     }
 }
 
-
-impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for LocalValue<'gcx> {
-    fn hash_stable<W: StableHasherResult>(
-        &self,
-        hcx: &mut StableHashingContext<'a>,
-        hasher: &mut StableHasher<W>,
-    ) {
-        self.state.hash_stable(hcx, hasher);
-    }
-}
+impl_stable_hash_for!(struct LocalValue<'tcx> {
+    state,
+    layout -> _,
+});
 
 impl<'a, 'b, 'mir, 'tcx: 'a+'mir> SnapshotContext<'b>
     for Memory<'a, 'mir, 'tcx, CompileTimeInterpreter<'a, 'mir, 'tcx>>

From 765fa81a6e8d063db60be20e710e51d8ca995fbd Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Wed, 30 Jan 2019 17:51:59 +0100
Subject: [PATCH 10/11] Swap the names of `LocalValue` and `LocalState`

---
 src/librustc/mir/interpret/value.rs        |  2 +-
 src/librustc_mir/interpret/eval_context.rs | 46 +++++++++++-----------
 src/librustc_mir/interpret/mod.rs          |  2 +-
 src/librustc_mir/interpret/snapshot.rs     | 16 ++++----
 4 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs
index 4ac84bcfd1903..1328a1aeeab96 100644
--- a/src/librustc/mir/interpret/value.rs
+++ b/src/librustc/mir/interpret/value.rs
@@ -14,7 +14,7 @@ pub struct RawConst<'tcx> {
 }
 
 /// Represents a constant value in Rust. Scalar and ScalarPair are optimizations which
-/// matches the LocalValue optimizations for easy conversions between Value and ConstValue.
+/// matches the LocalState optimizations for easy conversions between Value and ConstValue.
 #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, RustcEncodable, RustcDecodable, Hash)]
 pub enum ConstValue<'tcx> {
     /// Used only for types with layout::abi::Scalar ABI and ZSTs
diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index 4e66d21214b29..1b976d822ebff 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -76,7 +76,7 @@ pub struct Frame<'mir, 'tcx: 'mir, Tag=(), Extra=()> {
     /// The locals are stored as `Option<Value>`s.
     /// `None` represents a local that is currently dead, while a live local
     /// can either directly contain `Scalar` or refer to some part of an `Allocation`.
-    pub locals: IndexVec<mir::Local, LocalValue<'tcx, Tag>>,
+    pub locals: IndexVec<mir::Local, LocalState<'tcx, Tag>>,
 
     ////////////////////////////////////////////////////////////////////////////////
     // Current position within the function
@@ -107,15 +107,15 @@ pub enum StackPopCleanup {
 
 /// State of a local variable including a memoized layout
 #[derive(Clone, PartialEq, Eq)]
-pub struct LocalValue<'tcx, Tag=(), Id=AllocId> {
-    pub state: LocalState<Tag, Id>,
+pub struct LocalState<'tcx, Tag=(), Id=AllocId> {
+    pub state: LocalValue<Tag, Id>,
     /// Don't modify if `Some`, this is only used to prevent computing the layout twice
     pub layout: Cell<Option<TyLayout<'tcx>>>,
 }
 
 /// State of a local variable
 #[derive(Copy, Clone, PartialEq, Eq, Hash)]
-pub enum LocalState<Tag=(), Id=AllocId> {
+pub enum LocalValue<Tag=(), Id=AllocId> {
     Dead,
     // Mostly for convenience, we re-use the `Operand` type here.
     // This is an optimization over just always having a pointer here;
@@ -124,18 +124,18 @@ pub enum LocalState<Tag=(), Id=AllocId> {
     Live(Operand<Tag, Id>),
 }
 
-impl<'tcx, Tag> LocalValue<'tcx, Tag> {
+impl<'tcx, Tag> LocalState<'tcx, Tag> {
     pub fn access(&self) -> EvalResult<'tcx, &Operand<Tag>> {
         match self.state {
-            LocalState::Dead => err!(DeadLocal),
-            LocalState::Live(ref val) => Ok(val),
+            LocalValue::Dead => err!(DeadLocal),
+            LocalValue::Live(ref val) => Ok(val),
         }
     }
 
     pub fn access_mut(&mut self) -> EvalResult<'tcx, &mut Operand<Tag>> {
         match self.state {
-            LocalState::Dead => err!(DeadLocal),
-            LocalState::Live(ref mut val) => Ok(val),
+            LocalValue::Dead => err!(DeadLocal),
+            LocalValue::Live(ref mut val) => Ok(val),
         }
     }
 }
@@ -474,10 +474,10 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
         // don't allocate at all for trivial constants
         if mir.local_decls.len() > 1 {
             // We put some marker immediate into the locals that we later want to initialize.
-            // This can be anything except for LocalState::Dead -- because *that* is the
+            // This can be anything except for LocalValue::Dead -- because *that* is the
             // value we use for things that we know are initially dead.
-            let dummy = LocalValue {
-                state: LocalState::Live(Operand::Immediate(Immediate::Scalar(
+            let dummy = LocalState {
+                state: LocalValue::Live(Operand::Immediate(Immediate::Scalar(
                     ScalarMaybeUndef::Undef,
                 ))),
                 layout: Cell::new(None),
@@ -485,7 +485,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
             let mut locals = IndexVec::from_elem(dummy, &mir.local_decls);
             // Return place is handled specially by the `eval_place` functions, and the
             // entry in `locals` should never be used. Make it dead, to be sure.
-            locals[mir::RETURN_PLACE].state = LocalState::Dead;
+            locals[mir::RETURN_PLACE].state = LocalValue::Dead;
             // Now mark those locals as dead that we do not want to initialize
             match self.tcx.describe_def(instance.def_id()) {
                 // statics and constants don't have `Storage*` statements, no need to look for them
@@ -498,7 +498,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
                             match stmt.kind {
                                 StorageLive(local) |
                                 StorageDead(local) => {
-                                    locals[local].state = LocalState::Dead;
+                                    locals[local].state = LocalValue::Dead;
                                 }
                                 _ => {}
                             }
@@ -509,14 +509,14 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
             // Finally, properly initialize all those that still have the dummy value
             for (idx, local) in locals.iter_enumerated_mut() {
                 match local.state {
-                    LocalState::Live(_) => {
+                    LocalValue::Live(_) => {
                         // This needs to be properly initialized.
                         let ty = self.monomorphize(mir.local_decls[idx].ty)?;
                         let layout = self.layout_of(ty)?;
-                        local.state = LocalState::Live(self.uninit_operand(layout)?);
+                        local.state = LocalValue::Live(self.uninit_operand(layout)?);
                         local.layout = Cell::new(Some(layout));
                     }
-                    LocalState::Dead => {
+                    LocalValue::Dead => {
                         // Nothing to do
                     }
                 }
@@ -603,31 +603,31 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
     pub fn storage_live(
         &mut self,
         local: mir::Local
-    ) -> EvalResult<'tcx, LocalState<M::PointerTag>> {
+    ) -> EvalResult<'tcx, LocalValue<M::PointerTag>> {
         assert!(local != mir::RETURN_PLACE, "Cannot make return place live");
         trace!("{:?} is now live", local);
 
         let layout = self.layout_of_local(self.frame(), local, None)?;
-        let init = LocalState::Live(self.uninit_operand(layout)?);
+        let init = LocalValue::Live(self.uninit_operand(layout)?);
         // StorageLive *always* kills the value that's currently stored
         Ok(mem::replace(&mut self.frame_mut().locals[local].state, init))
     }
 
     /// Returns the old value of the local.
     /// Remember to deallocate that!
-    pub fn storage_dead(&mut self, local: mir::Local) -> LocalState<M::PointerTag> {
+    pub fn storage_dead(&mut self, local: mir::Local) -> LocalValue<M::PointerTag> {
         assert!(local != mir::RETURN_PLACE, "Cannot make return place dead");
         trace!("{:?} is now dead", local);
 
-        mem::replace(&mut self.frame_mut().locals[local].state, LocalState::Dead)
+        mem::replace(&mut self.frame_mut().locals[local].state, LocalValue::Dead)
     }
 
     pub(super) fn deallocate_local(
         &mut self,
-        local: LocalState<M::PointerTag>,
+        local: LocalValue<M::PointerTag>,
     ) -> EvalResult<'tcx> {
         // FIXME: should we tell the user that there was a local which was never written to?
-        if let LocalState::Live(Operand::Indirect(MemPlace { ptr, .. })) = local {
+        if let LocalValue::Live(Operand::Indirect(MemPlace { ptr, .. })) = local {
             trace!("deallocating local");
             let ptr = ptr.to_ptr()?;
             self.memory.dump_alloc(ptr.alloc_id);
diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs
index 6ee7d3309f464..d2ab3fcb7a30a 100644
--- a/src/librustc_mir/interpret/mod.rs
+++ b/src/librustc_mir/interpret/mod.rs
@@ -18,7 +18,7 @@ mod visitor;
 pub use rustc::mir::interpret::*; // have all the `interpret` symbols in one place: here
 
 pub use self::eval_context::{
-    EvalContext, Frame, StackPopCleanup, LocalValue, LocalState,
+    EvalContext, Frame, StackPopCleanup, LocalState, LocalValue,
 };
 
 pub use self::place::{Place, PlaceTy, MemPlace, MPlaceTy};
diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs
index c9fe673c847d1..c4b56f571db9c 100644
--- a/src/librustc_mir/interpret/snapshot.rs
+++ b/src/librustc_mir/interpret/snapshot.rs
@@ -23,8 +23,8 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use syntax::ast::Mutability;
 use syntax::source_map::Span;
 
-use super::eval_context::{LocalValue, StackPopCleanup};
-use super::{Frame, Memory, Operand, MemPlace, Place, Immediate, ScalarMaybeUndef, LocalState};
+use super::eval_context::{LocalState, StackPopCleanup};
+use super::{Frame, Memory, Operand, MemPlace, Place, Immediate, ScalarMaybeUndef, LocalValue};
 use const_eval::CompileTimeInterpreter;
 
 #[derive(Default)]
@@ -250,11 +250,11 @@ impl_snapshot_for!(enum Operand {
     Indirect(m),
 });
 
-impl_stable_hash_for!(enum ::interpret::LocalState {
+impl_stable_hash_for!(enum ::interpret::LocalValue {
     Dead,
     Live(x),
 });
-impl_snapshot_for!(enum LocalState {
+impl_snapshot_for!(enum LocalValue {
     Live(v),
     Dead,
 });
@@ -309,7 +309,7 @@ struct FrameSnapshot<'a, 'tcx: 'a> {
     span: &'a Span,
     return_to_block: &'a StackPopCleanup,
     return_place: Option<Place<(), AllocIdSnapshot<'a>>>,
-    locals: IndexVec<mir::Local, LocalState<(), AllocIdSnapshot<'a>>>,
+    locals: IndexVec<mir::Local, LocalValue<(), AllocIdSnapshot<'a>>>,
     block: &'a mir::BasicBlock,
     stmt: usize,
 }
@@ -356,17 +356,17 @@ impl<'a, 'mir, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a Frame<'mir, 'tcx>
     }
 }
 
-impl<'a, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a LocalValue<'tcx>
+impl<'a, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a LocalState<'tcx>
     where Ctx: SnapshotContext<'a>,
 {
-    type Item = LocalState<(), AllocIdSnapshot<'a>>;
+    type Item = LocalValue<(), AllocIdSnapshot<'a>>;
 
     fn snapshot(&self, ctx: &'a Ctx) -> Self::Item {
         self.state.snapshot(ctx)
     }
 }
 
-impl_stable_hash_for!(struct LocalValue<'tcx> {
+impl_stable_hash_for!(struct LocalState<'tcx> {
     state,
     layout -> _,
 });

From 8c26c590b4b22066f4ae5ac245dfa7c2e5ae4044 Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Wed, 30 Jan 2019 19:29:10 +0100
Subject: [PATCH 11/11] Failure resistent trait implementing

---
 src/librustc_mir/interpret/snapshot.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs
index c4b56f571db9c..5fae461bdc203 100644
--- a/src/librustc_mir/interpret/snapshot.rs
+++ b/src/librustc_mir/interpret/snapshot.rs
@@ -362,7 +362,8 @@ impl<'a, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a LocalState<'tcx>
     type Item = LocalValue<(), AllocIdSnapshot<'a>>;
 
     fn snapshot(&self, ctx: &'a Ctx) -> Self::Item {
-        self.state.snapshot(ctx)
+        let LocalState { state, layout: _ } = self;
+        state.snapshot(ctx)
     }
 }