diff options
author | Mazdak Farrokhzad <twingoow@gmail.com> | 2019-01-31 02:10:50 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-01-31 02:10:50 +0100 |
commit | 880f6334f7840d522b8ea9d484c3980e5e2e10dc (patch) | |
tree | 7de2a3c3a14f88b846a06541edb5db9de88f738a | |
parent | bb91a192c0b3b56d1cf88a1db13b767aa3c890d0 (diff) | |
parent | 8c26c590b4b22066f4ae5ac245dfa7c2e5ae4044 (diff) | |
download | rust-880f6334f7840d522b8ea9d484c3980e5e2e10dc.tar.gz |
Rollup merge of #58000 - oli-obk:fixes_and_cleanups, r=RalfJung
Fixes and cleanups
Address the points raised in https://github.com/rust-lang/rust/pull/57677/files by @eddyb and @RalfJung
-rw-r--r-- | src/librustc/mir/interpret/value.rs | 2 | ||||
-rw-r--r-- | src/librustc_mir/interpret/eval_context.rs | 72 | ||||
-rw-r--r-- | src/librustc_mir/interpret/mod.rs | 2 | ||||
-rw-r--r-- | src/librustc_mir/interpret/operand.rs | 14 | ||||
-rw-r--r-- | src/librustc_mir/interpret/place.rs | 6 | ||||
-rw-r--r-- | src/librustc_mir/interpret/snapshot.rs | 22 | ||||
-rw-r--r-- | src/librustc_mir/interpret/terminator.rs | 4 |
7 files changed, 77 insertions, 45 deletions
diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 4ac84bcfd19..1328a1aeeab 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 34443bb353e..1b976d822eb 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, LocalState<'tcx, Tag>>, //////////////////////////////////////////////////////////////////////////////// // Current position within the function @@ -106,7 +105,15 @@ 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 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 LocalValue<Tag=(), Id=AllocId> { Dead, @@ -117,16 +124,16 @@ pub enum LocalValue<Tag=(), Id=AllocId> { Live(Operand<Tag, Id>), } -impl<'tcx, Tag> LocalValue<Tag> { +impl<'tcx, Tag> LocalState<'tcx, Tag> { pub fn access(&self) -> EvalResult<'tcx, &Operand<Tag>> { - match self { + match self.state { LocalValue::Dead => err!(DeadLocal), LocalValue::Live(ref val) => Ok(val), } } pub fn access_mut(&mut self) -> EvalResult<'tcx, &mut Operand<Tag>> { - match self { + match self.state { LocalValue::Dead => err!(DeadLocal), LocalValue::Live(ref mut val) => Ok(val), } @@ -310,17 +317,21 @@ 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.local_layouts[local]; - 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); - let layout = 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>> { @@ -454,7 +465,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, @@ -466,12 +476,16 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc // 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 // value we use for things that we know are initially dead. - let dummy = - LocalValue::Live(Operand::Immediate(Immediate::Scalar(ScalarMaybeUndef::Undef))); + let dummy = LocalState { + state: LocalValue::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 = 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 @@ -484,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] = LocalValue::Dead; + locals[local].state = LocalValue::Dead; } _ => {} } @@ -494,11 +508,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 { + match local.state { LocalValue::Live(_) => { - // This needs to be peoperly initialized. - let layout = self.layout_of_local(self.frame(), idx)?; - *local = LocalValue::Live(self.uninit_operand(layout)?); + // This needs to be properly initialized. + let ty = self.monomorphize(mir.local_decls[idx].ty)?; + let layout = self.layout_of(ty)?; + local.state = LocalValue::Live(self.uninit_operand(layout)?); + local.layout = Cell::new(Some(layout)); } LocalValue::Dead => { // Nothing to do @@ -543,7 +559,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. @@ -591,10 +607,10 @@ 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 = LocalValue::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. @@ -603,7 +619,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc 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, LocalValue::Dead) } pub(super) fn deallocate_local( diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index e3ab90a6020..d2ab3fcb7a3 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, LocalState, LocalValue, }; pub use self::place::{Place, PlaceTy, MemPlace, MPlaceTy}; diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index e4bee24c88c..37e421c2e73 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>> { @@ -457,14 +457,15 @@ 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, + 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 f3a948a6ca3..9ca7f9d8e27 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(self.monomorphize(self.frame().mir.return_ty())?)?, }, 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/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs index 53105266b39..5fae461bdc2 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}; +use super::eval_context::{LocalState, StackPopCleanup}; +use super::{Frame, Memory, Operand, MemPlace, Place, Immediate, ScalarMaybeUndef, LocalValue}; use const_eval::CompileTimeInterpreter; #[derive(Default)] @@ -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,22 @@ impl<'a, 'mir, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a Frame<'mir, 'tcx> } } +impl<'a, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a LocalState<'tcx> + where Ctx: SnapshotContext<'a>, +{ + type Item = LocalValue<(), AllocIdSnapshot<'a>>; + + fn snapshot(&self, ctx: &'a Ctx) -> Self::Item { + let LocalState { state, layout: _ } = self; + state.snapshot(ctx) + } +} + +impl_stable_hash_for!(struct LocalState<'tcx> { + state, + layout -> _, +}); + impl<'a, 'b, 'mir, 'tcx: 'a+'mir> SnapshotContext<'b> for Memory<'a, 'mir, 'tcx, CompileTimeInterpreter<'a, 'mir, 'tcx>> { diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 951e9fabe59..7e823524c18 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 |