Skip to content

Commit 2a43ce0

Browse files
authored
Rollup merge of #133702 - RalfJung:single-variant, r=oli-obk
Variants::Single: do not use invalid VariantIdx for uninhabited enums ~~Stacked on top of #133681, only the last commit is new.~~ Currently, `Variants::Single` for an empty enum contains a `VariantIdx` of 0; looking that up in the enum variant list will ICE. That's quite confusing. So let's fix that by adding a new `Variants::Empty` case for types that have 0 variants. try-job: i686-msvc
2 parents bab18a5 + 397ae3c commit 2a43ce0

File tree

33 files changed

+185
-164
lines changed

33 files changed

+185
-164
lines changed

Diff for: compiler/rustc_abi/src/callconv.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
206206
let (mut result, mut total) = from_fields_at(*self, Size::ZERO)?;
207207

208208
match &self.variants {
209-
abi::Variants::Single { .. } => {}
209+
abi::Variants::Single { .. } | abi::Variants::Empty => {}
210210
abi::Variants::Multiple { variants, .. } => {
211211
// Treat enum variants like union members.
212212
// HACK(eddyb) pretend the `enum` field (discriminant)

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

+4-3
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
213213
&self,
214214
) -> LayoutData<FieldIdx, VariantIdx> {
215215
let dl = self.cx.data_layout();
216+
// This is also used for uninhabited enums, so we use `Variants::Empty`.
216217
LayoutData {
217-
variants: Variants::Single { index: VariantIdx::new(0) },
218+
variants: Variants::Empty,
218219
fields: FieldsShape::Primitive,
219220
backend_repr: BackendRepr::Uninhabited,
220221
largest_niche: None,
@@ -1004,8 +1005,8 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
10041005
Variants::Multiple { tag, tag_encoding, tag_field, .. } => {
10051006
Variants::Multiple { tag, tag_encoding, tag_field, variants: best_layout.variants }
10061007
}
1007-
Variants::Single { .. } => {
1008-
panic!("encountered a single-variant enum during multi-variant layout")
1008+
Variants::Single { .. } | Variants::Empty => {
1009+
panic!("encountered a single-variant or empty enum during multi-variant layout")
10091010
}
10101011
};
10111012
Ok(best_layout.layout)

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -1504,10 +1504,12 @@ impl BackendRepr {
15041504
#[derive(PartialEq, Eq, Hash, Clone, Debug)]
15051505
#[cfg_attr(feature = "nightly", derive(HashStable_Generic))]
15061506
pub enum Variants<FieldIdx: Idx, VariantIdx: Idx> {
1507+
/// A type with no valid variants. Must be uninhabited.
1508+
Empty,
1509+
15071510
/// Single enum variants, structs/tuples, unions, and all non-ADTs.
15081511
Single {
1509-
/// Always 0 for non-enums/generators.
1510-
/// For enums without a variant, this is an invalid index!
1512+
/// Always `0` for types that cannot have multiple variants.
15111513
index: VariantIdx,
15121514
},
15131515

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

+2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ pub(crate) fn codegen_set_discriminant<'tcx>(
1818
return;
1919
}
2020
match layout.variants {
21+
Variants::Empty => unreachable!("we already handled uninhabited types"),
2122
Variants::Single { index } => {
2223
assert_eq!(index, variant_index);
2324
}
@@ -85,6 +86,7 @@ pub(crate) fn codegen_get_discriminant<'tcx>(
8586
}
8687

8788
let (tag_scalar, tag_field, tag_encoding) = match &layout.variants {
89+
Variants::Empty => unreachable!("we already handled uninhabited types"),
8890
Variants::Single { index } => {
8991
let discr_val = layout
9092
.ty

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

+11-14
Original file line numberDiff line numberDiff line change
@@ -212,21 +212,17 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
212212
),
213213
|cx, enum_type_di_node| {
214214
match enum_type_and_layout.variants {
215-
Variants::Single { index: variant_index } => {
216-
if enum_adt_def.variants().is_empty() {
217-
// Uninhabited enums have Variants::Single. We don't generate
218-
// any members for them.
219-
return smallvec![];
220-
}
221-
222-
build_single_variant_union_fields(
223-
cx,
224-
enum_adt_def,
225-
enum_type_and_layout,
226-
enum_type_di_node,
227-
variant_index,
228-
)
215+
Variants::Empty => {
216+
// We don't generate any members for uninhabited types.
217+
return smallvec![];
229218
}
219+
Variants::Single { index: variant_index } => build_single_variant_union_fields(
220+
cx,
221+
enum_adt_def,
222+
enum_type_and_layout,
223+
enum_type_di_node,
224+
variant_index,
225+
),
230226
Variants::Multiple {
231227
tag_encoding: TagEncoding::Direct,
232228
ref variants,
@@ -303,6 +299,7 @@ pub(super) fn build_coroutine_di_node<'ll, 'tcx>(
303299
)
304300
}
305301
Variants::Single { .. }
302+
| Variants::Empty
306303
| Variants::Multiple { tag_encoding: TagEncoding::Niche { .. }, .. } => {
307304
bug!(
308305
"Encountered coroutine with non-direct-tag layout: {:?}",

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ fn compute_discriminant_value<'ll, 'tcx>(
392392
variant_index: VariantIdx,
393393
) -> DiscrResult {
394394
match enum_type_and_layout.layout.variants() {
395-
&Variants::Single { .. } => DiscrResult::NoDiscriminant,
395+
&Variants::Single { .. } | &Variants::Empty => DiscrResult::NoDiscriminant,
396396
&Variants::Multiple { tag_encoding: TagEncoding::Direct, .. } => DiscrResult::Value(
397397
enum_type_and_layout.ty.discriminant_for_variant(cx.tcx, variant_index).unwrap().val,
398398
),

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,8 @@ fn build_discr_member_di_node<'ll, 'tcx>(
358358
let containing_scope = enum_or_coroutine_type_di_node;
359359

360360
match enum_or_coroutine_type_and_layout.layout.variants() {
361-
// A single-variant enum has no discriminant.
362-
&Variants::Single { .. } => None,
361+
// A single-variant or no-variant enum has no discriminant.
362+
&Variants::Single { .. } | &Variants::Empty => None,
363363

364364
&Variants::Multiple { tag_field, .. } => {
365365
let tag_base_type = tag_base_type(cx.tcx, enum_or_coroutine_type_and_layout);

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ fn uncached_llvm_type<'a, 'tcx>(
3838
if let (&ty::Adt(def, _), &Variants::Single { 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
}

Diff for: compiler/rustc_codegen_ssa/src/debuginfo/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ fn tag_base_type_opt<'tcx>(
6565
});
6666

6767
match enum_type_and_layout.layout.variants() {
68-
// A single-variant enum has no discriminant.
69-
Variants::Single { .. } => None,
68+
// A single-variant or no-variant enum has no discriminant.
69+
Variants::Single { .. } | Variants::Empty => None,
7070

7171
Variants::Multiple { tag_encoding: TagEncoding::Niche { .. }, tag, .. } => {
7272
// Niche tags are always normalized to unsized integers of the correct size.

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

+4-3
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
243243
return bx.cx().const_poison(cast_to);
244244
}
245245
let (tag_scalar, tag_encoding, tag_field) = match self.layout.variants {
246+
Variants::Empty => unreachable!("we already handled uninhabited types"),
246247
Variants::Single { index } => {
247248
let discr_val = self
248249
.layout
@@ -365,9 +366,9 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
365366
return;
366367
}
367368
match self.layout.variants {
368-
Variants::Single { index } => {
369-
assert_eq!(index, variant_index);
370-
}
369+
Variants::Empty => unreachable!("we already handled uninhabited types"),
370+
Variants::Single { index } => assert_eq!(index, variant_index),
371+
371372
Variants::Multiple { tag_encoding: TagEncoding::Direct, tag_field, .. } => {
372373
let ptr = self.project_field(bx, tag_field);
373374
let to =

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

+12-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.
@@ -65,21 +65,17 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
6565
// We use "tag" to refer to how the discriminant is encoded in memory, which can be either
6666
// straight-forward (`TagEncoding::Direct`) or with a niche (`TagEncoding::Niche`).
6767
let (tag_scalar_layout, tag_encoding, tag_field) = match op.layout().variants {
68+
Variants::Empty => {
69+
throw_ub!(UninhabitedEnumVariantRead(None));
70+
}
6871
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-
}
72+
if op.layout().is_uninhabited() {
7673
// For consistency with `write_discriminant`, and to make sure that
7774
// `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-
}
75+
// for uninhabited enums.
76+
throw_ub!(UninhabitedEnumVariantRead(Some(index)));
8277
}
78+
// Since the type is inhabited, there must be an index.
8379
return interp_ok(index);
8480
}
8581
Variants::Multiple { tag, ref tag_encoding, tag_field, .. } => {
@@ -199,11 +195,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
199195
// `uninhabited_enum_branching` MIR pass. It also ensures consistency with
200196
// `write_discriminant`.
201197
if op.layout().for_variant(self, index).is_uninhabited() {
202-
throw_ub!(UninhabitedEnumVariantRead(index))
198+
throw_ub!(UninhabitedEnumVariantRead(Some(index)))
203199
}
204200
interp_ok(index)
205201
}
206202

203+
/// Read discriminant, return the user-visible discriminant.
204+
/// Can also legally be called on non-enums (e.g. through the discriminant_value intrinsic)!
207205
pub fn discriminant_for_variant(
208206
&self,
209207
ty: Ty<'tcx>,
@@ -243,6 +241,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
243241
}
244242

245243
match layout.variants {
244+
abi::Variants::Empty => unreachable!("we already handled uninhabited types"),
246245
abi::Variants::Single { .. } => {
247246
// The tag of a `Single` enum is like the tag of the niched
248247
// variant: there's no tag as the discriminant is encoded

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
302302
};
303303
}
304304
}
305-
Variants::Single { .. } => {}
305+
Variants::Single { .. } | Variants::Empty => {}
306306
}
307307

308308
// Now we know we are projecting to a field, so figure out which one.
@@ -344,6 +344,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
344344
// Inside a variant
345345
PathElem::Field(def.variant(index).fields[FieldIdx::from_usize(field)].name)
346346
}
347+
Variants::Empty => panic!("there is no field in Variants::Empty types"),
347348
Variants::Multiple { .. } => bug!("we handled variants above"),
348349
}
349350
}
@@ -1010,7 +1011,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
10101011
}
10111012
// Don't forget potential other variants.
10121013
match &layout.variants {
1013-
Variants::Single { .. } => {
1014+
Variants::Single { .. } | Variants::Empty => {
10141015
// Fully handled above.
10151016
}
10161017
Variants::Multiple { variants, .. } => {

Diff for: compiler/rustc_const_eval/src/interpret/visitor.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,8 @@ pub trait ValueVisitor<'tcx, M: Machine<'tcx>>: Sized {
218218
// recurse with the inner type
219219
self.visit_variant(v, idx, &inner)?;
220220
}
221-
// For single-variant layouts, we already did anything there is to do.
222-
Variants::Single { .. } => {}
221+
// For single-variant layouts, we already did everything there is to do.
222+
Variants::Single { .. } | Variants::Empty => {}
223223
}
224224

225225
interp_ok(())

Diff for: compiler/rustc_const_eval/src/util/check_validity_requirement.rs

+1
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ fn check_validity_requirement_lax<'tcx>(
155155
}
156156

157157
match &this.variants {
158+
Variants::Empty => return Ok(false),
158159
Variants::Single { .. } => {
159160
// All fields of this single variant have already been checked above, there is nothing
160161
// else to do.

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ pub enum UndefinedBehaviorInfo<'tcx> {
392392
/// A discriminant of an uninhabited enum variant is written.
393393
UninhabitedEnumVariantWritten(VariantIdx),
394394
/// An uninhabited enum variant is projected.
395-
UninhabitedEnumVariantRead(VariantIdx),
395+
UninhabitedEnumVariantRead(Option<VariantIdx>),
396396
/// Trying to set discriminant to the niched variant, but the value does not match.
397397
InvalidNichedEnumVariantWritten { enum_ty: Ty<'tcx> },
398398
/// ABI-incompatible argument types.

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

+9-6
Original file line numberDiff line numberDiff line change
@@ -734,21 +734,22 @@ where
734734
let layout = match this.variants {
735735
Variants::Single { index }
736736
// If all variants but one are uninhabited, the variant layout is the enum layout.
737-
if index == variant_index &&
738-
// Don't confuse variants of uninhabited enums with the enum itself.
739-
// For more details see https://github.com/rust-lang/rust/issues/69763.
740-
this.fields != FieldsShape::Primitive =>
737+
if index == variant_index =>
741738
{
742739
this.layout
743740
}
744741

745-
Variants::Single { index } => {
742+
Variants::Single { .. } | Variants::Empty => {
743+
// Single-variant and no-variant enums *can* have other variants, but those are
744+
// uninhabited. Produce a layout that has the right fields for that variant, so that
745+
// the rest of the compiler can project fields etc as usual.
746+
746747
let tcx = cx.tcx();
747748
let typing_env = cx.typing_env();
748749

749750
// Deny calling for_variant more than once for non-Single enums.
750751
if let Ok(original_layout) = tcx.layout_of(typing_env.as_query_input(this.ty)) {
751-
assert_eq!(original_layout.variants, Variants::Single { index });
752+
assert_eq!(original_layout.variants, this.variants);
752753
}
753754

754755
let fields = match this.ty.kind() {
@@ -902,6 +903,7 @@ where
902903
),
903904

904905
ty::Coroutine(def_id, args) => match this.variants {
906+
Variants::Empty => unreachable!(),
905907
Variants::Single { index } => TyMaybeWithLayout::Ty(
906908
args.as_coroutine()
907909
.state_tys(def_id, tcx)
@@ -927,6 +929,7 @@ where
927929
let field = &def.variant(index).fields[FieldIdx::from_usize(i)];
928930
TyMaybeWithLayout::Ty(field.ty(tcx, args))
929931
}
932+
Variants::Empty => panic!("there is no field in Variants::Empty types"),
930933

931934
// Discriminant field for enums (where applicable).
932935
Variants::Multiple { tag, .. } => {

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/large_enums.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ impl EnumSizeOpt {
216216
};
217217
let layout = tcx.layout_of(typing_env.as_query_input(ty)).ok()?;
218218
let variants = match &layout.variants {
219-
Variants::Single { .. } => return None,
219+
Variants::Single { .. } | Variants::Empty => return None,
220220
Variants::Multiple { tag_encoding: TagEncoding::Niche { .. }, .. } => return None,
221221

222222
Variants::Multiple { variants, .. } if variants.len() <= 1 => return None,

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

+4
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ fn variant_discriminants<'tcx>(
5454
tcx: TyCtxt<'tcx>,
5555
) -> FxHashSet<u128> {
5656
match &layout.variants {
57+
Variants::Empty => {
58+
// Uninhabited, no valid discriminant.
59+
FxHashSet::default()
60+
}
5761
Variants::Single { index } => {
5862
let mut res = FxHashSet::default();
5963
res.insert(

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

+1
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ impl<'tcx> Stable<'tcx> for rustc_abi::Variants<rustc_abi::FieldIdx, rustc_abi::
167167
rustc_abi::Variants::Single { index } => {
168168
VariantsShape::Single { index: index.stable(tables) }
169169
}
170+
rustc_abi::Variants::Empty => VariantsShape::Empty,
170171
rustc_abi::Variants::Multiple { tag, tag_encoding, tag_field, variants } => {
171172
VariantsShape::Multiple {
172173
tag: tag.stable(tables),

0 commit comments

Comments
 (0)