Skip to content

Commit

Permalink
Disable dead variant removal for #[repr(C)] enums.
Browse files Browse the repository at this point in the history
  • Loading branch information
GoldsteinE committed Mar 25, 2024
1 parent af98101 commit eea0d6a
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 2 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_abi/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ pub trait LayoutCalculator {
let (present_first, present_second) = {
let mut present_variants = variants
.iter_enumerated()
.filter_map(|(i, v)| if absent(v) { None } else { Some(i) });
.filter_map(|(i, v)| if !repr.c() && absent(v) { None } else { Some(i) });
(present_variants.next(), present_variants.next())
};
let present_first = match present_first {
Expand Down Expand Up @@ -621,7 +621,7 @@ where
let discr_type = repr.discr_type();
let bits = Integer::from_attr(dl, discr_type).size().bits();
for (i, mut val) in discriminants {
if variants[i].iter().any(|f| f.abi.is_uninhabited()) {
if !repr.c() && variants[i].iter().any(|f| f.abi.is_uninhabited()) {
continue;
}
if discr_type.is_signed() {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1388,6 +1388,7 @@ pub enum Variants<FieldIdx: Idx, VariantIdx: Idx> {
/// Single enum variants, structs/tuples, unions, and all non-ADTs.
Single { index: VariantIdx },

// REVIEW: probably this wording has to be adjusted somehow?
/// Enum-likes with more than one inhabited variant: each variant comes with
/// a *discriminant* (usually the same as the variant index but the user can
/// assign explicit discriminant values). That discriminant is encoded
Expand Down
73 changes: 73 additions & 0 deletions tests/ui/repr/repr-c-dead-variants.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
//@ run-pass

#![allow(dead_code)]

use std::{alloc::Layout, ffi::c_int, mem::MaybeUninit, ptr};

// A simple uninhabited type.
enum Void {}

#[repr(C)]
enum Univariant<T> {
Variant(T),
}

#[repr(C, u8)]
enum UnivariantU8<T> {
Variant(T),
}

#[repr(C)]
enum TwoVariants<T> {
Variant1(T),
Variant2(u8),
}

#[repr(C, u8)]
enum TwoVariantsU8<T> {
Variant1(T),
Variant2(u8),
}

#[repr(C, u8)]
enum DeadBranchHasOtherField<T> {
Variant1(T, u64),
Variant2(u8),
}

macro_rules! assert_layout_eq {
($a:ty, $b:ty) => {
assert_eq!(Layout::new::<$a>(), Layout::new::<$b>());
};
}

fn main() {
// Compiler must not remove dead variants of `#[repr(C)]` ADTs.
assert_layout_eq!(Univariant<Void>, c_int);
// This should also hold for `#[repr(C, int)]` ADTs.
assert_layout_eq!(UnivariantU8<Void>, u8);
// And for ADTs with more than one variant.
// These are twice the size: a tag plus the field in a second branch.
assert_layout_eq!(TwoVariants<Void>, [c_int; 2]);
assert_layout_eq!(TwoVariantsU8<Void>, [u8; 2]);
// This one is 2 x u64: we reserve space for fields in a dead branch.
assert_layout_eq!(DeadBranchHasOtherField<Void>, [u64; 2]);

// Some other useful invariants. See this UCG thread for more context:
// https://github.com/rust-lang/unsafe-code-guidelines/issues/500
assert_layout_eq!(Univariant<Void>, Univariant<MaybeUninit<Void>>);
assert_layout_eq!(UnivariantU8<Void>, UnivariantU8<MaybeUninit<Void>>);
assert_layout_eq!(TwoVariants<Void>, TwoVariants<MaybeUninit<Void>>);
assert_layout_eq!(TwoVariantsU8<Void>, TwoVariantsU8<MaybeUninit<Void>>);
assert_layout_eq!(DeadBranchHasOtherField<Void>, DeadBranchHasOtherField<MaybeUninit<Void>>);

// Check that discriminants are allocated properly.
// SAFETY:
// 1. We checked that layout requires proper alignment.
// 2. Discriminant is guaranteed to be the first field of a `#[repr(C)]` enum.
unsafe {
assert_eq!(*ptr::from_ref(&TwoVariants::<Void>::Variant2(42)).cast::<c_int>(), 1);
assert_eq!(*ptr::from_ref(&TwoVariantsU8::<Void>::Variant2(42)).cast::<u8>(), 1);
assert_eq!(*ptr::from_ref(&DeadBranchHasOtherField::<Void>::Variant2(42)).cast::<u8>(), 1);
}
}

0 comments on commit eea0d6a

Please sign in to comment.