Skip to content

Commit a2bcafa

Browse files
committed
interpret: refactor projection code to work on a common trait, and use that for visitors
1 parent a593de4 commit a2bcafa

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+871
-1218
lines changed

compiler/rustc_abi/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1189,7 +1189,7 @@ impl FieldsShape {
11891189
}
11901190
FieldsShape::Array { stride, count } => {
11911191
let i = u64::try_from(i).unwrap();
1192-
assert!(i < count);
1192+
assert!(i < count, "tried to access field {} of array with {} fields", i, count);
11931193
stride * i
11941194
}
11951195
FieldsShape::Arbitrary { ref offsets, .. } => offsets[FieldIdx::from_usize(i)],

compiler/rustc_const_eval/messages.ftl

+4-1
Original file line numberDiff line numberDiff line change
@@ -408,8 +408,11 @@ const_eval_undefined_behavior =
408408
const_eval_undefined_behavior_note =
409409
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.
410410
411+
const_eval_uninhabited_enum_tag = {$front_matter}: encountered an uninhabited enum variant
412+
const_eval_uninhabited_enum_variant_read =
413+
read discriminant of an uninhabited enum variant
411414
const_eval_uninhabited_enum_variant_written =
412-
writing discriminant of an uninhabited enum
415+
writing discriminant of an uninhabited enum variant
413416
const_eval_uninhabited_val = {$front_matter}: encountered a value of uninhabited type `{$ty}`
414417
const_eval_uninit = {$front_matter}: encountered uninitialized bytes
415418
const_eval_uninit_bool = {$front_matter}: encountered uninitialized memory, but expected a boolean

compiler/rustc_const_eval/src/const_eval/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ pub(crate) fn try_destructure_mir_constant_for_diagnostics<'tcx>(
102102
}
103103
ty::Adt(def, _) => {
104104
let variant = ecx.read_discriminant(&op).ok()?.1;
105-
let down = ecx.operand_downcast(&op, variant).ok()?;
105+
let down = ecx.project_downcast(&op, variant).ok()?;
106106
(def.variants()[variant].fields.len(), Some(variant), down)
107107
}
108108
ty::Tuple(args) => (args.len(), None, op),
@@ -111,7 +111,7 @@ pub(crate) fn try_destructure_mir_constant_for_diagnostics<'tcx>(
111111

112112
let fields_iter = (0..field_count)
113113
.map(|i| {
114-
let field_op = ecx.operand_field(&down, i).ok()?;
114+
let field_op = ecx.project_field(&down, i).ok()?;
115115
let val = op_to_const(&ecx, &field_op);
116116
Some((val, field_op.layout.ty))
117117
})

compiler/rustc_const_eval/src/const_eval/valtrees.rs

+9-11
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ use super::eval_queries::{mk_eval_cx, op_to_const};
22
use super::machine::CompileTimeEvalContext;
33
use super::{ValTreeCreationError, ValTreeCreationResult, VALTREE_MAX_NODES};
44
use crate::const_eval::CanAccessStatics;
5+
use crate::interpret::MPlaceTy;
56
use crate::interpret::{
67
intern_const_alloc_recursive, ConstValue, ImmTy, Immediate, InternKind, MemPlaceMeta,
7-
MemoryKind, PlaceTy, Scalar,
8+
MemoryKind, PlaceTy, Projectable, Scalar,
89
};
9-
use crate::interpret::{MPlaceTy, Value};
1010
use rustc_middle::ty::{self, ScalarInt, Ty, TyCtxt};
1111
use rustc_span::source_map::DUMMY_SP;
1212
use rustc_target::abi::{Align, FieldIdx, VariantIdx, FIRST_VARIANT};
@@ -20,15 +20,15 @@ fn branches<'tcx>(
2020
num_nodes: &mut usize,
2121
) -> ValTreeCreationResult<'tcx> {
2222
let place = match variant {
23-
Some(variant) => ecx.mplace_downcast(&place, variant).unwrap(),
23+
Some(variant) => ecx.project_downcast(place, variant).unwrap(),
2424
None => *place,
2525
};
2626
let variant = variant.map(|variant| Some(ty::ValTree::Leaf(ScalarInt::from(variant.as_u32()))));
2727
debug!(?place, ?variant);
2828

2929
let mut fields = Vec::with_capacity(n);
3030
for i in 0..n {
31-
let field = ecx.mplace_field(&place, i).unwrap();
31+
let field = ecx.project_field(&place, i).unwrap();
3232
let valtree = const_to_valtree_inner(ecx, &field, num_nodes)?;
3333
fields.push(Some(valtree));
3434
}
@@ -55,13 +55,11 @@ fn slice_branches<'tcx>(
5555
place: &MPlaceTy<'tcx>,
5656
num_nodes: &mut usize,
5757
) -> ValTreeCreationResult<'tcx> {
58-
let n = place
59-
.len(&ecx.tcx.tcx)
60-
.unwrap_or_else(|_| panic!("expected to use len of place {:?}", place));
58+
let n = place.len(ecx).unwrap_or_else(|_| panic!("expected to use len of place {:?}", place));
6159

6260
let mut elems = Vec::with_capacity(n as usize);
6361
for i in 0..n {
64-
let place_elem = ecx.mplace_index(place, i).unwrap();
62+
let place_elem = ecx.project_index(place, i).unwrap();
6563
let valtree = const_to_valtree_inner(ecx, &place_elem, num_nodes)?;
6664
elems.push(valtree);
6765
}
@@ -386,7 +384,7 @@ fn valtree_into_mplace<'tcx>(
386384
debug!(?variant);
387385

388386
(
389-
place.project_downcast(ecx, variant_idx).unwrap(),
387+
ecx.project_downcast(place, variant_idx).unwrap(),
390388
&branches[1..],
391389
Some(variant_idx),
392390
)
@@ -401,7 +399,7 @@ fn valtree_into_mplace<'tcx>(
401399
debug!(?i, ?inner_valtree);
402400

403401
let mut place_inner = match ty.kind() {
404-
ty::Str | ty::Slice(_) => ecx.mplace_index(&place, i as u64).unwrap(),
402+
ty::Str | ty::Slice(_) => ecx.project_index(place, i as u64).unwrap(),
405403
_ if !ty.is_sized(*ecx.tcx, ty::ParamEnv::empty())
406404
&& i == branches.len() - 1 =>
407405
{
@@ -441,7 +439,7 @@ fn valtree_into_mplace<'tcx>(
441439
)
442440
.unwrap()
443441
}
444-
_ => ecx.mplace_field(&place_adjusted, i).unwrap(),
442+
_ => ecx.project_field(&place_adjusted, i).unwrap(),
445443
};
446444

447445
debug!(?place_inner);

compiler/rustc_const_eval/src/errors.rs

+10-4
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,8 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
511511
InvalidUninitBytes(Some(_)) => const_eval_invalid_uninit_bytes,
512512
DeadLocal => const_eval_dead_local,
513513
ScalarSizeMismatch(_) => const_eval_scalar_size_mismatch,
514-
UninhabitedEnumVariantWritten => const_eval_uninhabited_enum_variant_written,
514+
UninhabitedEnumVariantWritten(_) => const_eval_uninhabited_enum_variant_written,
515+
UninhabitedEnumVariantRead(_) => const_eval_uninhabited_enum_variant_read,
515516
Validation(e) => e.diagnostic_message(),
516517
Custom(x) => (x.msg)(),
517518
}
@@ -535,7 +536,8 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
535536
| InvalidMeta(InvalidMetaKind::TooBig)
536537
| InvalidUninitBytes(None)
537538
| DeadLocal
538-
| UninhabitedEnumVariantWritten => {}
539+
| UninhabitedEnumVariantWritten(_)
540+
| UninhabitedEnumVariantRead(_) => {}
539541
BoundsCheckFailed { len, index } => {
540542
builder.set_arg("len", len);
541543
builder.set_arg("index", index);
@@ -623,6 +625,7 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
623625
UnsafeCell => const_eval_unsafe_cell,
624626
UninhabitedVal { .. } => const_eval_uninhabited_val,
625627
InvalidEnumTag { .. } => const_eval_invalid_enum_tag,
628+
UninhabitedEnumTag => const_eval_uninhabited_enum_tag,
626629
UninitEnumTag => const_eval_uninit_enum_tag,
627630
UninitStr => const_eval_uninit_str,
628631
Uninit { expected: ExpectedKind::Bool } => const_eval_uninit_bool,
@@ -760,7 +763,8 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
760763
| InvalidMetaSliceTooLarge { .. }
761764
| InvalidMetaTooLarge { .. }
762765
| DanglingPtrUseAfterFree { .. }
763-
| DanglingPtrOutOfBounds { .. } => {}
766+
| DanglingPtrOutOfBounds { .. }
767+
| UninhabitedEnumTag => {}
764768
}
765769
}
766770
}
@@ -835,7 +839,9 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> {
835839
rustc_middle::error::middle_adjust_for_foreign_abi_error
836840
}
837841
InvalidProgramInfo::SizeOfUnsizedType(_) => const_eval_size_of_unsized,
838-
InvalidProgramInfo::ConstPropNonsense => panic!("We had const-prop nonsense, this should never be printed"),
842+
InvalidProgramInfo::ConstPropNonsense => {
843+
panic!("We had const-prop nonsense, this should never be printed")
844+
}
839845
}
840846
}
841847
fn add_args<G: EmissionGuarantee>(

compiler/rustc_const_eval/src/interpret/cast.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -420,8 +420,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
420420
if cast_ty_field.is_zst() {
421421
continue;
422422
}
423-
let src_field = self.operand_field(src, i)?;
424-
let dst_field = self.place_field(dest, i)?;
423+
let src_field = self.project_field(src, i)?;
424+
let dst_field = self.project_field(dest, i)?;
425425
if src_field.layout.ty == cast_ty_field.ty {
426426
self.copy_op(&src_field, &dst_field, /*allow_transmute*/ false)?;
427427
} else {

compiler/rustc_const_eval/src/interpret/discriminant.rs

+22-6
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
2222
// When evaluating we will always error before even getting here, but ConstProp 'executes'
2323
// dead code, so we cannot ICE here.
2424
if dest.layout.for_variant(self, variant_index).abi.is_uninhabited() {
25-
throw_ub!(UninhabitedEnumVariantWritten)
25+
throw_ub!(UninhabitedEnumVariantWritten(variant_index))
2626
}
2727

2828
match dest.layout.variants {
@@ -47,7 +47,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
4747
let size = tag_layout.size(self);
4848
let tag_val = size.truncate(discr_val);
4949

50-
let tag_dest = self.place_field(dest, tag_field)?;
50+
let tag_dest = self.project_field(dest, tag_field)?;
5151
self.write_scalar(Scalar::from_uint(tag_val, size), &tag_dest)?;
5252
}
5353
abi::Variants::Multiple {
@@ -78,7 +78,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
7878
&niche_start_val,
7979
)?;
8080
// Write result.
81-
let niche_dest = self.place_field(dest, tag_field)?;
81+
let niche_dest = self.project_field(dest, tag_field)?;
8282
self.write_immediate(*tag_val, &niche_dest)?;
8383
}
8484
}
@@ -106,6 +106,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
106106
// straight-forward (`TagEncoding::Direct`) or with a niche (`TagEncoding::Niche`).
107107
let (tag_scalar_layout, tag_encoding, tag_field) = match op.layout.variants {
108108
Variants::Single { index } => {
109+
// Hilariously, `Single` is used even for 0-variant enums.
110+
// (See https://github.com/rust-lang/rust/issues/89765).
111+
if matches!(op.layout.ty.kind(), ty::Adt(def, ..) if def.variants().is_empty()) {
112+
throw_ub!(UninhabitedEnumVariantRead(index))
113+
}
109114
let discr = match op.layout.ty.discriminant_for_variant(*self.tcx, index) {
110115
Some(discr) => {
111116
// This type actually has discriminants.
@@ -118,6 +123,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
118123
Scalar::from_uint(index.as_u32(), discr_layout.size)
119124
}
120125
};
126+
// For consisteny with `write_discriminant`, and to make sure that
127+
// `project_downcast` cannot fail due to strange layouts, we declare immediate UB
128+
// for uninhabited variants.
129+
if op.layout.ty.is_enum() && op.layout.for_variant(self, index).abi.is_uninhabited() {
130+
throw_ub!(UninhabitedEnumVariantRead(index))
131+
}
121132
return Ok((discr, index));
122133
}
123134
Variants::Multiple { tag, ref tag_encoding, tag_field, .. } => {
@@ -138,13 +149,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
138149
let tag_layout = self.layout_of(tag_scalar_layout.primitive().to_int_ty(*self.tcx))?;
139150

140151
// Read tag and sanity-check `tag_layout`.
141-
let tag_val = self.read_immediate(&self.operand_field(op, tag_field)?)?;
152+
let tag_val = self.read_immediate(&self.project_field(op, tag_field)?)?;
142153
assert_eq!(tag_layout.size, tag_val.layout.size);
143154
assert_eq!(tag_layout.abi.is_signed(), tag_val.layout.abi.is_signed());
144155
trace!("tag value: {}", tag_val);
145156

146157
// Figure out which discriminant and variant this corresponds to.
147-
Ok(match *tag_encoding {
158+
let (discr, index) = match *tag_encoding {
148159
TagEncoding::Direct => {
149160
let scalar = tag_val.to_scalar();
150161
// 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> {
232243
// encoded in the tag.
233244
(Scalar::from_uint(variant.as_u32(), discr_layout.size), variant)
234245
}
235-
})
246+
};
247+
// 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.
248+
if op.layout.for_variant(self, index).abi.is_uninhabited() {
249+
throw_ub!(UninhabitedEnumVariantRead(index))
250+
}
251+
Ok((discr, index))
236252
}
237253
}

compiler/rustc_const_eval/src/interpret/intern.rs

+57-70
Original file line numberDiff line numberDiff line change
@@ -164,75 +164,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
164164
&self.ecx
165165
}
166166

167-
fn visit_aggregate(
168-
&mut self,
169-
mplace: &MPlaceTy<'tcx>,
170-
fields: impl Iterator<Item = InterpResult<'tcx, Self::V>>,
171-
) -> InterpResult<'tcx> {
172-
// We want to walk the aggregate to look for references to intern. While doing that we
173-
// also need to take special care of interior mutability.
174-
//
175-
// As an optimization, however, if the allocation does not contain any references: we don't
176-
// need to do the walk. It can be costly for big arrays for example (e.g. issue #93215).
177-
let is_walk_needed = |mplace: &MPlaceTy<'tcx>| -> InterpResult<'tcx, bool> {
178-
// ZSTs cannot contain pointers, we can avoid the interning walk.
179-
if mplace.layout.is_zst() {
180-
return Ok(false);
181-
}
182-
183-
// Now, check whether this allocation could contain references.
184-
//
185-
// Note, this check may sometimes not be cheap, so we only do it when the walk we'd like
186-
// to avoid could be expensive: on the potentially larger types, arrays and slices,
187-
// rather than on all aggregates unconditionally.
188-
if matches!(mplace.layout.ty.kind(), ty::Array(..) | ty::Slice(..)) {
189-
let Some((size, align)) = self.ecx.size_and_align_of_mplace(&mplace)? else {
190-
// We do the walk if we can't determine the size of the mplace: we may be
191-
// dealing with extern types here in the future.
192-
return Ok(true);
193-
};
194-
195-
// If there is no provenance in this allocation, it does not contain references
196-
// that point to another allocation, and we can avoid the interning walk.
197-
if let Some(alloc) = self.ecx.get_ptr_alloc(mplace.ptr, size, align)? {
198-
if !alloc.has_provenance() {
199-
return Ok(false);
200-
}
201-
} else {
202-
// We're encountering a ZST here, and can avoid the walk as well.
203-
return Ok(false);
204-
}
205-
}
206-
207-
// In the general case, we do the walk.
208-
Ok(true)
209-
};
210-
211-
// If this allocation contains no references to intern, we avoid the potentially costly
212-
// walk.
213-
//
214-
// We can do this before the checks for interior mutability below, because only references
215-
// are relevant in that situation, and we're checking if there are any here.
216-
if !is_walk_needed(mplace)? {
217-
return Ok(());
218-
}
219-
220-
if let Some(def) = mplace.layout.ty.ty_adt_def() {
221-
if def.is_unsafe_cell() {
222-
// We are crossing over an `UnsafeCell`, we can mutate again. This means that
223-
// References we encounter inside here are interned as pointing to mutable
224-
// allocations.
225-
// Remember the `old` value to handle nested `UnsafeCell`.
226-
let old = std::mem::replace(&mut self.inside_unsafe_cell, true);
227-
let walked = self.walk_aggregate(mplace, fields);
228-
self.inside_unsafe_cell = old;
229-
return walked;
230-
}
231-
}
232-
233-
self.walk_aggregate(mplace, fields)
234-
}
235-
236167
fn visit_value(&mut self, mplace: &MPlaceTy<'tcx>) -> InterpResult<'tcx> {
237168
// Handle Reference types, as these are the only types with provenance supported by const eval.
238169
// 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
315246
}
316247
Ok(())
317248
} else {
318-
// Not a reference -- proceed recursively.
249+
// Not a reference. Check if we want to recurse.
250+
let is_walk_needed = |mplace: &MPlaceTy<'tcx>| -> InterpResult<'tcx, bool> {
251+
// ZSTs cannot contain pointers, we can avoid the interning walk.
252+
if mplace.layout.is_zst() {
253+
return Ok(false);
254+
}
255+
256+
// Now, check whether this allocation could contain references.
257+
//
258+
// Note, this check may sometimes not be cheap, so we only do it when the walk we'd like
259+
// to avoid could be expensive: on the potentially larger types, arrays and slices,
260+
// rather than on all aggregates unconditionally.
261+
if matches!(mplace.layout.ty.kind(), ty::Array(..) | ty::Slice(..)) {
262+
let Some((size, align)) = self.ecx.size_and_align_of_mplace(&mplace)? else {
263+
// We do the walk if we can't determine the size of the mplace: we may be
264+
// dealing with extern types here in the future.
265+
return Ok(true);
266+
};
267+
268+
// If there is no provenance in this allocation, it does not contain references
269+
// that point to another allocation, and we can avoid the interning walk.
270+
if let Some(alloc) = self.ecx.get_ptr_alloc(mplace.ptr, size, align)? {
271+
if !alloc.has_provenance() {
272+
return Ok(false);
273+
}
274+
} else {
275+
// We're encountering a ZST here, and can avoid the walk as well.
276+
return Ok(false);
277+
}
278+
}
279+
280+
// In the general case, we do the walk.
281+
Ok(true)
282+
};
283+
284+
// If this allocation contains no references to intern, we avoid the potentially costly
285+
// walk.
286+
//
287+
// We can do this before the checks for interior mutability below, because only references
288+
// are relevant in that situation, and we're checking if there are any here.
289+
if !is_walk_needed(mplace)? {
290+
return Ok(());
291+
}
292+
293+
if let Some(def) = mplace.layout.ty.ty_adt_def() {
294+
if def.is_unsafe_cell() {
295+
// We are crossing over an `UnsafeCell`, we can mutate again. This means that
296+
// References we encounter inside here are interned as pointing to mutable
297+
// allocations.
298+
// Remember the `old` value to handle nested `UnsafeCell`.
299+
let old = std::mem::replace(&mut self.inside_unsafe_cell, true);
300+
let walked = self.walk_value(mplace);
301+
self.inside_unsafe_cell = old;
302+
return walked;
303+
}
304+
}
305+
319306
self.walk_value(mplace)
320307
}
321308
}

0 commit comments

Comments
 (0)