Skip to content

Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited) #136985

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

Merged
merged 6 commits into from
Feb 21, 2025
Merged
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: 0 additions & 2 deletions compiler/rustc_abi/src/callconv.rs
Original file line number Diff line number Diff line change
@@ -65,8 +65,6 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
Ty: TyAbiInterface<'a, C> + Copy,
{
match self.backend_repr {
BackendRepr::Uninhabited => Err(Heterogeneous),

// The primitive for this algorithm.
BackendRepr::Scalar(scalar) => {
let kind = match scalar.primitive() {
23 changes: 12 additions & 11 deletions compiler/rustc_abi/src/layout.rs
Original file line number Diff line number Diff line change
@@ -130,6 +130,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
},
backend_repr: BackendRepr::ScalarPair(a, b),
largest_niche,
uninhabited: false,
align,
size,
max_repr_align: None,
@@ -221,8 +222,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
LayoutData {
variants: Variants::Empty,
fields: FieldsShape::Primitive,
backend_repr: BackendRepr::Uninhabited,
backend_repr: BackendRepr::Memory { sized: true },
largest_niche: None,
uninhabited: true,
align: dl.i8_align,
size: Size::ZERO,
max_repr_align: None,
@@ -400,6 +402,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
fields: FieldsShape::Union(union_field_count),
backend_repr: abi,
largest_niche: None,
uninhabited: false,
align,
size: size.align_to(align.abi),
max_repr_align,
@@ -447,7 +450,6 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
Scalar::Union { .. } => {}
};
match &mut st.backend_repr {
BackendRepr::Uninhabited => {}
BackendRepr::Scalar(scalar) => hide_niches(scalar),
BackendRepr::ScalarPair(a, b) => {
hide_niches(a);
@@ -639,9 +641,8 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
let same_size = size == variant_layouts[largest_variant_index].size;
let same_align = align == variant_layouts[largest_variant_index].align;

let abi = if variant_layouts.iter().all(|v| v.is_uninhabited()) {
BackendRepr::Uninhabited
} else if same_size && same_align && others_zst {
let uninhabited = variant_layouts.iter().all(|v| v.is_uninhabited());
let abi = if same_size && same_align && others_zst {
match variant_layouts[largest_variant_index].backend_repr {
// When the total alignment and size match, we can use the
// same ABI as the scalar variant with the reserved niche.
@@ -683,6 +684,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
},
backend_repr: abi,
largest_niche,
uninhabited,
size,
align,
max_repr_align,
@@ -853,9 +855,8 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
};
let mut abi = BackendRepr::Memory { sized: true };

if layout_variants.iter().all(|v| v.is_uninhabited()) {
abi = BackendRepr::Uninhabited;
} else if tag.size(dl) == size {
let uninhabited = layout_variants.iter().all(|v| v.is_uninhabited());
if tag.size(dl) == size {
// Make sure we only use scalar layout when the enum is entirely its
// own tag (i.e. it has no padding nor any non-ZST variant fields).
abi = BackendRepr::Scalar(tag);
@@ -995,6 +996,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
memory_index: [0].into(),
},
largest_niche,
uninhabited,
backend_repr: abi,
align,
size,
@@ -1355,9 +1357,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
_ => {}
}
}
if fields.iter().any(|f| f.is_uninhabited()) {
abi = BackendRepr::Uninhabited;
}
let uninhabited = fields.iter().any(|f| f.is_uninhabited());

let unadjusted_abi_align = if repr.transparent() {
match layout_of_single_non_zst_field {
@@ -1378,6 +1378,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
fields: FieldsShape::Arbitrary { offsets, memory_index },
backend_repr: abi,
largest_niche,
uninhabited,
align,
size,
max_repr_align,
35 changes: 16 additions & 19 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1404,7 +1404,6 @@ impl AddressSpace {
#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
#[cfg_attr(feature = "nightly", derive(HashStable_Generic))]
pub enum BackendRepr {
Uninhabited,
Scalar(Scalar),
ScalarPair(Scalar, Scalar),
Vector {
@@ -1423,10 +1422,9 @@ impl BackendRepr {
#[inline]
pub fn is_unsized(&self) -> bool {
match *self {
BackendRepr::Uninhabited
| BackendRepr::Scalar(_)
| BackendRepr::ScalarPair(..)
| BackendRepr::Vector { .. } => false,
BackendRepr::Scalar(_) | BackendRepr::ScalarPair(..) | BackendRepr::Vector { .. } => {
false
}
BackendRepr::Memory { sized } => !sized,
}
}
@@ -1445,12 +1443,6 @@ impl BackendRepr {
}
}

/// Returns `true` if this is an uninhabited type
#[inline]
pub fn is_uninhabited(&self) -> bool {
matches!(*self, BackendRepr::Uninhabited)
}

/// Returns `true` if this is a scalar type
#[inline]
pub fn is_scalar(&self) -> bool {
@@ -1471,7 +1463,7 @@ impl BackendRepr {
BackendRepr::Vector { element, count } => {
cx.data_layout().vector_align(element.size(cx) * count)
}
BackendRepr::Uninhabited | BackendRepr::Memory { .. } => return None,
BackendRepr::Memory { .. } => return None,
})
}

@@ -1492,7 +1484,7 @@ impl BackendRepr {
// to make the size a multiple of align (e.g. for vectors of size 3).
(element.size(cx) * count).align_to(self.inherent_align(cx)?.abi)
}
BackendRepr::Uninhabited | BackendRepr::Memory { .. } => return None,
BackendRepr::Memory { .. } => return None,
})
}

@@ -1506,9 +1498,7 @@ impl BackendRepr {
BackendRepr::Vector { element, count } => {
BackendRepr::Vector { element: element.to_union(), count }
}
BackendRepr::Uninhabited | BackendRepr::Memory { .. } => {
BackendRepr::Memory { sized: true }
}
BackendRepr::Memory { .. } => BackendRepr::Memory { sized: true },
}
}

@@ -1704,6 +1694,11 @@ pub struct LayoutData<FieldIdx: Idx, VariantIdx: Idx> {
/// The leaf scalar with the largest number of invalid values
/// (i.e. outside of its `valid_range`), if it exists.
pub largest_niche: Option<Niche>,
/// Is this type known to be uninhabted?
///
/// This is separate from BackendRepr because uninhabited return types can affect ABI,
/// especially in the case of by-pointer struct returns, which allocate stack even when unused.
pub uninhabited: bool,

pub align: AbiAndPrefAlign,
pub size: Size,
@@ -1735,14 +1730,14 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
/// Returns `true` if this is an aggregate type (including a ScalarPair!)
pub fn is_aggregate(&self) -> bool {
match self.backend_repr {
BackendRepr::Uninhabited | BackendRepr::Scalar(_) | BackendRepr::Vector { .. } => false,
BackendRepr::Scalar(_) | BackendRepr::Vector { .. } => false,
BackendRepr::ScalarPair(..) | BackendRepr::Memory { .. } => true,
}
}

/// Returns `true` if this is an uninhabited type
pub fn is_uninhabited(&self) -> bool {
self.backend_repr.is_uninhabited()
self.uninhabited
}

pub fn scalar<C: HasDataLayout>(cx: &C, scalar: Scalar) -> Self {
@@ -1778,6 +1773,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
fields: FieldsShape::Primitive,
backend_repr: BackendRepr::Scalar(scalar),
largest_niche,
uninhabited: false,
size,
align,
max_repr_align: None,
@@ -1802,6 +1798,7 @@ where
backend_repr,
fields,
largest_niche,
uninhabited,
variants,
max_repr_align,
unadjusted_abi_align,
@@ -1813,6 +1810,7 @@ where
.field("abi", backend_repr)
.field("fields", fields)
.field("largest_niche", largest_niche)
.field("uninhabited", uninhabited)
.field("variants", variants)
.field("max_repr_align", max_repr_align)
.field("unadjusted_abi_align", unadjusted_abi_align)
@@ -1877,7 +1875,6 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
BackendRepr::Scalar(_) | BackendRepr::ScalarPair(..) | BackendRepr::Vector { .. } => {
false
}
BackendRepr::Uninhabited => self.size.bytes() == 0,
BackendRepr::Memory { sized } => sized && self.size.bytes() == 0,
}
}
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_cranelift/src/value_and_place.rs
Original file line number Diff line number Diff line change
@@ -638,9 +638,7 @@ impl<'tcx> CPlace<'tcx> {
}
CPlaceInner::Addr(_, Some(_)) => bug!("Can't write value to unsized place {:?}", self),
CPlaceInner::Addr(to_ptr, None) => {
if dst_layout.size == Size::ZERO
|| dst_layout.backend_repr == BackendRepr::Uninhabited
{
if dst_layout.size == Size::ZERO {
return;
}

2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/intrinsic/mod.rs
Original file line number Diff line number Diff line change
@@ -315,7 +315,7 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tc
let layout = self.layout_of(tp_ty).layout;
let _use_integer_compare = match layout.backend_repr() {
Scalar(_) | ScalarPair(_, _) => true,
Uninhabited | Vector { .. } => false,
Vector { .. } => false,
Memory { .. } => {
// For rusty ABIs, small aggregates are actually passed
// as `RegKind::Integer` (see `FnAbi::adjust_for_abi`),
13 changes: 5 additions & 8 deletions compiler/rustc_codegen_gcc/src/type_of.rs
Original file line number Diff line number Diff line change
@@ -84,7 +84,7 @@ fn uncached_gcc_type<'gcc, 'tcx>(
false,
);
}
BackendRepr::Uninhabited | BackendRepr::Memory { .. } => {}
BackendRepr::Memory { .. } => {}
}

let name = match *layout.ty.kind() {
@@ -179,19 +179,16 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {
fn is_gcc_immediate(&self) -> bool {
match self.backend_repr {
BackendRepr::Scalar(_) | BackendRepr::Vector { .. } => true,
BackendRepr::ScalarPair(..) | BackendRepr::Uninhabited | BackendRepr::Memory { .. } => {
false
}
BackendRepr::ScalarPair(..) | BackendRepr::Memory { .. } => false,
}
}

fn is_gcc_scalar_pair(&self) -> bool {
match self.backend_repr {
BackendRepr::ScalarPair(..) => true,
BackendRepr::Uninhabited
| BackendRepr::Scalar(_)
| BackendRepr::Vector { .. }
| BackendRepr::Memory { .. } => false,
BackendRepr::Scalar(_) | BackendRepr::Vector { .. } | BackendRepr::Memory { .. } => {
false
}
}
}

2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/intrinsic.rs
Original file line number Diff line number Diff line change
@@ -476,7 +476,7 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
let layout = self.layout_of(tp_ty).layout;
let use_integer_compare = match layout.backend_repr() {
Scalar(_) | ScalarPair(_, _) => true,
Uninhabited | Vector { .. } => false,
Vector { .. } => false,
Memory { .. } => {
// For rusty ABIs, small aggregates are actually passed
// as `RegKind::Integer` (see `FnAbi::adjust_for_abi`),
13 changes: 5 additions & 8 deletions compiler/rustc_codegen_llvm/src/type_of.rs
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@ fn uncached_llvm_type<'a, 'tcx>(
let element = layout.scalar_llvm_type_at(cx, element);
return cx.type_vector(element, count);
}
BackendRepr::Uninhabited | BackendRepr::Memory { .. } | BackendRepr::ScalarPair(..) => {}
BackendRepr::Memory { .. } | BackendRepr::ScalarPair(..) => {}
}

let name = match layout.ty.kind() {
@@ -172,19 +172,16 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> {
fn is_llvm_immediate(&self) -> bool {
match self.backend_repr {
BackendRepr::Scalar(_) | BackendRepr::Vector { .. } => true,
BackendRepr::ScalarPair(..) | BackendRepr::Uninhabited | BackendRepr::Memory { .. } => {
false
}
BackendRepr::ScalarPair(..) | BackendRepr::Memory { .. } => false,
}
}

fn is_llvm_scalar_pair(&self) -> bool {
match self.backend_repr {
BackendRepr::ScalarPair(..) => true,
BackendRepr::Uninhabited
| BackendRepr::Scalar(_)
| BackendRepr::Vector { .. }
| BackendRepr::Memory { .. } => false,
BackendRepr::Scalar(_) | BackendRepr::Vector { .. } | BackendRepr::Memory { .. } => {
false
}
}
}

28 changes: 7 additions & 21 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
@@ -4,9 +4,7 @@ use rustc_abi::{BackendRepr, ExternAbi, HasDataLayout, Reg, WrappingRange};
use rustc_ast as ast;
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
use rustc_hir::lang_items::LangItem;
use rustc_middle::mir::{
self, AssertKind, BasicBlock, InlineAsmMacro, SwitchTargets, UnwindTerminateReason,
};
use rustc_middle::mir::{self, AssertKind, InlineAsmMacro, SwitchTargets, UnwindTerminateReason};
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, ValidityRequirement};
use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
use rustc_middle::ty::{self, Instance, Ty};
@@ -942,7 +940,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
&fn_abi.ret,
&mut llargs,
Some(intrinsic),
target,
);
let dest = match ret_dest {
_ if fn_abi.ret.is_indirect() => llargs[0],
@@ -998,19 +995,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
};

let mut llargs = Vec::with_capacity(arg_count);
let destination = target.as_ref().map(|&target| {
(
self.make_return_dest(
bx,
destination,
&fn_abi.ret,
&mut llargs,
None,
Some(target),
),
target,
)
});

// We still need to call `make_return_dest` even if there's no `target`, since
// `fn_abi.ret` could be `PassMode::Indirect`, even if it is uninhabited,
// and `make_return_dest` adds the return-place indirect pointer to `llargs`.
let return_dest = self.make_return_dest(bx, destination, &fn_abi.ret, &mut llargs, None);
let destination = target.map(|target| (return_dest, target));

// Split the rust-call tupled arguments off.
let (first_args, untuple) = if abi == ExternAbi::RustCall && !args.is_empty() {
@@ -1813,11 +1803,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
fn_ret: &ArgAbi<'tcx, Ty<'tcx>>,
llargs: &mut Vec<Bx::Value>,
intrinsic: Option<ty::IntrinsicDef>,
target: Option<BasicBlock>,
) -> ReturnDest<'tcx, Bx::Value> {
if target.is_none() {
return ReturnDest::Nothing;
}
// If the return is ignored, we can just return a do-nothing `ReturnDest`.
if fn_ret.is_ignore() {
return ReturnDest::Nothing;
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
@@ -422,9 +422,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
See <https://github.com/rust-lang/rust/issues/137108>",
);
}
BackendRepr::Uninhabited
| BackendRepr::ScalarPair(_, _)
| BackendRepr::Memory { sized: false } => bug!(),
BackendRepr::ScalarPair(_, _) | BackendRepr::Memory { sized: false } => bug!(),
})
};

2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
@@ -385,7 +385,7 @@ impl<'tcx, Prov: Provenance> ImmTy<'tcx, Prov> {
(Immediate::Uninit, _) => Immediate::Uninit,
// If the field is uninhabited, we can forget the data (can happen in ConstProp).
// `enum S { A(!), B, C }` is an example of an enum with Scalar layout that
// has an `Uninhabited` variant, which means this case is possible.
// has an uninhabited variant, which means this case is possible.
_ if layout.is_uninhabited() => Immediate::Uninit,
// the field contains no information, can be left uninit
// (Scalar/ScalarPair can contain even aligned ZST, not just 1-ZST)
Loading