Auto merge of rust-lang#114071 - RalfJung:interpret-generic-read-write, r=oli-obk

interpret: make read/write methods generic

Instead of always having to call `into()` to convert things to `PlaceTy`/`OpTy`, make the relevant methods generic. This also means that when we read from an `MPlaceTy`, we avoid creating an intermediate `PlaceTy`.

This makes it feasible to remove the `Copy` from `MPlaceTy`. All the other `*Ty` interpreter types already had their `Copy` removed a while ago so this is only consistent. (And in fact we had one function that accidentally took `MPlaceTy` instead of `&MPlaceTy`.)

Author: bors

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -58,7 +58,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
     ecx.push_stack_frame(
         cid.instance,
         body,
-        &ret.into(),
+        &ret.clone().into(),
         StackPopCleanup::Root { cleanup: false },
     )?;
 
@@ -356,7 +356,7 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
             // Since evaluation had no errors, validate the resulting constant.
             // This is a separate `try` block to provide more targeted error reporting.
             let validation: Result<_, InterpErrorInfo<'_>> = try {
-                let mut ref_tracking = RefTracking::new(mplace);
+                let mut ref_tracking = RefTracking::new(mplace.clone());
                 let mut inner = false;
                 while let Some((mplace, path)) = ref_tracking.todo.pop() {
                     let mode = match tcx.static_mutability(cid.instance.def_id()) {

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -216,7 +216,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
 
             let mut msg_place = self.deref_operand(&args[0])?;
             while msg_place.layout.ty.is_ref() {
-                msg_place = self.deref_operand(&msg_place.into())?;
+                msg_place = self.deref_operand(&msg_place)?;
             }
 
             let msg = Symbol::intern(self.read_str(&msg_place)?);

compiler/rustc_const_eval/src/const_eval/valtrees.rs

@@ -5,7 +5,7 @@ use crate::const_eval::CanAccessStatics;
 use crate::interpret::MPlaceTy;
 use crate::interpret::{
     intern_const_alloc_recursive, ConstValue, ImmTy, Immediate, InternKind, MemPlaceMeta,
-    MemoryKind, PlaceTy, Projectable, Scalar,
+    MemoryKind, Place, Projectable, Scalar,
 };
 use rustc_middle::ty::{self, ScalarInt, Ty, TyCtxt};
 use rustc_span::source_map::DUMMY_SP;
@@ -21,7 +21,7 @@ fn branches<'tcx>(
 ) -> ValTreeCreationResult<'tcx> {
     let place = match variant {
         Some(variant) => ecx.project_downcast(place, variant).unwrap(),
-        None => *place,
+        None => place.clone(),
     };
     let variant = variant.map(|variant| Some(ty::ValTree::Leaf(ScalarInt::from(variant.as_u32()))));
     debug!(?place, ?variant);
@@ -86,7 +86,7 @@ pub(crate) fn const_to_valtree_inner<'tcx>(
             Ok(ty::ValTree::zst())
         }
         ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char => {
-            let Ok(val) = ecx.read_immediate(&place.into()) else {
+            let Ok(val) = ecx.read_immediate(place) else {
                 return Err(ValTreeCreationError::Other);
             };
             let val = val.to_scalar();
@@ -102,7 +102,7 @@ pub(crate) fn const_to_valtree_inner<'tcx>(
         ty::FnPtr(_) | ty::RawPtr(_) => Err(ValTreeCreationError::NonSupportedType),
 
         ty::Ref(_, _, _)  => {
-            let Ok(derefd_place)= ecx.deref_operand(&place.into()) else {
+            let Ok(derefd_place)= ecx.deref_operand(place) else {
                 return Err(ValTreeCreationError::Other);
             };
             debug!(?derefd_place);
@@ -130,7 +130,7 @@ pub(crate) fn const_to_valtree_inner<'tcx>(
                 bug!("uninhabited types should have errored and never gotten converted to valtree")
             }
 
-            let Ok(variant) = ecx.read_discriminant(&place.into()) else {
+            let Ok(variant) = ecx.read_discriminant(place) else {
                 return Err(ValTreeCreationError::Other);
             };
             branches(ecx, place, def.variant(variant).fields.len(), def.is_enum().then_some(variant), num_nodes)
@@ -280,7 +280,7 @@ pub fn valtree_to_const_value<'tcx>(
             ),
         },
         ty::Ref(_, _, _) | ty::Tuple(_) | ty::Array(_, _) | ty::Adt(..) => {
-            let mut place = match ty.kind() {
+            let place = match ty.kind() {
                 ty::Ref(_, inner_ty, _) => {
                     // Need to create a place for the pointee to fill for Refs
                     create_pointee_place(&mut ecx, *inner_ty, valtree)
@@ -289,8 +289,8 @@ pub fn valtree_to_const_value<'tcx>(
             };
             debug!(?place);
 
-            valtree_into_mplace(&mut ecx, &mut place, valtree);
-            dump_place(&ecx, place.into());
+            valtree_into_mplace(&mut ecx, &place, valtree);
+            dump_place(&ecx, &place);
             intern_const_alloc_recursive(&mut ecx, InternKind::Constant, &place).unwrap();
 
             match ty.kind() {
@@ -329,7 +329,7 @@ pub fn valtree_to_const_value<'tcx>(
 #[instrument(skip(ecx), level = "debug")]
 fn valtree_into_mplace<'tcx>(
     ecx: &mut CompileTimeEvalContext<'tcx, 'tcx>,
-    place: &mut MPlaceTy<'tcx>,
+    place: &MPlaceTy<'tcx>,
     valtree: ty::ValTree<'tcx>,
 ) {
     // This will match on valtree and write the value(s) corresponding to the ValTree
@@ -345,14 +345,14 @@ fn valtree_into_mplace<'tcx>(
         ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char => {
             let scalar_int = valtree.unwrap_leaf();
             debug!("writing trivial valtree {:?} to place {:?}", scalar_int, place);
-            ecx.write_immediate(Immediate::Scalar(scalar_int.into()), &place.into()).unwrap();
+            ecx.write_immediate(Immediate::Scalar(scalar_int.into()), place).unwrap();
         }
         ty::Ref(_, inner_ty, _) => {
-            let mut pointee_place = create_pointee_place(ecx, *inner_ty, valtree);
+            let pointee_place = create_pointee_place(ecx, *inner_ty, valtree);
             debug!(?pointee_place);
 
-            valtree_into_mplace(ecx, &mut pointee_place, valtree);
-            dump_place(ecx, pointee_place.into());
+            valtree_into_mplace(ecx, &pointee_place, valtree);
+            dump_place(ecx, &pointee_place);
             intern_const_alloc_recursive(ecx, InternKind::Constant, &pointee_place).unwrap();
 
             let imm = match inner_ty.kind() {
@@ -369,7 +369,7 @@ fn valtree_into_mplace<'tcx>(
             };
             debug!(?imm);
 
-            ecx.write_immediate(imm, &place.into()).unwrap();
+            ecx.write_immediate(imm, place).unwrap();
         }
         ty::Adt(_, _) | ty::Tuple(_) | ty::Array(_, _) | ty::Str | ty::Slice(_) => {
             let branches = valtree.unwrap_branch();
@@ -389,7 +389,7 @@ fn valtree_into_mplace<'tcx>(
                         Some(variant_idx),
                     )
                 }
-                _ => (*place, branches, None),
+                _ => (place.clone(), branches, None),
             };
             debug!(?place_adjusted, ?branches);
 
@@ -398,7 +398,7 @@ fn valtree_into_mplace<'tcx>(
             for (i, inner_valtree) in branches.iter().enumerate() {
                 debug!(?i, ?inner_valtree);
 
-                let mut place_inner = match ty.kind() {
+                let place_inner = match ty.kind() {
                     ty::Str | ty::Slice(_) => ecx.project_index(place, i as u64).unwrap(),
                     _ if !ty.is_sized(*ecx.tcx, ty::ParamEnv::empty())
                         && i == branches.len() - 1 =>
@@ -443,25 +443,25 @@ fn valtree_into_mplace<'tcx>(
                 };
 
                 debug!(?place_inner);
-                valtree_into_mplace(ecx, &mut place_inner, *inner_valtree);
-                dump_place(&ecx, place_inner.into());
+                valtree_into_mplace(ecx, &place_inner, *inner_valtree);
+                dump_place(&ecx, &place_inner);
             }
 
             debug!("dump of place_adjusted:");
-            dump_place(ecx, place_adjusted.into());
+            dump_place(ecx, &place_adjusted);
 
             if let Some(variant_idx) = variant_idx {
                 // don't forget filling the place with the discriminant of the enum
-                ecx.write_discriminant(variant_idx, &place.into()).unwrap();
+                ecx.write_discriminant(variant_idx, place).unwrap();
             }
 
             debug!("dump of place after writing discriminant:");
-            dump_place(ecx, place.into());
+            dump_place(ecx, place);
         }
         _ => bug!("shouldn't have created a ValTree for {:?}", ty),
     }
 }
 
-fn dump_place<'tcx>(ecx: &CompileTimeEvalContext<'tcx, 'tcx>, place: PlaceTy<'tcx>) {
-    trace!("{:?}", ecx.dump_place(*place));
+fn dump_place<'tcx>(ecx: &CompileTimeEvalContext<'tcx, 'tcx>, place: &MPlaceTy<'tcx>) {
+    trace!("{:?}", ecx.dump_place(Place::Ptr(**place)));
 }

compiler/rustc_const_eval/src/interpret/cast.rs

@@ -56,7 +56,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
 
             CastKind::FnPtrToPtr | CastKind::PtrToPtr => {
-                let src = self.read_immediate(&src)?;
+                let src = self.read_immediate(src)?;
                 let res = self.ptr_to_ptr(&src, cast_ty)?;
                 self.write_immediate(res, dest)?;
             }

compiler/rustc_const_eval/src/interpret/discriminant.rs

@@ -5,27 +5,27 @@ use rustc_middle::{mir, ty};
 use rustc_target::abi::{self, TagEncoding};
 use rustc_target::abi::{VariantIdx, Variants};
 
-use super::{ImmTy, InterpCx, InterpResult, Machine, OpTy, PlaceTy, Scalar};
+use super::{ImmTy, InterpCx, InterpResult, Machine, Readable, Scalar, Writeable};
 
 impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// Writes the discriminant of the given variant.
     #[instrument(skip(self), level = "trace")]
     pub fn write_discriminant(
         &mut self,
         variant_index: VariantIdx,
-        dest: &PlaceTy<'tcx, M::Provenance>,
+        dest: &impl Writeable<'tcx, M::Provenance>,
     ) -> InterpResult<'tcx> {
         // Layout computation excludes uninhabited variants from consideration
         // therefore there's no way to represent those variants in the given layout.
         // Essentially, uninhabited variants do not have a tag that corresponds to their
         // discriminant, so we cannot do anything here.
         // When evaluating we will always error before even getting here, but ConstProp 'executes'
         // dead code, so we cannot ICE here.
-        if dest.layout.for_variant(self, variant_index).abi.is_uninhabited() {
+        if dest.layout().for_variant(self, variant_index).abi.is_uninhabited() {
             throw_ub!(UninhabitedEnumVariantWritten(variant_index))
         }
 
-        match dest.layout.variants {
+        match dest.layout().variants {
             abi::Variants::Single { index } => {
                 assert_eq!(index, variant_index);
             }
@@ -38,8 +38,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 // No need to validate that the discriminant here because the
                 // `TyAndLayout::for_variant()` call earlier already checks the variant is valid.
 
-                let discr_val =
-                    dest.layout.ty.discriminant_for_variant(*self.tcx, variant_index).unwrap().val;
+                let discr_val = dest
+                    .layout()
+                    .ty
+                    .discriminant_for_variant(*self.tcx, variant_index)
+                    .unwrap()
+                    .val;
 
                 // raw discriminants for enums are isize or bigger during
                 // their computation, but the in-memory tag is the smallest possible
@@ -92,32 +96,32 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     #[instrument(skip(self), level = "trace")]
     pub fn read_discriminant(
         &self,
-        op: &OpTy<'tcx, M::Provenance>,
+        op: &impl Readable<'tcx, M::Provenance>,
     ) -> InterpResult<'tcx, VariantIdx> {
-        trace!("read_discriminant_value {:#?}", op.layout);
+        let ty = op.layout().ty;
+        trace!("read_discriminant_value {:#?}", op.layout());
         // Get type and layout of the discriminant.
-        let discr_layout = self.layout_of(op.layout.ty.discriminant_ty(*self.tcx))?;
+        let discr_layout = self.layout_of(ty.discriminant_ty(*self.tcx))?;
         trace!("discriminant type: {:?}", discr_layout.ty);
 
         // We use "discriminant" to refer to the value associated with a particular enum variant.
         // This is not to be confused with its "variant index", which is just determining its position in the
         // declared list of variants -- they can differ with explicitly assigned discriminants.
         // We use "tag" to refer to how the discriminant is encoded in memory, which can be either
         // straight-forward (`TagEncoding::Direct`) or with a niche (`TagEncoding::Niche`).
-        let (tag_scalar_layout, tag_encoding, tag_field) = match op.layout.variants {
+        let (tag_scalar_layout, tag_encoding, tag_field) = match op.layout().variants {
             Variants::Single { index } => {
                 // Do some extra checks on enums.
-                if op.layout.ty.is_enum() {
+                if ty.is_enum() {
                     // Hilariously, `Single` is used even for 0-variant enums.
                     // (See https://github.com/rust-lang/rust/issues/89765).
-                    if matches!(op.layout.ty.kind(), ty::Adt(def, ..) if def.variants().is_empty())
-                    {
+                    if matches!(ty.kind(), ty::Adt(def, ..) if def.variants().is_empty()) {
                         throw_ub!(UninhabitedEnumVariantRead(index))
                     }
                     // For consisteny with `write_discriminant`, and to make sure that
                     // `project_downcast` cannot fail due to strange layouts, we declare immediate UB
                     // for uninhabited variants.
-                    if op.layout.for_variant(self, index).abi.is_uninhabited() {
+                    if op.layout().for_variant(self, index).abi.is_uninhabited() {
                         throw_ub!(UninhabitedEnumVariantRead(index))
                     }
                 }
@@ -163,7 +167,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     self.cast_from_int_like(scalar, tag_val.layout, discr_layout.ty).unwrap();
                 let discr_bits = discr_val.assert_bits(discr_layout.size);
                 // Convert discriminant to variant index, and catch invalid discriminants.
-                let index = match *op.layout.ty.kind() {
+                let index = match *ty.kind() {
                     ty::Adt(adt, _) => {
                         adt.discriminants(*self.tcx).find(|(_, var)| var.val == discr_bits)
                     }
@@ -217,12 +221,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                                     .checked_add(variant_index_relative)
                                     .expect("overflow computing absolute variant idx"),
                             );
-                            let variants = op
-                                .layout
-                                .ty
-                                .ty_adt_def()
-                                .expect("tagged layout for non adt")
-                                .variants();
+                            let variants =
+                                ty.ty_adt_def().expect("tagged layout for non adt").variants();
                             assert!(variant_index < variants.next_index());
                             variant_index
                         } else {
@@ -237,7 +237,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
         };
         // For consisteny with `write_discriminant`, and to make sure that `project_downcast` cannot fail due to strange layouts, we declare immediate UB for uninhabited variants.
-        if op.layout.for_variant(self, index).abi.is_uninhabited() {
+        if op.layout().for_variant(self, index).abi.is_uninhabited() {
             throw_ub!(UninhabitedEnumVariantRead(index))
         }
         Ok(index)

compiler/rustc_const_eval/src/interpret/intern.rs

@@ -170,7 +170,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
         let tcx = self.ecx.tcx;
         let ty = mplace.layout.ty;
         if let ty::Ref(_, referenced_ty, ref_mutability) = *ty.kind() {
-            let value = self.ecx.read_immediate(&mplace.into())?;
+            let value = self.ecx.read_immediate(mplace)?;
             let mplace = self.ecx.ref_to_mplace(&value)?;
             assert_eq!(mplace.layout.ty, referenced_ty);
             // Handle trait object vtables.
@@ -358,7 +358,7 @@ pub fn intern_const_alloc_recursive<
         Some(ret.layout.ty),
     );
 
-    ref_tracking.track((*ret, base_intern_mode), || ());
+    ref_tracking.track((ret.clone(), base_intern_mode), || ());
 
     while let Some(((mplace, mode), _)) = ref_tracking.todo.pop() {
         let res = InternVisitor {
@@ -464,7 +464,7 @@ impl<'mir, 'tcx: 'mir, M: super::intern::CompileTimeMachine<'mir, 'tcx, !>>
         ) -> InterpResult<'tcx, ()>,
     ) -> InterpResult<'tcx, ConstAllocation<'tcx>> {
         let dest = self.allocate(layout, MemoryKind::Stack)?;
-        f(self, &dest.into())?;
+        f(self, &dest.clone().into())?;
         let mut alloc = self.memory.alloc_map.remove(&dest.ptr.provenance.unwrap()).unwrap().1;
         alloc.mutability = Mutability::Not;
         Ok(self.tcx.mk_const_alloc(alloc))

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -226,7 +226,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
             sym::discriminant_value => {
                 let place = self.deref_operand(&args[0])?;
-                let variant = self.read_discriminant(&place.into())?;
+                let variant = self.read_discriminant(&place)?;
                 let discr = self.discriminant_for_variant(place.layout, variant)?;
                 self.write_scalar(discr, dest)?;
             }
@@ -432,7 +432,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     } else {
                         self.project_index(&input, i)?.into()
                     };
-                    self.copy_op(&value, &place.into(), /*allow_transmute*/ false)?;
+                    self.copy_op(&value, &place, /*allow_transmute*/ false)?;
                 }
             }
             sym::simd_extract => {
@@ -445,7 +445,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     input_len
                 );
                 self.copy_op(
-                    &self.project_index(&input, index)?.into(),
+                    &self.project_index(&input, index)?,
                     dest,
                     /*allow_transmute*/ false,
                 )?;
@@ -610,7 +610,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         count: &OpTy<'tcx, <M as Machine<'mir, 'tcx>>::Provenance>,
         nonoverlapping: bool,
     ) -> InterpResult<'tcx> {
-        let count = self.read_target_usize(&count)?;
+        let count = self.read_target_usize(count)?;
         let layout = self.layout_of(src.layout.ty.builtin_deref(true).unwrap().ty)?;
         let (size, align) = (layout.size, layout.align.abi);
         // `checked_mul` enforces a too small bound (the correct one would probably be target_isize_max),
@@ -622,8 +622,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             )
         })?;
 
-        let src = self.read_pointer(&src)?;
-        let dst = self.read_pointer(&dst)?;
+        let src = self.read_pointer(src)?;
+        let dst = self.read_pointer(dst)?;
 
         self.mem_copy(src, align, dst, align, size, nonoverlapping)
     }
@@ -636,9 +636,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     ) -> InterpResult<'tcx> {
         let layout = self.layout_of(dst.layout.ty.builtin_deref(true).unwrap().ty)?;
 
-        let dst = self.read_pointer(&dst)?;
-        let byte = self.read_scalar(&byte)?.to_u8()?;
-        let count = self.read_target_usize(&count)?;
+        let dst = self.read_pointer(dst)?;
+        let byte = self.read_scalar(byte)?.to_u8()?;
+        let count = self.read_target_usize(count)?;
 
         // `checked_mul` enforces a too small bound (the correct one would probably be target_isize_max),
         // but no actual allocation can be big enough for the difference to be noticeable.