Skip to content

Commit

Permalink
Auto merge of #99959 - cuviper:niche-size, r=eddyb
Browse files Browse the repository at this point in the history
Fix the size of niche enums with ZST alignment

For enums with an aligned ZST variant, like `[T; 0]`, the niche layout
was not computing a sufficient size to be consistent with alignment. Now
we pad that size up to the alignment, and also make sure to only use the
niche variant's ABI when the size and alignment still match.

Fixes #99836
r? `@eddyb`
  • Loading branch information
bors committed Jul 30, 2022
2 parents 0f4bcad + 1e7e745 commit a231865
Show file tree
Hide file tree
Showing 3 changed files with 477 additions and 2 deletions.
10 changes: 8 additions & 2 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1231,11 +1231,15 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
.collect::<Result<IndexVec<VariantIdx, _>, _>>()?;

let offset = st[i].fields().offset(field_index) + niche.offset;
let size = st[i].size();

// Align the total size to the largest alignment.
let size = st[i].size().align_to(align.abi);

let abi = if st.iter().all(|v| v.abi().is_uninhabited()) {
Abi::Uninhabited
} else {
} else if align == st[i].align() && size == st[i].size() {
// When the total alignment and size match, we can use the
// same ABI as the scalar variant with the reserved niche.
match st[i].abi() {
Abi::Scalar(_) => Abi::Scalar(niche_scalar),
Abi::ScalarPair(first, second) => {
Expand All @@ -1249,6 +1253,8 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
}
_ => Abi::Aggregate { sized: true },
}
} else {
Abi::Aggregate { sized: true }
};

let largest_niche = Niche::from_scalar(dl, offset, niche_scalar);
Expand Down
45 changes: 45 additions & 0 deletions src/test/ui/layout/zero-sized-array-enum-niche.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN"
#![feature(rustc_attrs)]
#![crate_type = "lib"]

// Various tests around the behavior of zero-sized arrays and
// enum niches, especially that they have coherent size and alignment.

// The original problem in #99836 came from ndarray's `TryFrom` for
// `SliceInfo<[SliceInfoElem; 0], Din, Dout>`, where that returns
// `Result<Self, ShapeError>` ~= `Result<AlignedZST, TypeWithNiche>`.
// This is a close enough approximation:
#[rustc_layout(debug)]
type AlignedResult = Result<[u32; 0], bool>; //~ ERROR: layout_of
// The bug gave that size 1 with align 4, but the size should also be 4.
// It was also using the bool niche for the enum tag, which is fine, but
// after the fix, layout decides to use a direct tagged repr instead.

// Here's another case with multiple ZST alignments, where we should
// get the maximal alignment and matching size.
#[rustc_layout(debug)]
enum MultipleAlignments { //~ ERROR: layout_of
Align2([u16; 0]),
Align4([u32; 0]),
Niche(bool),
}

// Tagged repr is clever enough to grow tags to fill any padding, e.g.:
// 1. `T_FF` (one byte of Tag, one byte of padding, two bytes of align=2 Field)
// -> `TTFF` (Tag has expanded to two bytes, i.e. like `#[repr(u16)]`)
// 2. `TFF` (one byte of Tag, two bytes of align=1 Field)
// -> Tag has no room to expand!
// (this outcome can be forced onto 1. by wrapping Field in `Packed<...>`)
#[repr(packed)]
struct Packed<T>(T);

#[rustc_layout(debug)]
type NicheLosesToTagged = Result<[u32; 0], Packed<std::num::NonZeroU16>>; //~ ERROR: layout_of
// Should get tag_encoding: Direct, size == align == 4.

#[repr(u16)]
enum U16IsZero { _Zero = 0 }

#[rustc_layout(debug)]
type NicheWinsOverTagged = Result<[u32; 0], Packed<U16IsZero>>; //~ ERROR: layout_of
// Should get tag_encoding: Niche, size == align == 4.
Loading

0 comments on commit a231865

Please sign in to comment.