interpret: refactor projection code to work on a common trait, and use that for visitors

Author: RalfJung

compiler/rustc_abi/src/lib.rs

@@ -1189,7 +1189,7 @@ impl FieldsShape {
             }
             FieldsShape::Array { stride, count } => {
                 let i = u64::try_from(i).unwrap();
-                assert!(i < count);
+                assert!(i < count, "tried to access field {} of array with {} fields", i, count);
                 stride * i
             }
             FieldsShape::Arbitrary { ref offsets, .. } => offsets[FieldIdx::from_usize(i)],

compiler/rustc_const_eval/messages.ftl

@@ -408,8 +408,11 @@ const_eval_undefined_behavior =
 const_eval_undefined_behavior_note =
     The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 
+const_eval_uninhabited_enum_tag = {$front_matter}: encountered an uninhabited enum variant
+const_eval_uninhabited_enum_variant_read =
+    read discriminant of an uninhabited enum variant
 const_eval_uninhabited_enum_variant_written =
-    writing discriminant of an uninhabited enum
+    writing discriminant of an uninhabited enum variant
 const_eval_uninhabited_val = {$front_matter}: encountered a value of uninhabited type `{$ty}`
 const_eval_uninit = {$front_matter}: encountered uninitialized bytes
 const_eval_uninit_bool = {$front_matter}: encountered uninitialized memory, but expected a boolean

compiler/rustc_const_eval/src/const_eval/mod.rs

@@ -102,7 +102,7 @@ pub(crate) fn try_destructure_mir_constant_for_diagnostics<'tcx>(
         }
         ty::Adt(def, _) => {
             let variant = ecx.read_discriminant(&op).ok()?.1;
-            let down = ecx.operand_downcast(&op, variant).ok()?;
+            let down = ecx.project_downcast(&op, variant).ok()?;
             (def.variants()[variant].fields.len(), Some(variant), down)
         }
         ty::Tuple(args) => (args.len(), None, op),
@@ -111,7 +111,7 @@ pub(crate) fn try_destructure_mir_constant_for_diagnostics<'tcx>(
 
     let fields_iter = (0..field_count)
         .map(|i| {
-            let field_op = ecx.operand_field(&down, i).ok()?;
+            let field_op = ecx.project_field(&down, i).ok()?;
             let val = op_to_const(&ecx, &field_op);
             Some((val, field_op.layout.ty))
         })

compiler/rustc_const_eval/src/const_eval/valtrees.rs

@@ -2,11 +2,11 @@ use super::eval_queries::{mk_eval_cx, op_to_const};
 use super::machine::CompileTimeEvalContext;
 use super::{ValTreeCreationError, ValTreeCreationResult, VALTREE_MAX_NODES};
 use crate::const_eval::CanAccessStatics;
+use crate::interpret::MPlaceTy;
 use crate::interpret::{
     intern_const_alloc_recursive, ConstValue, ImmTy, Immediate, InternKind, MemPlaceMeta,
-    MemoryKind, PlaceTy, Scalar,
+    MemoryKind, PlaceTy, Projectable, Scalar,
 };
-use crate::interpret::{MPlaceTy, Value};
 use rustc_middle::ty::{self, ScalarInt, Ty, TyCtxt};
 use rustc_span::source_map::DUMMY_SP;
 use rustc_target::abi::{Align, FieldIdx, VariantIdx, FIRST_VARIANT};
@@ -20,15 +20,15 @@ fn branches<'tcx>(
     num_nodes: &mut usize,
 ) -> ValTreeCreationResult<'tcx> {
     let place = match variant {
-        Some(variant) => ecx.mplace_downcast(&place, variant).unwrap(),
+        Some(variant) => ecx.project_downcast(place, variant).unwrap(),
         None => *place,
     };
     let variant = variant.map(|variant| Some(ty::ValTree::Leaf(ScalarInt::from(variant.as_u32()))));
     debug!(?place, ?variant);
 
     let mut fields = Vec::with_capacity(n);
     for i in 0..n {
-        let field = ecx.mplace_field(&place, i).unwrap();
+        let field = ecx.project_field(&place, i).unwrap();
         let valtree = const_to_valtree_inner(ecx, &field, num_nodes)?;
         fields.push(Some(valtree));
     }
@@ -55,13 +55,11 @@ fn slice_branches<'tcx>(
     place: &MPlaceTy<'tcx>,
     num_nodes: &mut usize,
 ) -> ValTreeCreationResult<'tcx> {
-    let n = place
-        .len(&ecx.tcx.tcx)
-        .unwrap_or_else(|_| panic!("expected to use len of place {:?}", place));
+    let n = place.len(ecx).unwrap_or_else(|_| panic!("expected to use len of place {:?}", place));
 
     let mut elems = Vec::with_capacity(n as usize);
     for i in 0..n {
-        let place_elem = ecx.mplace_index(place, i).unwrap();
+        let place_elem = ecx.project_index(place, i).unwrap();
         let valtree = const_to_valtree_inner(ecx, &place_elem, num_nodes)?;
         elems.push(valtree);
     }
@@ -386,7 +384,7 @@ fn valtree_into_mplace<'tcx>(
                     debug!(?variant);
 
                     (
-                        place.project_downcast(ecx, variant_idx).unwrap(),
+                        ecx.project_downcast(place, variant_idx).unwrap(),
                         &branches[1..],
                         Some(variant_idx),
                     )
@@ -401,7 +399,7 @@ fn valtree_into_mplace<'tcx>(
                 debug!(?i, ?inner_valtree);
 
                 let mut place_inner = match ty.kind() {
-                    ty::Str | ty::Slice(_) => ecx.mplace_index(&place, i as u64).unwrap(),
+                    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 =>
                     {
@@ -441,7 +439,7 @@ fn valtree_into_mplace<'tcx>(
                             )
                             .unwrap()
                     }
-                    _ => ecx.mplace_field(&place_adjusted, i).unwrap(),
+                    _ => ecx.project_field(&place_adjusted, i).unwrap(),
                 };
 
                 debug!(?place_inner);

compiler/rustc_const_eval/src/errors.rs

@@ -511,7 +511,8 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
             InvalidUninitBytes(Some(_)) => const_eval_invalid_uninit_bytes,
             DeadLocal => const_eval_dead_local,
             ScalarSizeMismatch(_) => const_eval_scalar_size_mismatch,
-            UninhabitedEnumVariantWritten => const_eval_uninhabited_enum_variant_written,
+            UninhabitedEnumVariantWritten(_) => const_eval_uninhabited_enum_variant_written,
+            UninhabitedEnumVariantRead(_) => const_eval_uninhabited_enum_variant_read,
             Validation(e) => e.diagnostic_message(),
             Custom(x) => (x.msg)(),
         }
@@ -535,7 +536,8 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
             | InvalidMeta(InvalidMetaKind::TooBig)
             | InvalidUninitBytes(None)
             | DeadLocal
-            | UninhabitedEnumVariantWritten => {}
+            | UninhabitedEnumVariantWritten(_)
+            | UninhabitedEnumVariantRead(_) => {}
             BoundsCheckFailed { len, index } => {
                 builder.set_arg("len", len);
                 builder.set_arg("index", index);
@@ -623,6 +625,7 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
             UnsafeCell => const_eval_unsafe_cell,
             UninhabitedVal { .. } => const_eval_uninhabited_val,
             InvalidEnumTag { .. } => const_eval_invalid_enum_tag,
+            UninhabitedEnumTag => const_eval_uninhabited_enum_tag,
             UninitEnumTag => const_eval_uninit_enum_tag,
             UninitStr => const_eval_uninit_str,
             Uninit { expected: ExpectedKind::Bool } => const_eval_uninit_bool,
@@ -760,7 +763,8 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
             | InvalidMetaSliceTooLarge { .. }
             | InvalidMetaTooLarge { .. }
             | DanglingPtrUseAfterFree { .. }
-            | DanglingPtrOutOfBounds { .. } => {}
+            | DanglingPtrOutOfBounds { .. }
+            | UninhabitedEnumTag => {}
         }
     }
 }
@@ -835,7 +839,9 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> {
                 rustc_middle::error::middle_adjust_for_foreign_abi_error
             }
             InvalidProgramInfo::SizeOfUnsizedType(_) => const_eval_size_of_unsized,
-            InvalidProgramInfo::ConstPropNonsense => panic!("We had const-prop nonsense, this should never be printed"),
+            InvalidProgramInfo::ConstPropNonsense => {
+                panic!("We had const-prop nonsense, this should never be printed")
+            }
         }
     }
     fn add_args<G: EmissionGuarantee>(

compiler/rustc_const_eval/src/interpret/cast.rs

@@ -420,8 +420,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     if cast_ty_field.is_zst() {
                         continue;
                     }
-                    let src_field = self.operand_field(src, i)?;
-                    let dst_field = self.place_field(dest, i)?;
+                    let src_field = self.project_field(src, i)?;
+                    let dst_field = self.project_field(dest, i)?;
                     if src_field.layout.ty == cast_ty_field.ty {
                         self.copy_op(&src_field, &dst_field, /*allow_transmute*/ false)?;
                     } else {

compiler/rustc_const_eval/src/interpret/discriminant.rs

@@ -22,7 +22,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // 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() {
-            throw_ub!(UninhabitedEnumVariantWritten)
+            throw_ub!(UninhabitedEnumVariantWritten(variant_index))
         }
 
         match dest.layout.variants {
@@ -47,7 +47,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 let size = tag_layout.size(self);
                 let tag_val = size.truncate(discr_val);
 
-                let tag_dest = self.place_field(dest, tag_field)?;
+                let tag_dest = self.project_field(dest, tag_field)?;
                 self.write_scalar(Scalar::from_uint(tag_val, size), &tag_dest)?;
             }
             abi::Variants::Multiple {
@@ -78,7 +78,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         &niche_start_val,
                     )?;
                     // Write result.
-                    let niche_dest = self.place_field(dest, tag_field)?;
+                    let niche_dest = self.project_field(dest, tag_field)?;
                     self.write_immediate(*tag_val, &niche_dest)?;
                 }
             }
@@ -106,6 +106,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // straight-forward (`TagEncoding::Direct`) or with a niche (`TagEncoding::Niche`).
         let (tag_scalar_layout, tag_encoding, tag_field) = match op.layout.variants {
             Variants::Single { index } => {
+                // 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()) {
+                    throw_ub!(UninhabitedEnumVariantRead(index))
+                }
                 let discr = match op.layout.ty.discriminant_for_variant(*self.tcx, index) {
                     Some(discr) => {
                         // This type actually has discriminants.
@@ -118,6 +123,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         Scalar::from_uint(index.as_u32(), discr_layout.size)
                     }
                 };
+                // 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.ty.is_enum() && op.layout.for_variant(self, index).abi.is_uninhabited() {
+                    throw_ub!(UninhabitedEnumVariantRead(index))
+                }
                 return Ok((discr, index));
             }
             Variants::Multiple { tag, ref tag_encoding, tag_field, .. } => {
@@ -138,13 +149,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let tag_layout = self.layout_of(tag_scalar_layout.primitive().to_int_ty(*self.tcx))?;
 
         // Read tag and sanity-check `tag_layout`.
-        let tag_val = self.read_immediate(&self.operand_field(op, tag_field)?)?;
+        let tag_val = self.read_immediate(&self.project_field(op, tag_field)?)?;
         assert_eq!(tag_layout.size, tag_val.layout.size);
         assert_eq!(tag_layout.abi.is_signed(), tag_val.layout.abi.is_signed());
         trace!("tag value: {}", tag_val);
 
         // Figure out which discriminant and variant this corresponds to.
-        Ok(match *tag_encoding {
+        let (discr, index) = match *tag_encoding {
             TagEncoding::Direct => {
                 let scalar = tag_val.to_scalar();
                 // Generate a specific error if `tag_val` is not an integer.
@@ -232,6 +243,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 // encoded in the tag.
                 (Scalar::from_uint(variant.as_u32(), discr_layout.size), variant)
             }
-        })
+        };
+        // 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() {
+            throw_ub!(UninhabitedEnumVariantRead(index))
+        }
+        Ok((discr, index))
     }
 }

compiler/rustc_const_eval/src/interpret/intern.rs

@@ -164,75 +164,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
         &self.ecx
     }
 
-    fn visit_aggregate(
-        &mut self,
-        mplace: &MPlaceTy<'tcx>,
-        fields: impl Iterator<Item = InterpResult<'tcx, Self::V>>,
-    ) -> InterpResult<'tcx> {
-        // We want to walk the aggregate to look for references to intern. While doing that we
-        // also need to take special care of interior mutability.
-        //
-        // As an optimization, however, if the allocation does not contain any references: we don't
-        // need to do the walk. It can be costly for big arrays for example (e.g. issue #93215).
-        let is_walk_needed = |mplace: &MPlaceTy<'tcx>| -> InterpResult<'tcx, bool> {
-            // ZSTs cannot contain pointers, we can avoid the interning walk.
-            if mplace.layout.is_zst() {
-                return Ok(false);
-            }
-
-            // Now, check whether this allocation could contain references.
-            //
-            // Note, this check may sometimes not be cheap, so we only do it when the walk we'd like
-            // to avoid could be expensive: on the potentially larger types, arrays and slices,
-            // rather than on all aggregates unconditionally.
-            if matches!(mplace.layout.ty.kind(), ty::Array(..) | ty::Slice(..)) {
-                let Some((size, align)) = self.ecx.size_and_align_of_mplace(&mplace)? else {
-                    // We do the walk if we can't determine the size of the mplace: we may be
-                    // dealing with extern types here in the future.
-                    return Ok(true);
-                };
-
-                // If there is no provenance in this allocation, it does not contain references
-                // that point to another allocation, and we can avoid the interning walk.
-                if let Some(alloc) = self.ecx.get_ptr_alloc(mplace.ptr, size, align)? {
-                    if !alloc.has_provenance() {
-                        return Ok(false);
-                    }
-                } else {
-                    // We're encountering a ZST here, and can avoid the walk as well.
-                    return Ok(false);
-                }
-            }
-
-            // In the general case, we do the walk.
-            Ok(true)
-        };
-
-        // If this allocation contains no references to intern, we avoid the potentially costly
-        // walk.
-        //
-        // We can do this before the checks for interior mutability below, because only references
-        // are relevant in that situation, and we're checking if there are any here.
-        if !is_walk_needed(mplace)? {
-            return Ok(());
-        }
-
-        if let Some(def) = mplace.layout.ty.ty_adt_def() {
-            if def.is_unsafe_cell() {
-                // We are crossing over an `UnsafeCell`, we can mutate again. This means that
-                // References we encounter inside here are interned as pointing to mutable
-                // allocations.
-                // Remember the `old` value to handle nested `UnsafeCell`.
-                let old = std::mem::replace(&mut self.inside_unsafe_cell, true);
-                let walked = self.walk_aggregate(mplace, fields);
-                self.inside_unsafe_cell = old;
-                return walked;
-            }
-        }
-
-        self.walk_aggregate(mplace, fields)
-    }
-
     fn visit_value(&mut self, mplace: &MPlaceTy<'tcx>) -> InterpResult<'tcx> {
         // Handle Reference types, as these are the only types with provenance supported by const eval.
         // Raw pointers (and boxes) are handled by the `leftover_allocations` logic.
@@ -315,7 +246,63 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
             }
             Ok(())
         } else {
-            // Not a reference -- proceed recursively.
+            // Not a reference. Check if we want to recurse.
+            let is_walk_needed = |mplace: &MPlaceTy<'tcx>| -> InterpResult<'tcx, bool> {
+                // ZSTs cannot contain pointers, we can avoid the interning walk.
+                if mplace.layout.is_zst() {
+                    return Ok(false);
+                }
+
+                // Now, check whether this allocation could contain references.
+                //
+                // Note, this check may sometimes not be cheap, so we only do it when the walk we'd like
+                // to avoid could be expensive: on the potentially larger types, arrays and slices,
+                // rather than on all aggregates unconditionally.
+                if matches!(mplace.layout.ty.kind(), ty::Array(..) | ty::Slice(..)) {
+                    let Some((size, align)) = self.ecx.size_and_align_of_mplace(&mplace)? else {
+                        // We do the walk if we can't determine the size of the mplace: we may be
+                        // dealing with extern types here in the future.
+                        return Ok(true);
+                    };
+
+                    // If there is no provenance in this allocation, it does not contain references
+                    // that point to another allocation, and we can avoid the interning walk.
+                    if let Some(alloc) = self.ecx.get_ptr_alloc(mplace.ptr, size, align)? {
+                        if !alloc.has_provenance() {
+                            return Ok(false);
+                        }
+                    } else {
+                        // We're encountering a ZST here, and can avoid the walk as well.
+                        return Ok(false);
+                    }
+                }
+
+                // In the general case, we do the walk.
+                Ok(true)
+            };
+
+            // If this allocation contains no references to intern, we avoid the potentially costly
+            // walk.
+            //
+            // We can do this before the checks for interior mutability below, because only references
+            // are relevant in that situation, and we're checking if there are any here.
+            if !is_walk_needed(mplace)? {
+                return Ok(());
+            }
+
+            if let Some(def) = mplace.layout.ty.ty_adt_def() {
+                if def.is_unsafe_cell() {
+                    // We are crossing over an `UnsafeCell`, we can mutate again. This means that
+                    // References we encounter inside here are interned as pointing to mutable
+                    // allocations.
+                    // Remember the `old` value to handle nested `UnsafeCell`.
+                    let old = std::mem::replace(&mut self.inside_unsafe_cell, true);
+                    let walked = self.walk_value(mplace);
+                    self.inside_unsafe_cell = old;
+                    return walked;
+                }
+            }
+
             self.walk_value(mplace)
         }
     }