Skip to content

Commit

Permalink
Auto merge of #130508 - adwinwhite:niche-not-depend-on-order, r=the8472
Browse files Browse the repository at this point in the history
Get rid of niche selection's dependence on fields's order

Fixes #125630.
Use the optimal niche selection decided in `univariant()` rather than picking niche field manually.

r? `@the8472`
  • Loading branch information
bors committed Sep 19, 2024
2 parents 749f80a + 937b09b commit 902f295
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 13 deletions.
13 changes: 4 additions & 9 deletions compiler/rustc_abi/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,15 +527,10 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
let count =
(niche_variants.end().index() as u128 - niche_variants.start().index() as u128) + 1;

// Find the field with the largest niche
let (field_index, niche, (niche_start, niche_scalar)) = variants[largest_variant_index]
.iter()
.enumerate()
.filter_map(|(j, field)| Some((j, field.largest_niche?)))
.max_by_key(|(_, niche)| niche.available(dl))
.and_then(|(j, niche)| Some((j, niche, niche.reserve(dl, count)?)))?;
let niche_offset =
niche.offset + variant_layouts[largest_variant_index].fields.offset(field_index);
// Use the largest niche in the largest variant.
let niche = variant_layouts[largest_variant_index].largest_niche?;
let (niche_start, niche_scalar) = niche.reserve(dl, count)?;
let niche_offset = niche.offset;
let niche_size = niche.value.size(dl);
let size = variant_layouts[largest_variant_index].size.align_to(align.abi);

Expand Down
5 changes: 3 additions & 2 deletions src/tools/miri/tests/fail/uninit/padding-enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ fn main() {
assert!(matches!(*p.as_ptr(), E::None));

// Turns out the discriminant is (currently) stored
// in the 2nd pointer, so the first half is padding.
// in the 1st pointer, so the second half is padding.
let c = &p as *const _ as *const u8;
let padding_offset = mem::size_of::<&'static ()>();
// Read a padding byte.
let _val = *c.add(0);
let _val = *c.add(padding_offset);
//~^ERROR: uninitialized
}
}
4 changes: 2 additions & 2 deletions src/tools/miri/tests/fail/uninit/padding-enum.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
--> tests/fail/uninit/padding-enum.rs:LL:CC
|
LL | let _val = *c.add(0);
| ^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
LL | let _val = *c.add(padding_offset);
| ^^^^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/structs-enums/type-sizes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,23 @@ struct ReorderEndNiche {
b: MiddleNiche4,
}

// We want that the niche selection doesn't depend on order of the fields. See issue #125630.
pub enum NicheFieldOrder1 {
A {
x: NonZero<u64>,
y: [NonZero<u64>; 2],
},
B([u64; 2]),
}

pub enum NicheFieldOrder2 {
A {
y: [NonZero<u64>; 2],
x: NonZero<u64>,
},
B([u64; 2]),
}


// standins for std types which we want to be laid out in a reasonable way
struct RawVecDummy {
Expand Down Expand Up @@ -260,6 +277,9 @@ pub fn main() {
size_of::<EnumWithMaybeUninhabitedVariant<()>>());
assert_eq!(size_of::<NicheFilledEnumWithAbsentVariant>(), size_of::<&'static ()>());

assert_eq!(size_of::<NicheFieldOrder1>(), 24);
assert_eq!(size_of::<NicheFieldOrder2>(), 24);

assert_eq!(size_of::<Option<Option<(bool, &())>>>(), size_of::<(bool, &())>());
assert_eq!(size_of::<Option<Option<(&(), bool)>>>(), size_of::<(bool, &())>());
assert_eq!(size_of::<Option<Option2<bool, &()>>>(), size_of::<(bool, &())>());
Expand Down

0 comments on commit 902f295

Please sign in to comment.