Skip to content

Commit

Permalink
Rollup merge of rust-lang#133702 - RalfJung:single-variant, r=oli-obk
Browse files Browse the repository at this point in the history
Variants::Single: do not use invalid VariantIdx for uninhabited enums

~~Stacked on top of rust-lang#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.
  • Loading branch information
jieyouxu authored Dec 18, 2024
2 parents 37e7459 + b53a3b6 commit 2c8babe
Show file tree
Hide file tree
Showing 33 changed files with 186 additions and 165 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_abi/src/callconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
let (mut result, mut total) = from_fields_at(*self, Size::ZERO)?;

match &self.variants {
abi::Variants::Single { .. } => {}
abi::Variants::Single { .. } | abi::Variants::Empty => {}
abi::Variants::Multiple { variants, .. } => {
// Treat enum variants like union members.
// HACK(eddyb) pretend the `enum` field (discriminant)
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_abi/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
&self,
) -> LayoutData<FieldIdx, VariantIdx> {
let dl = self.cx.data_layout();
// This is also used for uninhabited enums, so we use `Variants::Empty`.
LayoutData {
variants: Variants::Single { index: VariantIdx::new(0) },
variants: Variants::Empty,
fields: FieldsShape::Primitive,
backend_repr: BackendRepr::Uninhabited,
largest_niche: None,
Expand Down Expand Up @@ -1004,8 +1005,8 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
Variants::Multiple { tag, tag_encoding, tag_field, .. } => {
Variants::Multiple { tag, tag_encoding, tag_field, variants: best_layout.variants }
}
Variants::Single { .. } => {
panic!("encountered a single-variant enum during multi-variant layout")
Variants::Single { .. } | Variants::Empty => {
panic!("encountered a single-variant or empty enum during multi-variant layout")
}
};
Ok(best_layout.layout)
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1504,10 +1504,12 @@ impl BackendRepr {
#[derive(PartialEq, Eq, Hash, Clone, Debug)]
#[cfg_attr(feature = "nightly", derive(HashStable_Generic))]
pub enum Variants<FieldIdx: Idx, VariantIdx: Idx> {
/// A type with no valid variants. Must be uninhabited.
Empty,

/// Single enum variants, structs/tuples, unions, and all non-ADTs.
Single {
/// Always 0 for non-enums/generators.
/// For enums without a variant, this is an invalid index!
/// Always `0` for types that cannot have multiple variants.
index: VariantIdx,
},

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_cranelift/src/discriminant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub(crate) fn codegen_set_discriminant<'tcx>(
return;
}
match layout.variants {
Variants::Empty => unreachable!("we already handled uninhabited types"),
Variants::Single { index } => {
assert_eq!(index, variant_index);
}
Expand Down Expand Up @@ -85,6 +86,7 @@ pub(crate) fn codegen_get_discriminant<'tcx>(
}

let (tag_scalar, tag_field, tag_encoding) = match &layout.variants {
Variants::Empty => unreachable!("we already handled uninhabited types"),
Variants::Single { index } => {
let discr_val = layout
.ty
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,21 +212,17 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
),
|cx, enum_type_di_node| {
match enum_type_and_layout.variants {
Variants::Single { index: variant_index } => {
if enum_adt_def.variants().is_empty() {
// Uninhabited enums have Variants::Single. We don't generate
// any members for them.
return smallvec![];
}

build_single_variant_union_fields(
cx,
enum_adt_def,
enum_type_and_layout,
enum_type_di_node,
variant_index,
)
Variants::Empty => {
// We don't generate any members for uninhabited types.
return smallvec![];
}
Variants::Single { index: variant_index } => build_single_variant_union_fields(
cx,
enum_adt_def,
enum_type_and_layout,
enum_type_di_node,
variant_index,
),
Variants::Multiple {
tag_encoding: TagEncoding::Direct,
ref variants,
Expand Down Expand Up @@ -303,6 +299,7 @@ pub(super) fn build_coroutine_di_node<'ll, 'tcx>(
)
}
Variants::Single { .. }
| Variants::Empty
| Variants::Multiple { tag_encoding: TagEncoding::Niche { .. }, .. } => {
bug!(
"Encountered coroutine with non-direct-tag layout: {:?}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ fn compute_discriminant_value<'ll, 'tcx>(
variant_index: VariantIdx,
) -> DiscrResult {
match enum_type_and_layout.layout.variants() {
&Variants::Single { .. } => DiscrResult::NoDiscriminant,
&Variants::Single { .. } | &Variants::Empty => DiscrResult::NoDiscriminant,
&Variants::Multiple { tag_encoding: TagEncoding::Direct, .. } => DiscrResult::Value(
enum_type_and_layout.ty.discriminant_for_variant(cx.tcx, variant_index).unwrap().val,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,8 +358,8 @@ fn build_discr_member_di_node<'ll, 'tcx>(
let containing_scope = enum_or_coroutine_type_di_node;

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

&Variants::Multiple { tag_field, .. } => {
let tag_base_type = tag_base_type(cx.tcx, enum_or_coroutine_type_and_layout);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn uncached_llvm_type<'a, 'tcx>(
if let (&ty::Adt(def, _), &Variants::Single { index }) =
(layout.ty.kind(), &layout.variants)
{
if def.is_enum() && !def.variants().is_empty() {
if def.is_enum() {
write!(&mut name, "::{}", def.variant(index).name).unwrap();
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ fn tag_base_type_opt<'tcx>(
});

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

Variants::Multiple { tag_encoding: TagEncoding::Niche { .. }, tag, .. } => {
// Niche tags are always normalized to unsized integers of the correct size.
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
return bx.cx().const_poison(cast_to);
}
let (tag_scalar, tag_encoding, tag_field) = match self.layout.variants {
Variants::Empty => unreachable!("we already handled uninhabited types"),
Variants::Single { index } => {
let discr_val = self
.layout
Expand Down Expand Up @@ -365,9 +366,9 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
return;
}
match self.layout.variants {
Variants::Single { index } => {
assert_eq!(index, variant_index);
}
Variants::Empty => unreachable!("we already handled uninhabited types"),
Variants::Single { index } => assert_eq!(index, variant_index),

Variants::Multiple { tag_encoding: TagEncoding::Direct, tag_field, .. } => {
let ptr = self.project_field(bx, tag_field);
let to =
Expand Down
25 changes: 12 additions & 13 deletions compiler/rustc_const_eval/src/interpret/discriminant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
}
}

/// Read discriminant, return the runtime value as well as the variant index.
/// Read discriminant, return the variant index.
/// Can also legally be called on non-enums (e.g. through the discriminant_value intrinsic)!
///
/// Will never return an uninhabited variant.
Expand All @@ -65,21 +65,17 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// 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 {
Variants::Empty => {
throw_ub!(UninhabitedEnumVariantRead(None));
}
Variants::Single { index } => {
// Do some extra checks on enums.
if ty.is_enum() {
// Hilariously, `Single` is used even for 0-variant enums.
// (See https://github.com/rust-lang/rust/issues/89765).
if ty.ty_adt_def().unwrap().variants().is_empty() {
throw_ub!(UninhabitedEnumVariantRead(index))
}
if op.layout().is_uninhabited() {
// For consistency 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).is_uninhabited() {
throw_ub!(UninhabitedEnumVariantRead(index))
}
// for uninhabited enums.
throw_ub!(UninhabitedEnumVariantRead(Some(index)));
}
// Since the type is inhabited, there must be an index.
return interp_ok(index);
}
Variants::Multiple { tag, ref tag_encoding, tag_field, .. } => {
Expand Down Expand Up @@ -199,11 +195,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// `uninhabited_enum_branching` MIR pass. It also ensures consistency with
// `write_discriminant`.
if op.layout().for_variant(self, index).is_uninhabited() {
throw_ub!(UninhabitedEnumVariantRead(index))
throw_ub!(UninhabitedEnumVariantRead(Some(index)))
}
interp_ok(index)
}

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

match layout.variants {
abi::Variants::Empty => unreachable!("we already handled uninhabited types"),
abi::Variants::Single { .. } => {
// The tag of a `Single` enum is like the tag of the niched
// variant: there's no tag as the discriminant is encoded
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
};
}
}
Variants::Single { .. } => {}
Variants::Single { .. } | Variants::Empty => {}
}

// Now we know we are projecting to a field, so figure out which one.
Expand Down Expand Up @@ -344,6 +344,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
// Inside a variant
PathElem::Field(def.variant(index).fields[FieldIdx::from_usize(field)].name)
}
Variants::Empty => panic!("there is no field in Variants::Empty types"),
Variants::Multiple { .. } => bug!("we handled variants above"),
}
}
Expand Down Expand Up @@ -1010,7 +1011,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
}
// Don't forget potential other variants.
match &layout.variants {
Variants::Single { .. } => {
Variants::Single { .. } | Variants::Empty => {
// Fully handled above.
}
Variants::Multiple { variants, .. } => {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ pub trait ValueVisitor<'tcx, M: Machine<'tcx>>: Sized {
// recurse with the inner type
self.visit_variant(v, idx, &inner)?;
}
// For single-variant layouts, we already did anything there is to do.
Variants::Single { .. } => {}
// For single-variant layouts, we already did everything there is to do.
Variants::Single { .. } | Variants::Empty => {}
}

interp_ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ fn check_validity_requirement_lax<'tcx>(
}

match &this.variants {
Variants::Empty => return Ok(false),
Variants::Single { .. } => {
// All fields of this single variant have already been checked above, there is nothing
// else to do.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ pub enum UndefinedBehaviorInfo<'tcx> {
/// A discriminant of an uninhabited enum variant is written.
UninhabitedEnumVariantWritten(VariantIdx),
/// An uninhabited enum variant is projected.
UninhabitedEnumVariantRead(VariantIdx),
UninhabitedEnumVariantRead(Option<VariantIdx>),
/// Trying to set discriminant to the niched variant, but the value does not match.
InvalidNichedEnumVariantWritten { enum_ty: Ty<'tcx> },
/// ABI-incompatible argument types.
Expand Down
15 changes: 9 additions & 6 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -734,21 +734,22 @@ where
let layout = match this.variants {
Variants::Single { index }
// If all variants but one are uninhabited, the variant layout is the enum layout.
if index == variant_index &&
// Don't confuse variants of uninhabited enums with the enum itself.
// For more details see https://github.com/rust-lang/rust/issues/69763.
this.fields != FieldsShape::Primitive =>
if index == variant_index =>
{
this.layout
}

Variants::Single { index } => {
Variants::Single { .. } | Variants::Empty => {
// Single-variant and no-variant enums *can* have other variants, but those are
// uninhabited. Produce a layout that has the right fields for that variant, so that
// the rest of the compiler can project fields etc as usual.

let tcx = cx.tcx();
let typing_env = cx.typing_env();

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

let fields = match this.ty.kind() {
Expand Down Expand Up @@ -902,6 +903,7 @@ where
),

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

// Discriminant field for enums (where applicable).
Variants::Multiple { tag, .. } => {
Expand Down
31 changes: 7 additions & 24 deletions compiler/rustc_mir_transform/src/jump_threading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
//! Likewise, applying the optimisation can create a lot of new MIR, so we bound the instruction
//! cost by `MAX_COST`.
use rustc_abi::{TagEncoding, Variants};
use rustc_arena::DroplessArena;
use rustc_const_eval::const_eval::DummyMachine;
use rustc_const_eval::interpret::{ImmTy, Immediate, InterpCx, OpTy, Projectable};
Expand Down Expand Up @@ -565,31 +564,15 @@ impl<'a, 'tcx> TOFinder<'a, 'tcx> {
StatementKind::SetDiscriminant { box place, variant_index } => {
let Some(discr_target) = self.map.find_discr(place.as_ref()) else { return };
let enum_ty = place.ty(self.body, self.tcx).ty;
// `SetDiscriminant` may be a no-op if the assigned variant is the untagged variant
// of a niche encoding. If we cannot ensure that we write to the discriminant, do
// nothing.
let Ok(enum_layout) = self.ecx.layout_of(enum_ty) else {
// `SetDiscriminant` guarantees that the discriminant is now `variant_index`.
// Even if the discriminant write does nothing due to niches, it is UB to set the
// discriminant when the data does not encode the desired discriminant.
let Some(discr) =
self.ecx.discriminant_for_variant(enum_ty, *variant_index).discard_err()
else {
return;
};
let writes_discriminant = match enum_layout.variants {
Variants::Single { index } => {
assert_eq!(index, *variant_index);
true
}
Variants::Multiple { tag_encoding: TagEncoding::Direct, .. } => true,
Variants::Multiple {
tag_encoding: TagEncoding::Niche { untagged_variant, .. },
..
} => *variant_index != untagged_variant,
};
if writes_discriminant {
let Some(discr) =
self.ecx.discriminant_for_variant(enum_ty, *variant_index).discard_err()
else {
return;
};
self.process_immediate(bb, discr_target, discr, state);
}
self.process_immediate(bb, discr_target, discr, state);
}
// If we expect `lhs ?= true`, we have an opportunity if we assume `lhs == true`.
StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/large_enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ impl EnumSizeOpt {
};
let layout = tcx.layout_of(typing_env.as_query_input(ty)).ok()?;
let variants = match &layout.variants {
Variants::Single { .. } => return None,
Variants::Single { .. } | Variants::Empty => return None,
Variants::Multiple { tag_encoding: TagEncoding::Niche { .. }, .. } => return None,

Variants::Multiple { variants, .. } if variants.len() <= 1 => return None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ fn variant_discriminants<'tcx>(
tcx: TyCtxt<'tcx>,
) -> FxHashSet<u128> {
match &layout.variants {
Variants::Empty => {
// Uninhabited, no valid discriminant.
FxHashSet::default()
}
Variants::Single { index } => {
let mut res = FxHashSet::default();
res.insert(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_smir/src/rustc_smir/convert/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ impl<'tcx> Stable<'tcx> for rustc_abi::Variants<rustc_abi::FieldIdx, rustc_abi::
rustc_abi::Variants::Single { index } => {
VariantsShape::Single { index: index.stable(tables) }
}
rustc_abi::Variants::Empty => VariantsShape::Empty,
rustc_abi::Variants::Multiple { tag, tag_encoding, tag_field, variants } => {
VariantsShape::Multiple {
tag: tag.stable(tables),
Expand Down
Loading

0 comments on commit 2c8babe

Please sign in to comment.