interpret: make Place always refer to the current frame

Author: RalfJung

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -210,7 +210,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
         if Some(def_id) == self.tcx.lang_items().panic_display()
             || Some(def_id) == self.tcx.lang_items().begin_panic_fn()
         {
-            let args = self.copy_fn_args(args)?;
+            let args = self.copy_fn_args(args);
             // &str or &&str
             assert!(args.len() == 1);
 
@@ -237,7 +237,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
 
             return Ok(Some(new_instance));
         } else if Some(def_id) == self.tcx.lang_items().align_offset_fn() {
-            let args = self.copy_fn_args(args)?;
+            let args = self.copy_fn_args(args);
             // For align_offset, we replace the function call if the pointer has no address.
             match self.align_offset(instance, &args, dest, ret)? {
                 ControlFlow::Continue(()) => return Ok(Some(instance)),

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -21,8 +21,8 @@ use rustc_target::abi::{call::FnAbi, Align, HasDataLayout, Size, TargetDataLayou
 
 use super::{
     AllocId, GlobalId, Immediate, InterpErrorInfo, InterpResult, MPlaceTy, Machine, MemPlace,
-    MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, PointerArithmetic, Provenance,
-    Scalar, StackPopJump,
+    MemPlaceMeta, Memory, MemoryKind, Operand, Place, PointerArithmetic, Provenance, Scalar,
+    StackPopJump,
 };
 use crate::errors::{self, ErroneousConstUsed};
 use crate::fluent_generated as fluent;
@@ -105,7 +105,7 @@ pub struct Frame<'mir, 'tcx, Prov: Provenance = AllocId, Extra = ()> {
 
     /// The location where the result of the current stack frame should be written to,
     /// and its layout in the caller.
-    pub return_place: PlaceTy<'tcx, Prov>,
+    pub return_place: MPlaceTy<'tcx, Prov>,
 
     /// The list of locals for this stack frame, stored in order as
     /// `[return_ptr, arguments..., variables..., temporaries...]`.
@@ -680,7 +680,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         &mut self,
         instance: ty::Instance<'tcx>,
         body: &'mir mir::Body<'tcx>,
-        return_place: &PlaceTy<'tcx, M::Provenance>,
+        return_place: &MPlaceTy<'tcx, M::Provenance>,
         return_to_block: StackPopCleanup,
     ) -> InterpResult<'tcx> {
         trace!("body: {:#?}", body);
@@ -810,7 +810,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             let op = self
                 .local_to_op(self.frame(), mir::RETURN_PLACE, None)
                 .expect("return place should always be live");
-            let dest = self.frame().return_place.clone();
+            let dest = self.frame().return_place.into();
             let err = self.copy_op(&op, &dest, /*allow_transmute*/ true);
             trace!("return value: {:?}", self.dump_place(*dest));
             // We delay actually short-circuiting on this error until *after* the stack frame is
@@ -1014,15 +1014,11 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> std::fmt::Debug
 {
     fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self.place {
-            Place::Local { frame, local } => {
+            Place::Local { local } => {
                 let mut allocs = Vec::new();
-                write!(fmt, "{:?}", local)?;
-                if frame != self.ecx.frame_idx() {
-                    write!(fmt, " ({} frames up)", self.ecx.frame_idx() - frame)?;
-                }
-                write!(fmt, ":")?;
+                write!(fmt, "{:?}:", local)?;
 
-                match self.ecx.stack()[frame].locals[local].value {
+                match self.ecx.frame().locals[local].value {
                     LocalValue::Dead => write!(fmt, " is dead")?,
                     LocalValue::Live(Operand::Immediate(Immediate::Uninit)) => {
                         write!(fmt, " is uninitialized")?

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -18,7 +18,7 @@ use crate::const_eval::CheckAlignment;
 
 use super::{
     AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
-    InterpResult, MemoryKind, OpTy, Operand, PlaceTy, Pointer, Provenance, Scalar,
+    InterpResult, MPlaceTy, MemoryKind, OpTy, Operand, PlaceTy, Pointer, Provenance, Scalar,
 };
 
 /// Data returned by Machine::stack_pop,
@@ -233,22 +233,18 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
         right: &ImmTy<'tcx, Self::Provenance>,
     ) -> InterpResult<'tcx, (Scalar<Self::Provenance>, bool, Ty<'tcx>)>;
 
-    /// Called to write the specified `local` from the `frame`.
+    /// Called to write the specified `local` from the current frame.
     /// Since writing a ZST is not actually accessing memory or locals, this is never invoked
     /// for ZST reads.
-    ///
-    /// Due to borrow checker trouble, we indicate the `frame` as an index rather than an `&mut
-    /// Frame`.
     #[inline]
     fn access_local_mut<'a>(
         ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
-        frame: usize,
         local: mir::Local,
     ) -> InterpResult<'tcx, &'a mut Operand<Self::Provenance>>
     where
         'tcx: 'mir,
     {
-        ecx.stack_mut()[frame].locals[local].access_mut()
+        ecx.frame_mut().locals[local].access_mut()
     }
 
     /// Called before a basic block terminator is executed.
@@ -424,10 +420,10 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
     /// argument/return value was actually copied or passed in-place..
     fn protect_in_place_function_argument(
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
-        place: &PlaceTy<'tcx, Self::Provenance>,
+        place: &MPlaceTy<'tcx, Self::Provenance>,
     ) -> InterpResult<'tcx> {
         // Without an aliasing model, all we can do is put `Uninit` into the place.
-        ecx.write_uninit(place)
+        ecx.write_uninit(&place.into())
     }
 
     /// Called immediately before a new stack frame gets pushed.

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -504,9 +504,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     ) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
         let op = match **place {
             Place::Ptr(mplace) => Operand::Indirect(mplace),
-            Place::Local { frame, local } => {
-                *self.local_to_op(&self.stack()[frame], local, None)?
-            }
+            Place::Local { local } => *self.local_to_op(&self.frame(), local, None)?,
         };
         Ok(OpTy { op, layout: place.layout, align: Some(place.align) })
     }

compiler/rustc_const_eval/src/interpret/place.rs

@@ -75,7 +75,9 @@ pub enum Place<Prov: Provenance = AllocId> {
 
     /// To support alloc-free locals, we are able to write directly to a local.
     /// (Without that optimization, we'd just always be a `MemPlace`.)
-    Local { frame: usize, local: mir::Local },
+    /// This always refers to a local in the current stack frame. That works because `Place` is
+    /// never stored anywhere long-term, only for the duration of evaluating a single statement.
+    Local { local: mir::Local },
 }
 
 #[derive(Clone, Debug)]
@@ -169,9 +171,9 @@ impl<Prov: Provenance> Place<Prov> {
     /// Returns the frame idx and the variable idx.
     #[inline]
     #[cfg_attr(debug_assertions, track_caller)] // only in debug builds due to perf (see #98980)
-    pub fn assert_local(&self) -> (usize, mir::Local) {
+    pub fn assert_local(&self) -> mir::Local {
         match self {
-            Place::Local { frame, local } => (*frame, *local),
+            Place::Local { local } => *local,
             _ => bug!("assert_local: expected Place::Local, got {:?}", self),
         }
     }
@@ -283,10 +285,10 @@ impl<'tcx, Prov: Provenance> OpTy<'tcx, Prov> {
 impl<'tcx, Prov: Provenance> PlaceTy<'tcx, Prov> {
     /// A place is either an mplace or some local.
     #[inline]
-    pub fn as_mplace_or_local(&self) -> Either<MPlaceTy<'tcx, Prov>, (usize, mir::Local)> {
+    pub fn as_mplace_or_local(&self) -> Either<MPlaceTy<'tcx, Prov>, mir::Local> {
         match **self {
             Place::Ptr(mplace) => Left(MPlaceTy { mplace, layout: self.layout, align: self.align }),
-            Place::Local { frame, local } => Right((frame, local)),
+            Place::Local { local } => Right(local),
         }
     }
 
@@ -348,7 +350,7 @@ where
         }
 
         let mplace = self.ref_to_mplace(&val)?;
-        self.check_mplace(mplace)?;
+        self.check_mplace(&mplace)?;
         Ok(mplace)
     }
 
@@ -379,7 +381,7 @@ where
     }
 
     /// Check if this mplace is dereferenceable and sufficiently aligned.
-    pub fn check_mplace(&self, mplace: MPlaceTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
+    pub fn check_mplace(&self, mplace: &MPlaceTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
         let (size, _align) = self
             .size_and_align_of_mplace(&mplace)?
             .unwrap_or((mplace.layout.size, mplace.layout.align.abi));
@@ -418,11 +420,10 @@ where
 
     pub fn local_to_place(
         &self,
-        frame: usize,
         local: mir::Local,
     ) -> InterpResult<'tcx, PlaceTy<'tcx, M::Provenance>> {
-        let layout = self.layout_of_local(&self.stack()[frame], local, None)?;
-        let place = Place::Local { frame, local };
+        let layout = self.layout_of_local(&self.frame(), local, None)?;
+        let place = Place::Local { local };
         Ok(PlaceTy { place, layout, align: layout.align.abi })
     }
 
@@ -433,7 +434,7 @@ where
         &mut self,
         mir_place: mir::Place<'tcx>,
     ) -> InterpResult<'tcx, PlaceTy<'tcx, M::Provenance>> {
-        let mut place = self.local_to_place(self.frame_idx(), mir_place.local)?;
+        let mut place = self.local_to_place(mir_place.local)?;
         // Using `try_fold` turned out to be bad for performance, hence the loop.
         for elem in mir_place.projection.iter() {
             place = self.place_projection(&place, elem)?
@@ -509,8 +510,8 @@ where
         // See if we can avoid an allocation. This is the counterpart to `read_immediate_raw`,
         // but not factored as a separate function.
         let mplace = match dest.place {
-            Place::Local { frame, local } => {
-                match M::access_local_mut(self, frame, local)? {
+            Place::Local { local } => {
+                match M::access_local_mut(self, local)? {
                     Operand::Immediate(local) => {
                         // Local can be updated in-place.
                         *local = src;
@@ -593,8 +594,8 @@ where
     pub fn write_uninit(&mut self, dest: &PlaceTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
         let mplace = match dest.as_mplace_or_local() {
             Left(mplace) => mplace,
-            Right((frame, local)) => {
-                match M::access_local_mut(self, frame, local)? {
+            Right(local) => {
+                match M::access_local_mut(self, local)? {
                     Operand::Immediate(local) => {
                         *local = Immediate::Uninit;
                         return Ok(());
@@ -728,16 +729,15 @@ where
         place: &PlaceTy<'tcx, M::Provenance>,
     ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
         let mplace = match place.place {
-            Place::Local { frame, local } => {
-                match M::access_local_mut(self, frame, local)? {
+            Place::Local { local } => {
+                match M::access_local_mut(self, local)? {
                     &mut Operand::Immediate(local_val) => {
                         // We need to make an allocation.
 
                         // 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, None)?;
+                        let local_layout = self.layout_of_local(&self.frame(), local, None)?;
                         if local_layout.is_unsized() {
                             throw_unsup_format!("unsized locals are not supported");
                         }
@@ -755,8 +755,7 @@ where
                         }
                         // Now we can call `access_mut` again, asserting it goes well,
                         // and actually overwrite things.
-                        *M::access_local_mut(self, frame, local).unwrap() =
-                            Operand::Indirect(mplace);
+                        *M::access_local_mut(self, local).unwrap() = Operand::Indirect(mplace);
                         mplace
                     }
                     &mut Operand::Indirect(mplace) => mplace, // this already was an indirect local