Skip to content

Change tag_field to FieldIdx in Variants::Multiple #142005

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 5, 2025
Merged

Change tag_field to FieldIdx in Variants::Multiple #142005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions compiler/rustc_abi/src/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
niche_variants,
niche_start,
},
tag_field: 0,
tag_field: FieldIdx::new(0),
variants: IndexVec::new(),
},
fields: FieldsShape::Arbitrary {
Expand Down Expand Up @@ -1072,7 +1072,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
variants: Variants::Multiple {
tag,
tag_encoding: TagEncoding::Direct,
tag_field: 0,
tag_field: FieldIdx::new(0),
variants: IndexVec::new(),
},
fields: FieldsShape::Arbitrary {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_abi/src/layout/coroutine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ pub(super) fn layout<
// Build a prefix layout, including "promoting" all ineligible
// locals as part of the prefix. We compute the layout of all of
// these fields at once to get optimal packing.
let tag_index = prefix_layouts.len();
let tag_index = prefix_layouts.next_index();

// `variant_fields` already accounts for the reserved variants, so no need to add them.
let max_discr = (variant_fields.len() - 1) as u128;
Expand Down Expand Up @@ -187,7 +187,7 @@ pub(super) fn layout<

// "a" (`0..b_start`) and "b" (`b_start..`) correspond to
// "outer" and "promoted" fields respectively.
let b_start = FieldIdx::new(tag_index + 1);
let b_start = tag_index.plus(1);
let offsets_b = IndexVec::from_raw(offsets.raw.split_off(b_start.index()));
let offsets_a = offsets;

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_abi/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1573,7 +1573,7 @@ pub enum Variants<FieldIdx: Idx, VariantIdx: Idx> {
Multiple {
tag: Scalar,
tag_encoding: TagEncoding<VariantIdx>,
tag_field: usize,
tag_field: FieldIdx,
variants: IndexVec<VariantIdx, LayoutData<FieldIdx, VariantIdx>>,
},
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_cranelift/src/discriminant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub(crate) fn codegen_set_discriminant<'tcx>(
tag_encoding: TagEncoding::Direct,
variants: _,
} => {
let ptr = place.place_field(fx, FieldIdx::new(tag_field));
let ptr = place.place_field(fx, tag_field);
let to = layout.ty.discriminant_for_variant(fx.tcx, variant_index).unwrap().val;
let to = match ptr.layout().ty.kind() {
ty::Uint(UintTy::U128) | ty::Int(IntTy::I128) => {
Expand All @@ -53,7 +53,7 @@ pub(crate) fn codegen_set_discriminant<'tcx>(
variants: _,
} => {
if variant_index != untagged_variant {
let niche = place.place_field(fx, FieldIdx::new(tag_field));
let niche = place.place_field(fx, tag_field);
let niche_type = fx.clif_type(niche.layout().ty).unwrap();
let niche_value = variant_index.as_u32() - niche_variants.start().as_u32();
let niche_value = (niche_value as u128).wrapping_add(niche_start);
Expand Down Expand Up @@ -118,7 +118,7 @@ pub(crate) fn codegen_get_discriminant<'tcx>(
let cast_to = fx.clif_type(dest_layout.ty).unwrap();

// Read the tag/niche-encoded discriminant from memory.
let tag = value.value_field(fx, FieldIdx::new(tag_field));
let tag = value.value_field(fx, tag_field);
let tag = tag.load_scalar(fx);

// Decode the discriminant (specifically if it's niche-encoded).
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::borrow::Cow;

use libc::c_uint;
use rustc_abi::{Align, Endian, Size, TagEncoding, VariantIdx, Variants};
use rustc_abi::{Align, Endian, FieldIdx, Size, TagEncoding, VariantIdx, Variants};
use rustc_codegen_ssa::debuginfo::type_names::compute_debuginfo_type_name;
use rustc_codegen_ssa::debuginfo::{tag_base_type, wants_c_like_enum_debuginfo};
use rustc_codegen_ssa::traits::{ConstCodegenMethods, MiscCodegenMethods};
Expand Down Expand Up @@ -401,7 +401,7 @@ fn build_union_fields_for_enum<'ll, 'tcx>(
enum_type_and_layout: TyAndLayout<'tcx>,
enum_type_di_node: &'ll DIType,
variant_indices: impl Iterator<Item = VariantIdx> + Clone,
tag_field: usize,
tag_field: FieldIdx,
untagged_variant_index: Option<VariantIdx>,
) -> SmallVec<&'ll DIType> {
let tag_base_type = tag_base_type(cx.tcx, enum_type_and_layout);
Expand Down Expand Up @@ -805,7 +805,7 @@ fn build_union_fields_for_direct_tag_enum_or_coroutine<'ll, 'tcx>(
variant_field_infos: &[VariantFieldInfo<'ll>],
discr_type_di_node: &'ll DIType,
tag_base_type: Ty<'tcx>,
tag_field: usize,
tag_field: FieldIdx,
untagged_variant_index: Option<VariantIdx>,
di_flags: DIFlags,
) -> SmallVec<&'ll DIType> {
Expand Down Expand Up @@ -858,7 +858,7 @@ fn build_union_fields_for_direct_tag_enum_or_coroutine<'ll, 'tcx>(
}));

assert_eq!(
cx.size_and_align_of(enum_type_and_layout.field(cx, tag_field).ty),
cx.size_and_align_of(enum_type_and_layout.field(cx, tag_field.as_usize()).ty),
cx.size_and_align_of(self::tag_base_type(cx.tcx, enum_type_and_layout))
);

Expand All @@ -875,7 +875,7 @@ fn build_union_fields_for_direct_tag_enum_or_coroutine<'ll, 'tcx>(
Endian::Big => (8, 0),
};

let tag_field_offset = enum_type_and_layout.fields.offset(tag_field).bytes();
let tag_field_offset = enum_type_and_layout.fields.offset(tag_field.as_usize()).bytes();
let lo_offset = Size::from_bytes(tag_field_offset + lo_offset);
let hi_offset = Size::from_bytes(tag_field_offset + hi_offset);

Expand Down Expand Up @@ -905,8 +905,8 @@ fn build_union_fields_for_direct_tag_enum_or_coroutine<'ll, 'tcx>(
cx,
enum_type_di_node,
TAG_FIELD_NAME,
enum_type_and_layout.field(cx, tag_field),
enum_type_and_layout.fields.offset(tag_field),
enum_type_and_layout.field(cx, tag_field.as_usize()),
enum_type_and_layout.fields.offset(tag_field.as_usize()),
di_flags,
tag_base_type_di_node,
None,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ fn build_discr_member_di_node<'ll, 'tcx>(
file,
UNKNOWN_LINE_NUMBER,
layout,
enum_or_coroutine_type_and_layout.fields.offset(tag_field),
enum_or_coroutine_type_and_layout.fields.offset(tag_field.as_usize()),
DIFlags::FlagArtificial,
ty,
))
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,10 +462,10 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
let tag_op = match self.val {
OperandValue::ZeroSized => bug!(),
OperandValue::Immediate(_) | OperandValue::Pair(_, _) => {
self.extract_field(fx, bx, tag_field)
self.extract_field(fx, bx, tag_field.as_usize())
}
OperandValue::Ref(place) => {
let tag = place.with_type(self.layout).project_field(bx, tag_field);
let tag = place.with_type(self.layout).project_field(bx, tag_field.as_usize());
bx.load_operand(tag)
}
};
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
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 ptr = self.project_field(bx, tag_field.as_usize());
let to =
self.layout.ty.discriminant_for_variant(bx.tcx(), variant_index).unwrap().val;
bx.store_to_place(
Expand All @@ -265,7 +265,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
..
} => {
if variant_index != untagged_variant {
let niche = self.project_field(bx, tag_field);
let niche = self.project_field(bx, tag_field.as_usize());
let niche_llty = bx.cx().immediate_backend_type(niche.layout);
let BackendRepr::Scalar(scalar) = niche.layout.backend_repr else {
bug!("expected a scalar placeref for the niche");
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_const_eval/src/interpret/discriminant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Functions for reading and writing discriminants of multi-variant layouts (enums and coroutines).

use rustc_abi::{self as abi, TagEncoding, VariantIdx, Variants};
use rustc_abi::{self as abi, FieldIdx, TagEncoding, VariantIdx, Variants};
use rustc_middle::ty::layout::{LayoutOf, PrimitiveExt, TyAndLayout};
use rustc_middle::ty::{self, CoroutineArgsExt, ScalarInt, Ty};
use rustc_middle::{mir, span_bug};
Expand All @@ -26,7 +26,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// No need to validate that the discriminant here because the
// `TyAndLayout::for_variant()` call earlier already checks the
// variant is valid.
let tag_dest = self.project_field(dest, tag_field)?;
let tag_dest = self.project_field(dest, tag_field.as_usize())?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

project_field should probably take FieldIdx... but that can be a future PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this hits the problem that a bunch of these things are also used for arrays, and I've never dug in enough to know whether it's actually ok to convert a bunch of these things over to FieldIdx -- if this is used for [u8; 1 << 32] then the capped-at-0xFFFFFF00 FieldIdx wouldn't work.

(Though of course usize already doesn't work today either for cross-compiling to a 64-bit target from a 32-bit host, but I suspect that nobody actually ever does that so nobody's cared about it being broken.)

Copy link
Member

@RalfJung RalfJung Jun 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

project_field should not be used for arrays, we have project_index for that. (Those used to be one function but I refactored that a long time ago.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, wait, InterpCx. I was thinking of https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.FieldsShape.html#method.offset and such.

If you say it'll work always here, I can give that a shot (in another PR).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you say it'll work always here

I think it should, but I can't be sure without just trying it myself.^^

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.FieldsShape.html#method.offset and such.

Yeah that should be cleaned up to separate field offsets from array element offsets.

self.write_scalar(tag, &tag_dest)
}
None => {
Expand Down Expand Up @@ -96,7 +96,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
let tag_layout = self.layout_of(tag_scalar_layout.primitive().to_int_ty(*self.tcx))?;

// Read tag and sanity-check `tag_layout`.
let tag_val = self.read_immediate(&self.project_field(op, tag_field)?)?;
let tag_val = self.read_immediate(&self.project_field(op, tag_field.as_usize())?)?;
assert_eq!(tag_layout.size, tag_val.layout.size);
assert_eq!(tag_layout.backend_repr.is_signed(), tag_val.layout.backend_repr.is_signed());
trace!("tag value: {}", tag_val);
Expand Down Expand Up @@ -231,7 +231,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
&self,
layout: TyAndLayout<'tcx>,
variant_index: VariantIdx,
) -> InterpResult<'tcx, Option<(ScalarInt, usize)>> {
) -> InterpResult<'tcx, Option<(ScalarInt, FieldIdx)>> {
// Layout computation excludes uninhabited variants from consideration.
// Therefore, there's no way to represent those variants in the given layout.
// Essentially, uninhabited variants do not have a tag that corresponds to their
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/validity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
// First, check if we are projecting to a variant.
match layout.variants {
Variants::Multiple { tag_field, .. } => {
if tag_field == field {
if tag_field.as_usize() == field {
return match layout.ty.kind() {
ty::Adt(def, ..) if def.is_enum() => PathElem::EnumTag,
ty::Coroutine(..) => PathElem::CoroutineTag,
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_middle/src/ty/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ where
.unwrap(),
),
Variants::Multiple { tag, tag_field, .. } => {
if i == tag_field {
if FieldIdx::from_usize(i) == tag_field {
return TyMaybeWithLayout::TyAndLayout(tag_layout(tag));
}
TyMaybeWithLayout::Ty(args.as_coroutine().prefix_tys()[i])
Expand Down Expand Up @@ -1060,8 +1060,10 @@ where
tag_field,
variants,
..
} if variants.len() == 2 && this.fields.offset(*tag_field) == offset => {
let tagged_variant = if untagged_variant.as_u32() == 0 {
} if variants.len() == 2
&& this.fields.offset(tag_field.as_usize()) == offset =>
{
let tagged_variant = if *untagged_variant == VariantIdx::ZERO {
VariantIdx::from_u32(1)
} else {
VariantIdx::from_u32(0)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_smir/src/rustc_smir/convert/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ impl<'tcx> Stable<'tcx> for rustc_abi::Variants<rustc_abi::FieldIdx, rustc_abi::
VariantsShape::Multiple {
tag: tag.stable(tables),
tag_encoding: tag_encoding.stable(tables),
tag_field: *tag_field,
tag_field: tag_field.stable(tables),
variants: variants.iter().as_slice().stable(tables),
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_transmute/src/layout/tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ pub(crate) mod rustc {

// For enums (but not coroutines), the tag field is
// currently always the first field of the layout.
assert_eq!(*tag_field, 0);
assert_eq!(*tag_field, FieldIdx::ZERO);

let variants = def.discriminants(cx.tcx()).try_fold(
Self::uninhabited(),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ty_utils/src/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -932,7 +932,7 @@ fn variant_info_for_coroutine<'tcx>(
// However, if the discriminant is placed past the end of the variant, then we need
// to factor in the size of the discriminant manually. This really should be refactored
// better, but this "works" for now.
if layout.fields.offset(tag_field) >= variant_size {
if layout.fields.offset(tag_field.as_usize()) >= variant_size {
variant_size += match tag_encoding {
TagEncoding::Direct => tag.size(cx),
_ => Size::ZERO,
Expand Down
Loading