Skip to content

Commit 1b86d85

Browse files
RalfJungRalfJung
committed
Variants::Single: do not use invalid VariantIdx for uninhabited enums
1 parent 3dc9b1b commit 1b86d85

File tree

24 files changed

+168
-152
lines changed

24 files changed

+168
-152
lines changed

Diff for: ‎compiler/rustc_abi/src/layout.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
120120
.max_by_key(|niche| niche.available(dl));
121121

122122
LayoutData {
123-
variants: Variants::Single { index: VariantIdx::new(0) },
123+
variants: Variants::Single { index: Some(VariantIdx::new(0)) },
124124
fields: FieldsShape::Arbitrary {
125125
offsets: [Size::ZERO, b_offset].into(),
126126
memory_index: [0, 1].into(),
@@ -214,7 +214,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
214214
) -> LayoutData<FieldIdx, VariantIdx> {
215215
let dl = self.cx.data_layout();
216216
LayoutData {
217-
variants: Variants::Single { index: VariantIdx::new(0) },
217+
variants: Variants::Single { index: None },
218218
fields: FieldsShape::Primitive,
219219
backend_repr: BackendRepr::Uninhabited,
220220
largest_niche: None,
@@ -385,7 +385,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
385385
};
386386

387387
Ok(LayoutData {
388-
variants: Variants::Single { index: only_variant_idx },
388+
variants: Variants::Single { index: Some(only_variant_idx) },
389389
fields: FieldsShape::Union(union_field_count),
390390
backend_repr: abi,
391391
largest_niche: None,
@@ -424,7 +424,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
424424
};
425425

426426
let mut st = self.univariant(&variants[v], repr, kind)?;
427-
st.variants = Variants::Single { index: v };
427+
st.variants = Variants::Single { index: Some(v) };
428428

429429
if is_unsafe_cell {
430430
let hide_niches = |scalar: &mut _| match scalar {
@@ -543,7 +543,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
543543
.iter_enumerated()
544544
.map(|(j, v)| {
545545
let mut st = self.univariant(v, repr, StructKind::AlwaysSized).ok()?;
546-
st.variants = Variants::Single { index: j };
546+
st.variants = Variants::Single { index: Some(j) };
547547

548548
align = align.max(st.align);
549549
max_repr_align = max_repr_align.max(st.max_repr_align);
@@ -736,7 +736,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
736736
repr,
737737
StructKind::Prefixed(min_ity.size(), prefix_align),
738738
)?;
739-
st.variants = Variants::Single { index: i };
739+
st.variants = Variants::Single { index: Some(i) };
740740
// Find the first field we can't move later
741741
// to make room for a larger discriminant.
742742
for field_idx in st.fields.index_by_increasing_offset() {
@@ -1344,7 +1344,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
13441344
};
13451345

13461346
Ok(LayoutData {
1347-
variants: Variants::Single { index: VariantIdx::new(0) },
1347+
variants: Variants::Single { index: Some(VariantIdx::new(0)) },
13481348
fields: FieldsShape::Arbitrary { offsets, memory_index },
13491349
backend_repr: abi,
13501350
largest_niche,

Diff for: ‎compiler/rustc_abi/src/lib.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -1506,9 +1506,9 @@ impl BackendRepr {
15061506
pub enum Variants<FieldIdx: Idx, VariantIdx: Idx> {
15071507
/// Single enum variants, structs/tuples, unions, and all non-ADTs.
15081508
Single {
1509-
/// Always 0 for non-enums/generators.
1510-
/// For enums without a variant, this is an invalid index!
1511-
index: VariantIdx,
1509+
/// Always `Some(0)` for types without variants (i.e., everything except for `!`, enums, and
1510+
/// generators). `None` indicates an uninhabited type; this is used for zero-variant enums.
1511+
index: Option<VariantIdx>,
15121512
},
15131513

15141514
/// Enum-likes with more than one variant: each variant comes with
@@ -1706,7 +1706,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
17061706
let size = scalar.size(cx);
17071707
let align = scalar.align(cx);
17081708
LayoutData {
1709-
variants: Variants::Single { index: VariantIdx::new(0) },
1709+
variants: Variants::Single { index: Some(VariantIdx::new(0)) },
17101710
fields: FieldsShape::Primitive,
17111711
backend_repr: BackendRepr::Scalar(scalar),
17121712
largest_niche,

Diff for: ‎compiler/rustc_codegen_cranelift/src/discriminant.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub(crate) fn codegen_set_discriminant<'tcx>(
1919
}
2020
match layout.variants {
2121
Variants::Single { index } => {
22-
assert_eq!(index, variant_index);
22+
assert_eq!(index.unwrap(), variant_index);
2323
}
2424
Variants::Multiple {
2525
tag: _,
@@ -86,9 +86,10 @@ pub(crate) fn codegen_get_discriminant<'tcx>(
8686

8787
let (tag_scalar, tag_field, tag_encoding) = match &layout.variants {
8888
Variants::Single { index } => {
89+
let index = index.unwrap();
8990
let discr_val = layout
9091
.ty
91-
.discriminant_for_variant(fx.tcx, *index)
92+
.discriminant_for_variant(fx.tcx, index)
9293
.map_or(u128::from(index.as_u32()), |discr| discr.val);
9394

9495
let val = match dest_layout.ty.kind() {

Diff for: ‎compiler/rustc_codegen_gcc/src/type_of.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -99,14 +99,14 @@ fn uncached_gcc_type<'gcc, 'tcx>(
9999
if !cx.sess().fewer_names() =>
100100
{
101101
let mut name = with_no_trimmed_paths!(layout.ty.to_string());
102-
if let (&ty::Adt(def, _), &Variants::Single { index }) =
102+
if let (&ty::Adt(def, _), &Variants::Single { index: Some(index) }) =
103103
(layout.ty.kind(), &layout.variants)
104104
{
105105
if def.is_enum() && !def.variants().is_empty() {
106106
write!(&mut name, "::{}", def.variant(index).name).unwrap();
107107
}
108108
}
109-
if let (&ty::Coroutine(_, _), &Variants::Single { index }) =
109+
if let (&ty::Coroutine(_, _), &Variants::Single { index: Some(index) }) =
110110
(layout.ty.kind(), &layout.variants)
111111
{
112112
write!(&mut name, "::{}", ty::CoroutineArgs::variant_name(index)).unwrap();
@@ -230,7 +230,7 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {
230230

231231
// Check the cache.
232232
let variant_index = match self.variants {
233-
Variants::Single { index } => Some(index),
233+
Variants::Single { index } => index,
234234
_ => None,
235235
};
236236
let cached_type = cx.types.borrow().get(&(self.ty, variant_index)).cloned();

Diff for: ‎compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -206,11 +206,11 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
206206
|cx, enum_type_di_node| {
207207
match enum_type_and_layout.variants {
208208
Variants::Single { index: variant_index } => {
209-
if enum_adt_def.variants().is_empty() {
209+
let Some(variant_index) = variant_index else {
210210
// Uninhabited enums have Variants::Single. We don't generate
211211
// any members for them.
212212
return smallvec![];
213-
}
213+
};
214214

215215
build_single_variant_union_fields(
216216
cx,

Diff for: ‎compiler/rustc_codegen_llvm/src/type_of.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,14 @@ fn uncached_llvm_type<'a, 'tcx>(
3535
if !cx.sess().fewer_names() =>
3636
{
3737
let mut name = with_no_visible_paths!(with_no_trimmed_paths!(layout.ty.to_string()));
38-
if let (&ty::Adt(def, _), &Variants::Single { index }) =
38+
if let (&ty::Adt(def, _), &Variants::Single { index: Some(index) }) =
3939
(layout.ty.kind(), &layout.variants)
4040
{
41-
if def.is_enum() && !def.variants().is_empty() {
41+
if def.is_enum() {
4242
write!(&mut name, "::{}", def.variant(index).name).unwrap();
4343
}
4444
}
45-
if let (&ty::Coroutine(_, _), &Variants::Single { index }) =
45+
if let (&ty::Coroutine(_, _), &Variants::Single { index: Some(index) }) =
4646
(layout.ty.kind(), &layout.variants)
4747
{
4848
write!(&mut name, "::{}", ty::CoroutineArgs::variant_name(index)).unwrap();
@@ -216,7 +216,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> {
216216

217217
// Check the cache.
218218
let variant_index = match self.variants {
219-
Variants::Single { index } => Some(index),
219+
Variants::Single { index } => index,
220220
_ => None,
221221
};
222222
if let Some(llty) = cx.type_lowering.borrow().get(&(self.ty, variant_index)) {

Diff for: ‎compiler/rustc_codegen_ssa/src/mir/place.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
243243
}
244244
let (tag_scalar, tag_encoding, tag_field) = match self.layout.variants {
245245
Variants::Single { index } => {
246+
let index = index.unwrap(); // we already checked `is_uninhabited`
246247
let discr_val = self
247248
.layout
248249
.ty
@@ -365,7 +366,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
365366
}
366367
match self.layout.variants {
367368
Variants::Single { index } => {
368-
assert_eq!(index, variant_index);
369+
assert_eq!(index.unwrap(), variant_index);
369370
}
370371
Variants::Multiple { tag_encoding: TagEncoding::Direct, tag_field, .. } => {
371372
let ptr = self.project_field(bx, tag_field);

Diff for: ‎compiler/rustc_const_eval/src/interpret/discriminant.rs

+10-13
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
4444
}
4545
}
4646

47-
/// Read discriminant, return the runtime value as well as the variant index.
47+
/// Read discriminant, return the variant index.
4848
/// Can also legally be called on non-enums (e.g. through the discriminant_value intrinsic)!
4949
///
5050
/// Will never return an uninhabited variant.
@@ -66,20 +66,15 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
6666
// straight-forward (`TagEncoding::Direct`) or with a niche (`TagEncoding::Niche`).
6767
let (tag_scalar_layout, tag_encoding, tag_field) = match op.layout().variants {
6868
Variants::Single { index } => {
69-
// Do some extra checks on enums.
70-
if ty.is_enum() {
71-
// Hilariously, `Single` is used even for 0-variant enums.
72-
// (See https://github.com/rust-lang/rust/issues/89765).
73-
if ty.ty_adt_def().unwrap().variants().is_empty() {
74-
throw_ub!(UninhabitedEnumVariantRead(index))
75-
}
69+
if op.layout().is_uninhabited() {
7670
// For consistency with `write_discriminant`, and to make sure that
7771
// `project_downcast` cannot fail due to strange layouts, we declare immediate UB
78-
// for uninhabited variants.
79-
if op.layout().for_variant(self, index).is_uninhabited() {
80-
throw_ub!(UninhabitedEnumVariantRead(index))
81-
}
72+
// for uninhabited enums.
73+
throw_ub!(UninhabitedEnumVariantRead(None));
8274
}
75+
// Sanity checks.
76+
let index = index.unwrap();
77+
assert!(!op.layout().for_variant(self, index).is_uninhabited());
8378
return interp_ok(index);
8479
}
8580
Variants::Multiple { tag, ref tag_encoding, tag_field, .. } => {
@@ -199,11 +194,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
199194
// `uninhabited_enum_branching` MIR pass. It also ensures consistency with
200195
// `write_discriminant`.
201196
if op.layout().for_variant(self, index).is_uninhabited() {
202-
throw_ub!(UninhabitedEnumVariantRead(index))
197+
throw_ub!(UninhabitedEnumVariantRead(Some(index)))
203198
}
204199
interp_ok(index)
205200
}
206201

202+
/// Read discriminant, return the user-visible discriminant.
203+
/// Can also legally be called on non-enums (e.g. through the discriminant_value intrinsic)!
207204
pub fn discriminant_for_variant(
208205
&self,
209206
ty: Ty<'tcx>,

Diff for: ‎compiler/rustc_const_eval/src/interpret/validity.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,9 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
342342
match layout.variants {
343343
Variants::Single { index } => {
344344
// Inside a variant
345-
PathElem::Field(def.variant(index).fields[FieldIdx::from_usize(field)].name)
345+
PathElem::Field(
346+
def.variant(index.unwrap()).fields[FieldIdx::from_usize(field)].name,
347+
)
346348
}
347349
Variants::Multiple { .. } => bug!("we handled variants above"),
348350
}

Diff for: ‎compiler/rustc_middle/src/mir/interpret/error.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ pub enum UndefinedBehaviorInfo<'tcx> {
399399
/// A discriminant of an uninhabited enum variant is written.
400400
UninhabitedEnumVariantWritten(VariantIdx),
401401
/// An uninhabited enum variant is projected.
402-
UninhabitedEnumVariantRead(VariantIdx),
402+
UninhabitedEnumVariantRead(Option<VariantIdx>),
403403
/// Trying to set discriminant to the niched variant, but the value does not match.
404404
InvalidNichedEnumVariantWritten { enum_ty: Ty<'tcx> },
405405
/// ABI-incompatible argument types.

Diff for: ‎compiler/rustc_middle/src/ty/layout.rs

+8-5
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,7 @@ where
732732
let layout = match this.variants {
733733
Variants::Single { index }
734734
// If all variants but one are uninhabited, the variant layout is the enum layout.
735-
if index == variant_index &&
735+
if index == Some(variant_index) &&
736736
// Don't confuse variants of uninhabited enums with the enum itself.
737737
// For more details see https://github.com/rust-lang/rust/issues/69763.
738738
this.fields != FieldsShape::Primitive =>
@@ -741,6 +741,8 @@ where
741741
}
742742

743743
Variants::Single { index } => {
744+
// `Single` variant enums *can* have other variants, but those are uninhabited.
745+
744746
let tcx = cx.tcx();
745747
let typing_env = cx.typing_env();
746748

@@ -756,7 +758,7 @@ where
756758
_ => bug!("`ty_and_layout_for_variant` on unexpected type {}", this.ty),
757759
};
758760
tcx.mk_layout(LayoutData {
759-
variants: Variants::Single { index: variant_index },
761+
variants: Variants::Single { index: Some(variant_index) },
760762
fields: match NonZero::new(fields) {
761763
Some(fields) => FieldsShape::Union(fields),
762764
None => FieldsShape::Arbitrary { offsets: IndexVec::new(), memory_index: IndexVec::new() },
@@ -773,7 +775,7 @@ where
773775
Variants::Multiple { ref variants, .. } => cx.tcx().mk_layout(variants[variant_index].clone()),
774776
};
775777

776-
assert_eq!(*layout.variants(), Variants::Single { index: variant_index });
778+
assert_eq!(*layout.variants(), Variants::Single { index: Some(variant_index) });
777779

778780
TyAndLayout { ty: this.ty, layout }
779781
}
@@ -903,7 +905,7 @@ where
903905
Variants::Single { index } => TyMaybeWithLayout::Ty(
904906
args.as_coroutine()
905907
.state_tys(def_id, tcx)
906-
.nth(index.as_usize())
908+
.nth(index.unwrap().as_usize())
907909
.unwrap()
908910
.nth(i)
909911
.unwrap(),
@@ -922,7 +924,8 @@ where
922924
ty::Adt(def, args) => {
923925
match this.variants {
924926
Variants::Single { index } => {
925-
let field = &def.variant(index).fields[FieldIdx::from_usize(i)];
927+
let field =
928+
&def.variant(index.unwrap()).fields[FieldIdx::from_usize(i)];
926929
TyMaybeWithLayout::Ty(field.ty(tcx, args))
927930
}
928931

Diff for: ‎compiler/rustc_mir_transform/src/jump_threading.rs

+7-24
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
//! Likewise, applying the optimisation can create a lot of new MIR, so we bound the instruction
3636
//! cost by `MAX_COST`.
3737
38-
use rustc_abi::{TagEncoding, Variants};
3938
use rustc_arena::DroplessArena;
4039
use rustc_const_eval::const_eval::DummyMachine;
4140
use rustc_const_eval::interpret::{ImmTy, Immediate, InterpCx, OpTy, Projectable};
@@ -565,31 +564,15 @@ impl<'a, 'tcx> TOFinder<'a, 'tcx> {
565564
StatementKind::SetDiscriminant { box place, variant_index } => {
566565
let Some(discr_target) = self.map.find_discr(place.as_ref()) else { return };
567566
let enum_ty = place.ty(self.body, self.tcx).ty;
568-
// `SetDiscriminant` may be a no-op if the assigned variant is the untagged variant
569-
// of a niche encoding. If we cannot ensure that we write to the discriminant, do
570-
// nothing.
571-
let Ok(enum_layout) = self.ecx.layout_of(enum_ty) else {
567+
// `SetDiscriminant` guarantees that the discriminant is now `variant_index`.
568+
// Even if the discriminant write does nothing due to niches, it is UB to set the
569+
// discriminant when the data does not encode the desired discriminant.
570+
let Some(discr) =
571+
self.ecx.discriminant_for_variant(enum_ty, *variant_index).discard_err()
572+
else {
572573
return;
573574
};
574-
let writes_discriminant = match enum_layout.variants {
575-
Variants::Single { index } => {
576-
assert_eq!(index, *variant_index);
577-
true
578-
}
579-
Variants::Multiple { tag_encoding: TagEncoding::Direct, .. } => true,
580-
Variants::Multiple {
581-
tag_encoding: TagEncoding::Niche { untagged_variant, .. },
582-
..
583-
} => *variant_index != untagged_variant,
584-
};
585-
if writes_discriminant {
586-
let Some(discr) =
587-
self.ecx.discriminant_for_variant(enum_ty, *variant_index).discard_err()
588-
else {
589-
return;
590-
};
591-
self.process_immediate(bb, discr_target, discr, state);
592-
}
575+
self.process_immediate(bb, discr_target, discr, state);
593576
}
594577
// If we expect `lhs ?= true`, we have an opportunity if we assume `lhs == true`.
595578
StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(

Diff for: ‎compiler/rustc_mir_transform/src/unreachable_enum_branching.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,11 @@ fn variant_discriminants<'tcx>(
5454
tcx: TyCtxt<'tcx>,
5555
) -> FxHashSet<u128> {
5656
match &layout.variants {
57-
Variants::Single { index } => {
57+
Variants::Single { index: None } => {
58+
// Uninhabited, no valid discriminant.
59+
FxHashSet::default()
60+
}
61+
Variants::Single { index: Some(index) } => {
5862
let mut res = FxHashSet::default();
5963
res.insert(
6064
ty.discriminant_for_variant(tcx, *index)

Diff for: ‎compiler/rustc_smir/src/rustc_smir/convert/abi.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,9 @@ impl<'tcx> Stable<'tcx> for rustc_abi::Variants<rustc_abi::FieldIdx, rustc_abi::
164164

165165
fn stable(&self, tables: &mut Tables<'_>) -> Self::T {
166166
match self {
167-
rustc_abi::Variants::Single { index } => {
168-
VariantsShape::Single { index: index.stable(tables) }
169-
}
167+
rustc_abi::Variants::Single { index } => VariantsShape::Single {
168+
index: index.unwrap_or(rustc_abi::VariantIdx::from_u32(0)).stable(tables),
169+
},
170170
rustc_abi::Variants::Multiple { tag, tag_encoding, tag_field, variants } => {
171171
VariantsShape::Multiple {
172172
tag: tag.stable(tables),

0 commit comments

Comments
 (0)