Skip to content

Commit 2edf6c8

Browse files
workingjubileecr1901WaffleLapkin
committed
Default repr(C) enums to c_int size
This is what ISO C strongly implies this is correct, and many processor-specific ABIs imply or mandate this size, so "everyone" (LLVM, gcc...) defaults to emitting enums this way. However, this is by no means guaranteed by ISO C, and the bare-metal Arm targets show it can be overridden, which rustc supports via `c-enum-min-bits` in a target.json. The override is a flag named `-fshort-enums` in clang and gcc, but introducing a CLI flag is probably unnecessary for rustc. This flag can be used by non-Arm microcontroller targets, like AVR and MSP430, but it is not enabled for them by default. Rust programmers who know the size of a target's enums can use explicit reprs, which also lets them match C23 code. This change is most relevant to 16-bit targets: AVR and MSP430. Most of rustc's targets use 32-bit ints, but ILP64 does exist. Regardless, rustc should now correctly handle enums for both very small and very large targets. Thanks to William for confirming MSP430 behavior, and to Waffle for better style and no-core size_of asserts. Co-authored-by: William D. Jones <[email protected]> Co-authored-by: Waffle Maybe <[email protected]>
1 parent 4781233 commit 2edf6c8

13 files changed

+81
-25
lines changed

‎compiler/rustc_abi/src/lib.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,9 @@ pub struct TargetDataLayout {
170170

171171
pub instruction_address_space: AddressSpace,
172172

173-
/// Minimum size of #[repr(C)] enums (default I32 bits)
173+
/// Minimum size of #[repr(C)] enums (default c_int::BITS, usually 32)
174+
/// Note: This isn't in LLVM's data layout string, it is `short_enum`
175+
/// so the only valid spec for LLVM is c_int::BITS or 8
174176
pub c_enum_min_size: Integer,
175177
}
176178

‎compiler/rustc_target/src/spec/armebv7r_none_eabi.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub fn target() -> Target {
1919
max_atomic_width: Some(32),
2020
emit_debug_gdb_scripts: false,
2121
// GCC and Clang default to 8 for arm-none here
22-
c_enum_min_bits: 8,
22+
c_enum_min_bits: Some(8),
2323
..Default::default()
2424
},
2525
}

‎compiler/rustc_target/src/spec/armebv7r_none_eabihf.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub fn target() -> Target {
2020
max_atomic_width: Some(32),
2121
emit_debug_gdb_scripts: false,
2222
// GCC and Clang default to 8 for arm-none here
23-
c_enum_min_bits: 8,
23+
c_enum_min_bits: Some(8),
2424
..Default::default()
2525
},
2626
}

‎compiler/rustc_target/src/spec/armv4t_none_eabi.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ pub fn target() -> Target {
4949
// from thumb_base, rust-lang/rust#44993.
5050
emit_debug_gdb_scripts: false,
5151
// from thumb_base, apparently gcc/clang give enums a minimum of 8 bits on no-os targets
52-
c_enum_min_bits: 8,
52+
c_enum_min_bits: Some(8),
5353
..Default::default()
5454
},
5555
}

‎compiler/rustc_target/src/spec/armv7a_none_eabi.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ pub fn target() -> Target {
2727
max_atomic_width: Some(64),
2828
panic_strategy: PanicStrategy::Abort,
2929
emit_debug_gdb_scripts: false,
30-
c_enum_min_bits: 8,
30+
c_enum_min_bits: Some(8),
3131
..Default::default()
3232
};
3333
Target {

‎compiler/rustc_target/src/spec/armv7a_none_eabihf.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub fn target() -> Target {
1919
panic_strategy: PanicStrategy::Abort,
2020
emit_debug_gdb_scripts: false,
2121
// GCC and Clang default to 8 for arm-none here
22-
c_enum_min_bits: 8,
22+
c_enum_min_bits: Some(8),
2323
..Default::default()
2424
};
2525
Target {

‎compiler/rustc_target/src/spec/armv7r_none_eabi.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub fn target() -> Target {
1818
max_atomic_width: Some(32),
1919
emit_debug_gdb_scripts: false,
2020
// GCC and Clang default to 8 for arm-none here
21-
c_enum_min_bits: 8,
21+
c_enum_min_bits: Some(8),
2222
..Default::default()
2323
},
2424
}

‎compiler/rustc_target/src/spec/armv7r_none_eabihf.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub fn target() -> Target {
1919
max_atomic_width: Some(32),
2020
emit_debug_gdb_scripts: false,
2121
// GCC and Clang default to 8 for arm-none here
22-
c_enum_min_bits: 8,
22+
c_enum_min_bits: Some(8),
2323
..Default::default()
2424
},
2525
}

‎compiler/rustc_target/src/spec/hexagon_unknown_linux_musl.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ pub fn target() -> Target {
1111
base.has_rpath = true;
1212
base.linker_flavor = LinkerFlavor::Unix(Cc::Yes);
1313

14-
base.c_enum_min_bits = 8;
14+
base.c_enum_min_bits = Some(8);
1515

1616
Target {
1717
llvm_target: "hexagon-unknown-linux-musl".into(),

‎compiler/rustc_target/src/spec/mod.rs

+16-14
Original file line numberDiff line numberDiff line change
@@ -1344,10 +1344,18 @@ impl Target {
13441344
});
13451345
}
13461346

1347-
dl.c_enum_min_size = match Integer::from_size(Size::from_bits(self.c_enum_min_bits)) {
1348-
Ok(bits) => bits,
1349-
Err(err) => return Err(TargetDataLayoutErrors::InvalidBitsSize { err }),
1350-
};
1347+
dl.c_enum_min_size = self
1348+
.c_enum_min_bits
1349+
.map_or_else(
1350+
|| {
1351+
self.c_int_width
1352+
.parse()
1353+
.map_err(|_| String::from("failed to parse c_int_width"))
1354+
},
1355+
Ok,
1356+
)
1357+
.and_then(|i| Integer::from_size(Size::from_bits(i)))
1358+
.map_err(|err| TargetDataLayoutErrors::InvalidBitsSize { err })?;
13511359

13521360
Ok(dl)
13531361
}
@@ -1701,8 +1709,8 @@ pub struct TargetOptions {
17011709
/// If present it's a default value to use for adjusting the C ABI.
17021710
pub default_adjusted_cabi: Option<Abi>,
17031711

1704-
/// Minimum number of bits in #[repr(C)] enum. Defaults to 32.
1705-
pub c_enum_min_bits: u64,
1712+
/// Minimum number of bits in #[repr(C)] enum. Defaults to the size of c_int
1713+
pub c_enum_min_bits: Option<u64>,
17061714

17071715
/// Whether or not the DWARF `.debug_aranges` section should be generated.
17081716
pub generate_arange_section: bool,
@@ -1932,7 +1940,7 @@ impl Default for TargetOptions {
19321940
supported_split_debuginfo: Cow::Borrowed(&[SplitDebuginfo::Off]),
19331941
supported_sanitizers: SanitizerSet::empty(),
19341942
default_adjusted_cabi: None,
1935-
c_enum_min_bits: 32,
1943+
c_enum_min_bits: None,
19361944
generate_arange_section: true,
19371945
supports_stack_protector: true,
19381946
entry_name: "main".into(),
@@ -2118,12 +2126,6 @@ impl Target {
21182126
base.$key_name = s;
21192127
}
21202128
} );
2121-
($key_name:ident, u64) => ( {
2122-
let name = (stringify!($key_name)).replace("_", "-");
2123-
if let Some(s) = obj.remove(&name).and_then(|j| Json::as_u64(&j)) {
2124-
base.$key_name = s;
2125-
}
2126-
} );
21272129
($key_name:ident, u32) => ( {
21282130
let name = (stringify!($key_name)).replace("_", "-");
21292131
if let Some(s) = obj.remove(&name).and_then(|b| b.as_u64()) {
@@ -2492,6 +2494,7 @@ impl Target {
24922494

24932495
key!(is_builtin, bool);
24942496
key!(c_int_width = "target-c-int-width");
2497+
key!(c_enum_min_bits, Option<u64>); // if None, matches c_int_width
24952498
key!(os);
24962499
key!(env);
24972500
key!(abi);
@@ -2587,7 +2590,6 @@ impl Target {
25872590
key!(supported_split_debuginfo, falliable_list)?;
25882591
key!(supported_sanitizers, SanitizerSet)?;
25892592
key!(default_adjusted_cabi, Option<Abi>)?;
2590-
key!(c_enum_min_bits, u64);
25912593
key!(generate_arange_section, bool);
25922594
key!(supports_stack_protector, bool);
25932595
key!(entry_name);

‎compiler/rustc_target/src/spec/thumb_base.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ pub fn opts() -> TargetOptions {
5353
frame_pointer: FramePointer::Always,
5454
// ARM supports multiple ABIs for enums, the linux one matches the default of 32 here
5555
// but any arm-none or thumb-none target will be defaulted to 8 on GCC and clang
56-
c_enum_min_bits: 8,
56+
c_enum_min_bits: Some(8),
5757
..Default::default()
5858
}
5959
}

‎compiler/rustc_target/src/spec/thumbv4t_none_eabi.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pub fn target() -> Target {
5555
// suggested from thumb_base, rust-lang/rust#44993.
5656
emit_debug_gdb_scripts: false,
5757
// suggested from thumb_base, with no-os gcc/clang use 8-bit enums
58-
c_enum_min_bits: 8,
58+
c_enum_min_bits: Some(8),
5959
frame_pointer: FramePointer::MayOmit,
6060

6161
main_needs_argc_argv: false,

‎tests/ui/repr/16-bit-repr-c-enum.rs

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// build-pass
2+
// revisions: avr msp430
3+
//
4+
// [avr] needs-llvm-components: avr
5+
// [avr] compile-flags: --target=avr-unknown-gnu-atmega328 --crate-type=rlib
6+
// [msp430] needs-llvm-components: msp430
7+
// [msp430] compile-flags: --target=msp430-none-elf --crate-type=rlib
8+
#![feature(no_core, lang_items, intrinsics, staged_api)]
9+
#![no_core]
10+
#![crate_type = "lib"]
11+
#![stable(feature = "", since = "")]
12+
#![allow(dead_code)]
13+
14+
// Test that the repr(C) attribute doesn't break compilation
15+
// Previous bad assumption was that 32-bit enum default width is fine on msp430, avr
16+
// But the width of the C int on these platforms is 16 bits, and C enums <= C int range
17+
// so we want no more than that, usually. This resulted in errors like
18+
// "layout decided on a larger discriminant type (I32) than typeck (I16)"
19+
#[repr(C)]
20+
enum Foo {
21+
Bar,
22+
}
23+
24+
extern "rust-intrinsic" {
25+
#[stable(feature = "", since = "")]
26+
#[rustc_const_stable(feature = "", since = "")]
27+
#[rustc_safe_intrinsic]
28+
fn size_of<T>() -> usize;
29+
}
30+
31+
#[lang="sized"]
32+
trait Sized {}
33+
#[lang="copy"]
34+
trait Copy {}
35+
36+
const EXPECTED: usize = 2;
37+
const ACTUAL: usize = size_of::<Foo>();
38+
// Validate that the size is indeed 16 bits, to match this C static_assert:
39+
/**
40+
```c
41+
#include <assert.h>
42+
enum foo {
43+
BAR
44+
};
45+
int main(void)
46+
{
47+
/* passes on msp430-elf-gcc */
48+
static_assert(sizeof(enum foo) == 2);
49+
}
50+
```
51+
*/
52+
const _: [(); EXPECTED] = [(); ACTUAL];

0 commit comments

Comments
 (0)