Skip to content

[WIP]: Do not store tag in uninhabited enum variants, or in the single inhabited variant. #145337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_abi/src/callconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
let (mut result, mut total) = from_fields_at(*self, Size::ZERO)?;

match &self.variants {
Variants::Single { .. } | Variants::Empty => {}
Variants::Single { .. } | Variants::Empty { .. } => {}
Variants::Multiple { variants, .. } => {
// Treat enum variants like union members.
// HACK(eddyb) pretend the `enum` field (discriminant)
Expand Down
265 changes: 216 additions & 49 deletions compiler/rustc_abi/src/layout.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion compiler/rustc_abi/src/layout/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ pub(super) fn layout<
&ReprOptions::default(),
StructKind::Prefixed(prefix_size, prefix_align.abi),
)?;
variant.variants = Variants::Single { index };
variant.variants = Variants::Single { index, variants: None };

let FieldsShape::Arbitrary { offsets, memory_index } = variant.fields else {
unreachable!();
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_abi/src/layout/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
pub fn unit<C: HasDataLayout>(cx: &C, sized: bool) -> Self {
let dl = cx.data_layout();
LayoutData {
variants: Variants::Single { index: VariantIdx::new(0) },
variants: Variants::Single { index: VariantIdx::new(0), variants: None },
fields: FieldsShape::Arbitrary {
offsets: IndexVec::new(),
memory_index: IndexVec::new(),
Expand All @@ -32,7 +32,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
let dl = cx.data_layout();
// This is also used for uninhabited enums, so we use `Variants::Empty`.
LayoutData {
variants: Variants::Empty,
variants: Variants::Empty { variants: None },
fields: FieldsShape::Primitive,
backend_repr: BackendRepr::Memory { sized: true },
largest_niche: None,
Expand Down Expand Up @@ -74,7 +74,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
.wrapping_add((range.end as u64).rotate_right(16));

LayoutData {
variants: Variants::Single { index: VariantIdx::new(0) },
variants: Variants::Single { index: VariantIdx::new(0), variants: None },
fields: FieldsShape::Primitive,
backend_repr: BackendRepr::Scalar(scalar),
largest_niche,
Expand Down Expand Up @@ -104,7 +104,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
let combined_seed = a.size(dl).bytes().wrapping_add(b.size(dl).bytes());

LayoutData {
variants: Variants::Single { index: VariantIdx::new(0) },
variants: Variants::Single { index: VariantIdx::new(0), variants: None },
fields: FieldsShape::Arbitrary {
offsets: [Size::ZERO, b_offset].into(),
memory_index: [0, 1].into(),
Expand All @@ -120,14 +120,14 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
}
}

/// Returns a dummy layout for an uninhabited variant.
/// Returns a dummy layout for an absent variant.
///
/// Uninhabited variants get pruned as part of the layout calculation,
/// Absent variants get pruned as part of the layout calculation,
/// so this can be used after the fact to reconstitute a layout.
pub fn uninhabited_variant<C: HasDataLayout>(cx: &C, index: VariantIdx, fields: usize) -> Self {
pub fn absent_variant<C: HasDataLayout>(cx: &C, index: VariantIdx, fields: usize) -> Self {
let dl = cx.data_layout();
LayoutData {
variants: Variants::Single { index },
variants: Variants::Single { index, variants: None },
fields: match NonZero::new(fields) {
Some(fields) => FieldsShape::Union(fields),
None => FieldsShape::Arbitrary {
Expand Down
19 changes: 17 additions & 2 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1852,12 +1852,27 @@ impl BackendRepr {
#[cfg_attr(feature = "nightly", derive(HashStable_Generic))]
pub enum Variants<FieldIdx: Idx, VariantIdx: Idx> {
/// A type with no valid variants. Must be uninhabited.
Empty,
///
/// We still need to hold variant layout information for `offset_of!`
/// on uninhabited enum variants with non-zero-sized fields.
Empty {
/// Always `None` for non-enums.
/// For enums, this is `None` if all variants are "absent"
/// (have no non-1-ZST fields), and is `Some` otherwise.
variants: Option<IndexVec<VariantIdx, LayoutData<FieldIdx, VariantIdx>>>,
},

/// Single enum variants, structs/tuples, unions, and all non-ADTs.
/// Enums with one inhabited variant and no tag, structs/tuples, unions, and all non-ADTs.
///
/// We still need to hold variant layout information for `offset_of!`
/// on uninhabited enum variants with non-zero-sized fields.
Single {
/// Always `0` for types that cannot have multiple variants.
index: VariantIdx,
/// Always `None` for non-enums.
/// For enums, this is `None` if all uninhabited variants are "absent"
/// (have no non-1-ZST fields), and is `Some` otherwise.
variants: Option<IndexVec<VariantIdx, LayoutData<FieldIdx, VariantIdx>>>,
},

/// Enum-likes with more than one variant: each variant comes with
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_cranelift/src/discriminant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ pub(crate) fn codegen_set_discriminant<'tcx>(
return;
}
match layout.variants {
Variants::Empty => unreachable!("we already handled uninhabited types"),
Variants::Single { index } => {
Variants::Empty { .. } => unreachable!("we already handled uninhabited types"),
Variants::Single { index, .. } => {
assert_eq!(index, variant_index);
}
Variants::Multiple {
Expand Down Expand Up @@ -86,8 +86,8 @@ 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 } => {
Variants::Empty { .. } => unreachable!("we already handled uninhabited types"),
Variants::Single { index, .. } => {
let discr_val = layout
.ty
.discriminant_for_variant(fx.tcx, *index)
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_gcc/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,14 @@ fn uncached_gcc_type<'gcc, 'tcx>(
if !cx.sess().fewer_names() =>
{
let mut name = with_no_trimmed_paths!(layout.ty.to_string());
if let (&ty::Adt(def, _), &Variants::Single { index }) =
if let (&ty::Adt(def, _), &Variants::Single { index, .. }) =
(layout.ty.kind(), &layout.variants)
&& def.is_enum()
&& !def.variants().is_empty()
{
write!(&mut name, "::{}", def.variant(index).name).unwrap();
}
if let (&ty::Coroutine(_, _), &Variants::Single { index }) =
if let (&ty::Coroutine(_, _), &Variants::Single { index, .. }) =
(layout.ty.kind(), &layout.variants)
{
write!(&mut name, "::{}", ty::CoroutineArgs::variant_name(index)).unwrap();
Expand Down Expand Up @@ -228,7 +228,7 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {

// Check the cache.
let variant_index = match self.variants {
Variants::Single { index } => Some(index),
Variants::Single { index, .. } => Some(index),
_ => None,
};
let cached_type = cx.types.borrow().get(&(self.ty, variant_index)).cloned();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,11 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
),
|cx, enum_type_di_node| {
match enum_type_and_layout.variants {
Variants::Empty => {
Variants::Empty { .. } => {
// We don't generate any members for uninhabited types.
return smallvec![];
}
Variants::Single { index: variant_index } => build_single_variant_union_fields(
Variants::Single { index: variant_index, .. } => build_single_variant_union_fields(
cx,
enum_adt_def,
enum_type_and_layout,
Expand Down Expand Up @@ -300,7 +300,7 @@ pub(super) fn build_coroutine_di_node<'ll, 'tcx>(
)
}
Variants::Single { .. }
| Variants::Empty
| 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 { .. } | &Variants::Empty => 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 @@ -357,7 +357,7 @@ fn build_discr_member_di_node<'ll, 'tcx>(

match enum_or_coroutine_type_and_layout.layout.variants() {
// A single-variant or no-variant enum has no discriminant.
&Variants::Single { .. } | &Variants::Empty => None,
&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
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_llvm/src/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ fn uncached_llvm_type<'a, 'tcx>(
if !cx.sess().fewer_names() =>
{
let mut name = with_no_visible_paths!(with_no_trimmed_paths!(layout.ty.to_string()));
if let (&ty::Adt(def, _), &Variants::Single { index }) =
if let (&ty::Adt(def, _), &Variants::Single { index, .. }) =
(layout.ty.kind(), &layout.variants)
{
if def.is_enum() {
write!(&mut name, "::{}", def.variant(index).name).unwrap();
}
}
if let (&ty::Coroutine(_, _), &Variants::Single { index }) =
if let (&ty::Coroutine(_, _), &Variants::Single { index, .. }) =
(layout.ty.kind(), &layout.variants)
{
write!(&mut name, "::{}", ty::CoroutineArgs::variant_name(index)).unwrap();
Expand Down Expand Up @@ -213,7 +213,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> {

// Check the cache.
let variant_index = match self.variants {
Variants::Single { index } => Some(index),
Variants::Single { index, .. } => Some(index),
_ => None,
};
if let Some(llty) = cx.type_lowering.borrow().get(&(self.ty, variant_index)) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ fn tag_base_type_opt<'tcx>(

match enum_type_and_layout.layout.variants() {
// A single-variant or no-variant enum has no discriminant.
Variants::Single { .. } | Variants::Empty => None,
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
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl<'a, 'b, 'tcx, Bx: BuilderMethods<'b, 'tcx>> LocalAnalyzer<'a, 'b, 'tcx, Bx>
layout = match *elem {
mir::PlaceElem::Field(fidx, ..) => layout.field(self.fx.cx, fidx.as_usize()),
mir::PlaceElem::Downcast(_, vidx)
if let abi::Variants::Single { index: single_variant } =
if let abi::Variants::Single { index: single_variant, .. } =
layout.variants
&& vidx == single_variant =>
{
Expand Down
16 changes: 10 additions & 6 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,8 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
}

let (tag_scalar, tag_encoding, tag_field) = match self.layout.variants {
Variants::Empty => unreachable!("we already handled uninhabited types"),
Variants::Single { index } => {
Variants::Empty { .. } => unreachable!("we already handled uninhabited types"),
Variants::Single { index, .. } => {
let discr_val =
if let Some(discr) = self.layout.ty.discriminant_for_variant(bx.tcx(), index) {
discr.val
Expand Down Expand Up @@ -949,10 +949,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
o = o.extract_field(self, bx, f.index());
}
mir::PlaceElem::Downcast(_, vidx) => {
debug_assert_eq!(
o.layout.variants,
abi::Variants::Single { index: vidx },
);
if cfg!(debug_assertions) {
match o.layout.variants {
abi::Variants::Single { index, .. } if index == vidx => {}
ref v => bug!(
"expected Variants::Single {{ index: {vidx:?}, .. }}, got {v:?}"
),
}
}
let layout = o.layout.for_variant(bx.cx(), vidx);
o = OperandRef { val: o.val, layout }
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,8 @@ pub(super) fn codegen_tag_value<'tcx, V>(
}

Ok(match layout.variants {
Variants::Empty => unreachable!("we already handled uninhabited types"),
Variants::Single { index } => {
Variants::Empty { .. } => unreachable!("we already handled uninhabited types"),
Variants::Single { index, .. } => {
assert_eq!(index, variant_index);
None
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/discriminant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ 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 => {
Variants::Empty { .. } => {
throw_ub!(UninhabitedEnumVariantRead(None));
}
Variants::Single { index } => {
Variants::Single { 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
Expand Down Expand Up @@ -241,7 +241,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
}

match layout.variants {
abi::Variants::Empty => unreachable!("we already handled uninhabited types"),
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
8 changes: 4 additions & 4 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
};
}
}
Variants::Single { .. } | Variants::Empty => {}
Variants::Single { .. } | Variants::Empty { .. } => {}
}

// Now we know we are projecting to a field, so figure out which one.
Expand Down Expand Up @@ -341,11 +341,11 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
ty::Adt(def, ..) if def.is_enum() => {
// we might be projecting *to* a variant, or to a field *in* a variant.
match layout.variants {
Variants::Single { index } => {
Variants::Single { index, .. } => {
// 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::Empty { .. } => panic!("there is no field in Variants::Empty types"),
Variants::Multiple { .. } => bug!("we handled variants above"),
}
}
Expand Down Expand Up @@ -1020,7 +1020,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
}
// Don't forget potential other variants.
match &layout.variants {
Variants::Single { .. } | Variants::Empty => {
Variants::Single { .. } | Variants::Empty { .. } => {
// Fully handled above.
}
Variants::Multiple { variants, .. } => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ pub trait ValueVisitor<'tcx, M: Machine<'tcx>>: Sized {
self.visit_variant(v, idx, &inner)?;
}
// For single-variant layouts, we already did everything there is to do.
Variants::Single { .. } | Variants::Empty => {}
Variants::Single { .. } | Variants::Empty { .. } => {}
}

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

match &this.variants {
Variants::Empty => return Ok(false),
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
Loading
Loading