Skip to content

Fix repr(align) enum handling #96814

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 2 commits into from
Jul 6, 2022
+205 −4
Merged

Fix repr(align) enum handling #96814

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cedc428
fix the layout of repr(align) enums
RalfJung May 7, 2022
d5721ce
add asserts
RalfJung Jul 5, 2022
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
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/ty/layout.rs
View file
Original file line number Diff line number Diff line change
@@ -1418,9 +1418,9 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {

if layout_variants.iter().all(|v| v.abi.is_uninhabited()) {
abi = Abi::Uninhabited;
} else if tag.size(dl) == size || variants.iter().all(|layout| layout.is_empty()) {
// Without latter check aligned enums with custom discriminant values
// Would result in ICE see the issue #92464 for more info
} else 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 = Abi::Scalar(tag);
} else {
// Try to use a ScalarPair for all tagged enums.
12 changes: 11 additions & 1 deletion src/test/ui/aligned_enum_cast.rs
View file
Original file line number Diff line number Diff line change
@@ -11,5 +11,15 @@ enum Aligned {
fn main() {
let aligned = Aligned::Zero;
let fo = aligned as u8;
println!("foo {}",fo);
println!("foo {}", fo);
assert_eq!(fo, 0);
println!("{}", tou8(Aligned::Zero));
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be an assertion instead of a print? Otherwise we're only testing that this code runs, not that it does the right thing

Copy link
Member Author

Choose a reason for hiding this comment

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

I have added assertions but kept the prints, just in case they were relevant to trigger the ICE.

assert_eq!(tou8(Aligned::Zero), 0);
}

#[inline(never)]
fn tou8(al: Aligned) -> u8 {
// Cast behind a function call so ConstProp does not see it
// (so that we can test codegen).
al as u8
}
19 changes: 19 additions & 0 deletions src/test/ui/layout/issue-96185-overaligned-enum.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN"
#![crate_type = "lib"]
#![feature(rustc_attrs)]

// This cannot use `Scalar` abi since there is padding.
#[rustc_layout(debug)]
#[repr(align(8))]
pub enum Aligned1 { //~ ERROR: layout_of
Zero = 0,
One = 1,
}

// This should use `Scalar` abi.
#[rustc_layout(debug)]
#[repr(align(1))]
pub enum Aligned2 { //~ ERROR: layout_of
Zero = 0,
One = 1,
}
172 changes: 172 additions & 0 deletions src/test/ui/layout/issue-96185-overaligned-enum.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
error: layout_of(Aligned1) = Layout {
fields: Arbitrary {
offsets: [
Size(0 bytes),
],
memory_index: [
0,
],
},
variants: Multiple {
tag: Initialized {
value: Int(
I8,
false,
),
valid_range: 0..=1,
},
tag_encoding: Direct,
tag_field: 0,
variants: [
Layout {
fields: Arbitrary {
offsets: [],
memory_index: [],
},
variants: Single {
index: 0,
},
abi: Aggregate {
sized: true,
},
largest_niche: None,
align: AbiAndPrefAlign {
abi: Align(8 bytes),
pref: $PREF_ALIGN,
},
size: Size(8 bytes),
},
Layout {
fields: Arbitrary {
offsets: [],
memory_index: [],
},
variants: Single {
index: 1,
},
abi: Aggregate {
sized: true,
},
largest_niche: None,
align: AbiAndPrefAlign {
abi: Align(8 bytes),
pref: $PREF_ALIGN,
},
size: Size(8 bytes),
},
],
},
abi: Aggregate {
sized: true,
},
largest_niche: Some(
Niche {
offset: Size(0 bytes),
value: Int(
I8,
false,
),
valid_range: 0..=1,
},
),
align: AbiAndPrefAlign {
abi: Align(8 bytes),
pref: $PREF_ALIGN,
},
size: Size(8 bytes),
}
--> $DIR/issue-96185-overaligned-enum.rs:8:1
|
LL | pub enum Aligned1 {
| ^^^^^^^^^^^^^^^^^

error: layout_of(Aligned2) = Layout {
fields: Arbitrary {
offsets: [
Size(0 bytes),
],
memory_index: [
0,
],
},
variants: Multiple {
tag: Initialized {
value: Int(
I8,
false,
),
valid_range: 0..=1,
},
tag_encoding: Direct,
tag_field: 0,
variants: [
Layout {
fields: Arbitrary {
offsets: [],
memory_index: [],
},
variants: Single {
index: 0,
},
abi: Aggregate {
sized: true,
},
largest_niche: None,
align: AbiAndPrefAlign {
abi: Align(1 bytes),
pref: $PREF_ALIGN,
},
size: Size(1 bytes),
},
Layout {
fields: Arbitrary {
offsets: [],
memory_index: [],
},
variants: Single {
index: 1,
},
abi: Aggregate {
sized: true,
},
largest_niche: None,
align: AbiAndPrefAlign {
abi: Align(1 bytes),
pref: $PREF_ALIGN,
},
size: Size(1 bytes),
},
],
},
abi: Scalar(
Initialized {
value: Int(
I8,
false,
),
valid_range: 0..=1,
},
),
largest_niche: Some(
Niche {
offset: Size(0 bytes),
value: Int(
I8,
false,
),
valid_range: 0..=1,
},
),
align: AbiAndPrefAlign {
abi: Align(1 bytes),
pref: $PREF_ALIGN,
},
size: Size(1 bytes),
}
--> $DIR/issue-96185-overaligned-enum.rs:16:1
|
LL | pub enum Aligned2 {
| ^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors