do not complain about enums where all discriminants fit into a c_uint

Author: RalfJung

compiler/rustc_hir_analysis/src/collect.rs

@@ -610,6 +610,10 @@ pub(super) fn lower_enum_variant_types(tcx: TyCtxt<'_>, def_id: LocalDefId) {
     let repr_type = def.repr().discr_type();
     let initial = repr_type.initial_discriminant(tcx);
     let mut prev_discr = None::<Discr<'_>>;
+    // Some of the logic below relies on `i128` being able to hold all c_int and c_uint values.
+    assert!(tcx.sess.target.c_int_width < 128);
+    let mut min_discr = i128::MAX;
+    let mut max_discr = i128::MIN;
 
     // fill the discriminant values and field types
     for variant in def.variants() {
@@ -631,19 +635,32 @@ pub(super) fn lower_enum_variant_types(tcx: TyCtxt<'_>, def_id: LocalDefId) {
         .unwrap_or(wrapped_discr);
 
         if def.repr().c() {
+            let c_int = Size::from_bits(tcx.sess.target.c_int_width);
+            let c_uint_max = i128::try_from(c_int.unsigned_int_max()).unwrap();
             // c_int is a signed type, so get a proper signed version of the discriminant
             let discr_size = cur_discr.ty.int_size_and_signed(tcx).0;
             let discr_val = discr_size.sign_extend(cur_discr.val);
+            min_discr = min_discr.min(discr_val);
+            max_discr = max_discr.max(discr_val);
 
-            let c_int = Size::from_bits(tcx.sess.target.c_int_width);
-            if discr_val < c_int.signed_int_min() || discr_val > c_int.signed_int_max() {
+            // The discriminant range must either fit into c_int or c_uint.
+            if !(min_discr >= c_int.signed_int_min() && max_discr <= c_int.signed_int_max())
+                && !(min_discr >= 0 && max_discr <= c_uint_max)
+            {
                 let span = tcx.def_span(variant.def_id);
+                let msg = if discr_val < c_int.signed_int_min() || discr_val > c_uint_max {
+                    "`repr(C)` enum discriminant does not fit into C `int` nor into C `unsigned int`"
+                } else if discr_val < 0 {
+                    "`repr(C)` enum discriminant does not fit into C `unsigned int`, and a previous discriminant does not fit into C `int`"
+                } else {
+                    "`repr(C)` enum discriminant does not fit into C `int`, and a previous discriminant does not fit into C `unsigned int`"
+                };
                 tcx.node_span_lint(
                     rustc_session::lint::builtin::REPR_C_ENUMS_LARGER_THAN_INT,
                     tcx.local_def_id_to_hir_id(def_id),
                     span,
                     |d| {
-                        d.primary_message("`repr(C)` enum discriminant does not fit into C `int`")
+                        d.primary_message(msg)
                         .note("`repr(C)` enums with big discriminants are non-portable, and their size in Rust might not match their size in C")
                         .help("use `repr($int_ty)` instead to explicitly set the size of this enum");
                     }

tests/ui/enum-discriminant/discriminant_size.rs

@@ -2,6 +2,7 @@
 #![feature(core_intrinsics)]
 
 use std::intrinsics::discriminant_value;
+use std::mem::size_of;
 
 enum E1 {
     A,
@@ -20,31 +21,53 @@ enum E3 {
     B = 100,
 }
 
+// Enums like this are found in the ecosystem, let's make sure they get the right size.
+#[repr(C)]
+#[allow(overflowing_literals)]
+enum UnsignedIntEnum {
+    A = 0,
+    O = 0xffffffff, // doesn't fit into `int`, but fits into `unsigned int`
+}
+
 #[repr(i128)]
 enum E4 {
     A = 0x1223_3445_5667_7889,
     B = -0x1223_3445_5667_7889,
 }
 
 fn main() {
+    assert_eq!(size_of::<E1>(), 1);
     let mut target: [isize; 3] = [0, 0, 0];
     target[1] = discriminant_value(&E1::A);
     assert_eq!(target, [0, 0, 0]);
     target[1] = discriminant_value(&E1::B);
     assert_eq!(target, [0, 1, 0]);
 
+    assert_eq!(size_of::<E2>(), 1);
     let mut target: [i8; 3] = [0, 0, 0];
     target[1] = discriminant_value(&E2::A);
     assert_eq!(target, [0, 7, 0]);
     target[1] = discriminant_value(&E2::B);
     assert_eq!(target, [0, -2, 0]);
 
+    // E3's size is target-dependent
     let mut target: [isize; 3] = [0, 0, 0];
     target[1] = discriminant_value(&E3::A);
     assert_eq!(target, [0, 42, 0]);
     target[1] = discriminant_value(&E3::B);
     assert_eq!(target, [0, 100, 0]);
 
+    #[allow(overflowing_literals)]
+    {
+        assert_eq!(size_of::<UnsignedIntEnum>(), 4);
+        let mut target: [isize; 3] = [0, -1, 0];
+        target[1] = discriminant_value(&UnsignedIntEnum::A);
+        assert_eq!(target, [0, 0, 0]);
+        target[1] = discriminant_value(&UnsignedIntEnum::O);
+        assert_eq!(target, [0, 0xffffffff as isize, 0]);
+    }
+
+    assert_eq!(size_of::<E4>(), 16);
     let mut target: [i128; 3] = [0, 0, 0];
     target[1] = discriminant_value(&E4::A);
     assert_eq!(target, [0, 0x1223_3445_5667_7889, 0]);

tests/ui/enum-discriminant/repr-c-big-discriminant1.ptr32.stderr

@@ -15,5 +15,21 @@ LL |     A = -2147483649, // i32::MIN-1
    |
    = note: the literal `-2147483649` does not fit into the type `isize` whose range is `-2147483648..=2147483647`
 
-error: aborting due to 2 previous errors
+error: literal out of range for `isize`
+  --> $DIR/repr-c-big-discriminant1.rs:32:9
+   |
+LL |     A = 2147483648, // i32::MAX+1
+   |         ^^^^^^^^^^
+   |
+   = note: the literal `2147483648` does not fit into the type `isize` whose range is `-2147483648..=2147483647`
+
+error: literal out of range for `isize`
+  --> $DIR/repr-c-big-discriminant1.rs:41:9
+   |
+LL |     A = 2147483648, // i32::MAX+1
+   |         ^^^^^^^^^^
+   |
+   = note: the literal `2147483648` does not fit into the type `isize` whose range is `-2147483648..=2147483647`
+
+error: aborting due to 4 previous errors
 

tests/ui/enum-discriminant/repr-c-big-discriminant1.ptr64.stderr

@@ -1,4 +1,4 @@
-error: `repr(C)` enum discriminant does not fit into C `int`
+error: `repr(C)` enum discriminant does not fit into C `int` nor into C `unsigned int`
   --> $DIR/repr-c-big-discriminant1.rs:16:5
    |
 LL |     A = 9223372036854775807, // i64::MAX
@@ -14,7 +14,7 @@ note: the lint level is defined here
 LL | #![deny(repr_c_enums_larger_than_int)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: `repr(C)` enum discriminant does not fit into C `int`
+error: `repr(C)` enum discriminant does not fit into C `int` nor into C `unsigned int`
   --> $DIR/repr-c-big-discriminant1.rs:24:5
    |
 LL |     A = -2147483649, // i32::MIN-1
@@ -25,9 +25,31 @@ LL |     A = -2147483649, // i32::MIN-1
    = note: `repr(C)` enums with big discriminants are non-portable, and their size in Rust might not match their size in C
    = help: use `repr($int_ty)` instead to explicitly set the size of this enum
 
-error: `repr(C)` enum discriminant does not fit into C `int`
+error: `repr(C)` enum discriminant does not fit into C `unsigned int`, and a previous discriminant does not fit into C `int`
   --> $DIR/repr-c-big-discriminant1.rs:34:5
    |
+LL |     B = -1,
+   |     ^
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #124403 <https://github.com/rust-lang/rust/issues/124403>
+   = note: `repr(C)` enums with big discriminants are non-portable, and their size in Rust might not match their size in C
+   = help: use `repr($int_ty)` instead to explicitly set the size of this enum
+
+error: `repr(C)` enum discriminant does not fit into C `int`, and a previous discriminant does not fit into C `unsigned int`
+  --> $DIR/repr-c-big-discriminant1.rs:41:5
+   |
+LL |     A = 2147483648, // i32::MAX+1
+   |     ^
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #124403 <https://github.com/rust-lang/rust/issues/124403>
+   = note: `repr(C)` enums with big discriminants are non-portable, and their size in Rust might not match their size in C
+   = help: use `repr($int_ty)` instead to explicitly set the size of this enum
+
+error: `repr(C)` enum discriminant does not fit into C `int` nor into C `unsigned int`
+  --> $DIR/repr-c-big-discriminant1.rs:51:5
+   |
 LL |     A = I64_MAX as isize,
    |     ^
    |
@@ -36,5 +58,5 @@ LL |     A = I64_MAX as isize,
    = note: `repr(C)` enums with big discriminants are non-portable, and their size in Rust might not match their size in C
    = help: use `repr($int_ty)` instead to explicitly set the size of this enum
 
-error: aborting due to 3 previous errors
+error: aborting due to 5 previous errors
 

tests/ui/enum-discriminant/repr-c-big-discriminant1.rs

@@ -15,26 +15,51 @@ use minicore::*;
 enum OverflowingEnum1 {
     A = 9223372036854775807, // i64::MAX
     //[ptr32]~^ ERROR: literal out of range
-    //[ptr64]~^^ ERROR: discriminant does not fit into C `int`
+    //[ptr64]~^^ ERROR: discriminant does not fit into C `int` nor into C `unsigned int`
     //[ptr64]~^^^ WARN: previously accepted
 }
 
 #[repr(C)]
 enum OverflowingEnum2 {
     A = -2147483649, // i32::MIN-1
     //[ptr32]~^ ERROR: literal out of range
-    //[ptr64]~^^ ERROR: discriminant does not fit into C `int`
+    //[ptr64]~^^ ERROR: discriminant does not fit into C `int` nor into C `unsigned int`
+    //[ptr64]~^^^ WARN: previously accepted
+}
+
+#[repr(C)]
+enum OverflowingEnum3a {
+    A = 2147483648, // i32::MAX+1
+    //[ptr32]~^ ERROR: literal out of range
+    B = -1,
+    //[ptr64]~^ ERROR: discriminant does not fit into C `unsigned int`, and a previous
+    //[ptr64]~^^ WARN: previously accepted
+}
+#[repr(C)]
+enum OverflowingEnum3b {
+    B = -1,
+    A = 2147483648, // i32::MAX+1
+    //[ptr32]~^ ERROR: literal out of range
+    //[ptr64]~^^ ERROR: discriminant does not fit into C `int`, and a previous
     //[ptr64]~^^^ WARN: previously accepted
 }
 
 const I64_MAX: i64 = 9223372036854775807;
 
 #[repr(C)]
-enum OverflowingEnum3 {
+enum OverflowingEnum4 {
     A = I64_MAX as isize,
-    //[ptr64]~^ ERROR: discriminant does not fit into C `int`
+    //[ptr64]~^ ERROR: discriminant does not fit into C `int` nor into C `unsigned int`
     //[ptr64]~^^ WARN: previously accepted
     // No warning/error on 32bit targets, but the `as isize` hints that wrapping is occurring.
 }
 
+// Enums like this are found in the ecosystem, let's make sure they get accepted.
+#[repr(C)]
+#[allow(overflowing_literals)]
+enum OkayEnum {
+    A = 0,
+    O = 0xffffffff,
+}
+
 fn main() {}

tests/ui/enum-discriminant/repr-c-big-discriminant2.ptr32.stderr

@@ -1,5 +1,5 @@
 error[E0370]: enum discriminant overflowed
-  --> $DIR/repr-c-big-discriminant2.rs:19:5
+  --> $DIR/repr-c-big-discriminant2.rs:22:5
    |
 LL |     B, // +1
    |     ^ overflowed on value after 2147483647

tests/ui/enum-discriminant/repr-c-big-discriminant2.ptr64.stderr

@@ -1,5 +1,5 @@
-error: `repr(C)` enum discriminant does not fit into C `int`
-  --> $DIR/repr-c-big-discriminant2.rs:19:5
+error: `repr(C)` enum discriminant does not fit into C `int`, and a previous discriminant does not fit into C `unsigned int`
+  --> $DIR/repr-c-big-discriminant2.rs:22:5
    |
 LL |     B, // +1
    |     ^

tests/ui/enum-discriminant/repr-c-big-discriminant2.rs

@@ -11,10 +11,13 @@
 extern crate minicore;
 use minicore::*;
 
-// Separate test since it suppresses other errors on ptr32
+// Separate test since it suppresses other errors on ptr32:
+// ensure we find the bad discriminant when it is implicitly computed by incrementing
+// the previous discriminant.
 
 #[repr(C)]
 enum OverflowingEnum {
+    NEG = -1,
     A = 2147483647, // i32::MAX
     B, // +1
     //[ptr32]~^ ERROR: enum discriminant overflowed