FCW for repr(C) enums whose discriminant values do not fit into a c_int

Author: RalfJung

compiler/rustc_hir_analysis/src/check/check.rs

@@ -782,7 +782,7 @@ pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(),
             tcx.ensure_ok().generics_of(def_id);
             tcx.ensure_ok().type_of(def_id);
             tcx.ensure_ok().predicates_of(def_id);
-            crate::collect::lower_enum_variant_types(tcx, def_id.to_def_id());
+            crate::collect::lower_enum_variant_types(tcx, def_id);
             check_enum(tcx, def_id);
             check_variances_for_type_defn(tcx, def_id);
         }

compiler/rustc_hir_analysis/src/collect.rs

@@ -19,7 +19,7 @@ use std::cell::Cell;
 use std::iter;
 use std::ops::Bound;
 
-use rustc_abi::ExternAbi;
+use rustc_abi::{ExternAbi, Size};
 use rustc_ast::Recovered;
 use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
 use rustc_data_structures::unord::UnordMap;
@@ -605,7 +605,7 @@ pub(super) fn lower_variant_ctor(tcx: TyCtxt<'_>, def_id: LocalDefId) {
     tcx.ensure_ok().predicates_of(def_id);
 }
 
-pub(super) fn lower_enum_variant_types(tcx: TyCtxt<'_>, def_id: DefId) {
+pub(super) fn lower_enum_variant_types(tcx: TyCtxt<'_>, def_id: LocalDefId) {
     let def = tcx.adt_def(def_id);
     let repr_type = def.repr().discr_type();
     let initial = repr_type.initial_discriminant(tcx);
@@ -614,23 +614,44 @@ pub(super) fn lower_enum_variant_types(tcx: TyCtxt<'_>, def_id: DefId) {
     // fill the discriminant values and field types
     for variant in def.variants() {
         let wrapped_discr = prev_discr.map_or(initial, |d| d.wrap_incr(tcx));
-        prev_discr = Some(
-            if let ty::VariantDiscr::Explicit(const_def_id) = variant.discr {
-                def.eval_explicit_discr(tcx, const_def_id).ok()
-            } else if let Some(discr) = repr_type.disr_incr(tcx, prev_discr) {
-                Some(discr)
-            } else {
+        let cur_discr = if let ty::VariantDiscr::Explicit(const_def_id) = variant.discr {
+            def.eval_explicit_discr(tcx, const_def_id).ok()
+        } else if let Some(discr) = repr_type.disr_incr(tcx, prev_discr) {
+            Some(discr)
+        } else {
+            let span = tcx.def_span(variant.def_id);
+            tcx.dcx().emit_err(errors::EnumDiscriminantOverflowed {
+                span,
+                discr: prev_discr.unwrap().to_string(),
+                item_name: tcx.item_ident(variant.def_id),
+                wrapped_discr: wrapped_discr.to_string(),
+            });
+            None
+        }
+        .unwrap_or(wrapped_discr);
+
+        if def.repr().c() {
+            // 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);
+
+            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() {
                 let span = tcx.def_span(variant.def_id);
-                tcx.dcx().emit_err(errors::EnumDiscriminantOverflowed {
+                tcx.node_span_lint(
+                    rustc_session::lint::builtin::REPR_C_ENUMS_LARGER_THAN_INT,
+                    tcx.local_def_id_to_hir_id(def_id),
                     span,
-                    discr: prev_discr.unwrap().to_string(),
-                    item_name: tcx.item_ident(variant.def_id),
-                    wrapped_discr: wrapped_discr.to_string(),
-                });
-                None
+                    |d| {
+                        d.primary_message("`repr(C)` enum discriminant does not fit into C `int`")
+                        .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");
+                    }
+                );
             }
-            .unwrap_or(wrapped_discr),
-        );
+        }
+
+        prev_discr = Some(cur_discr);
 
         for f in &variant.fields {
             tcx.ensure_ok().generics_of(f.did);

compiler/rustc_lint/src/types.rs

@@ -10,7 +10,7 @@ use rustc_span::{Span, Symbol, sym};
 use tracing::debug;
 use {rustc_ast as ast, rustc_hir as hir};
 
-mod improper_ctypes; // these filed do the implementation for ImproperCTypesDefinitions,ImproperCTypesDeclarations
+mod improper_ctypes; // these files do the implementation for ImproperCTypesDefinitions,ImproperCTypesDeclarations
 pub(crate) use improper_ctypes::ImproperCTypesLint;
 
 use crate::lints::{
@@ -25,7 +25,6 @@ use crate::lints::{
 use crate::{LateContext, LateLintPass, LintContext};
 
 mod literal;
-
 use literal::{int_ty_range, lint_literal, uint_ty_range};
 
 declare_lint! {

compiler/rustc_lint_defs/src/builtin.rs

@@ -86,6 +86,7 @@ declare_lint_pass! {
         REFINING_IMPL_TRAIT_INTERNAL,
         REFINING_IMPL_TRAIT_REACHABLE,
         RENAMED_AND_REMOVED_LINTS,
+        REPR_C_ENUMS_LARGER_THAN_INT,
         REPR_TRANSPARENT_NON_ZST_FIELDS,
         RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES,
         RUST_2021_INCOMPATIBLE_OR_PATTERNS,
@@ -5213,3 +5214,50 @@ declare_lint! {
     Warn,
     r#"detects when a function annotated with `#[inline(always)]` and `#[target_feature(enable = "..")]` is inlined into a caller without the required target feature"#,
 }
+
+declare_lint! {
+    /// The `repr_c_enums_larger_than_int` lint detects `repr(C)` enums with discriminant
+    /// values that do not fit into a C `int`.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,ignore (only errors on 64bit)
+    /// #[repr(C)]
+    /// enum E {
+    ///     V = 9223372036854775807, // i64::MAX
+    /// }
+    /// ```
+    ///
+    /// This will produce:
+    ///
+    /// ```text
+    /// error: `repr(C)` enum discriminant does not fit into C `int`
+    ///   --> $DIR/repr-c-big-discriminant1.rs:16:5
+    ///    |
+    /// LL |     A = 9223372036854775807, // i64::MAX
+    ///    |     ^
+    ///    |
+    ///    = 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
+    /// ```
+    ///
+    /// ### Explanation
+    ///
+    /// In C, enums with discriminants that do not fit into an `int` are a portability hazard: such
+    /// enums are only permitted since C23, and not supported e.g. by MSVC. Furthermore, Rust
+    /// interprets the discriminant values of `repr(C)` enums as expressions of type `isize`, which
+    /// cannot be changed in a backwards-compatible way. If the discriminant is given as a literal
+    /// that does not fit into `isize`, it is wrapped (with a warning). This makes it impossible to
+    /// implement the C23 behavior of enums where the enum discriminants have no predefined type and
+    /// instead the enum uses a type large enough to hold all discriminants.
+    ///
+    /// Therefore, `repr(C)` enums require all discriminants to fit into a C `int`.
+    pub REPR_C_ENUMS_LARGER_THAN_INT,
+    Warn,
+    "repr(C) enums with discriminant values that do not fit into a C int",
+    @future_incompatible = FutureIncompatibleInfo {
+        reason: FutureIncompatibilityReason::FutureReleaseError,
+        reference: "issue #124403 <https://github.com/rust-lang/rust/issues/124403>",
+        report_in_deps: false,
+    };
+}

compiler/rustc_middle/src/ty/layout.rs

@@ -111,7 +111,8 @@ impl abi::Integer {
             abi::Integer::I8
         };
 
-        // Pick the smallest fit.
+        // Pick the smallest fit. Prefer unsigned; that matches clang in cases where this makes a
+        // difference (https://godbolt.org/z/h4xEasW1d) so it is crucial for repr(C).
         if unsigned_fit <= signed_fit {
             (cmp::max(unsigned_fit, at_least), false)
         } else {

compiler/rustc_ty_utils/src/layout/invariant.rs

@@ -14,6 +14,8 @@ pub(super) fn layout_sanity_check<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayou
     if layout.size.bytes() >= tcx.data_layout.obj_size_bound() {
         bug!("size is too large, in the following layout:\n{layout:#?}");
     }
+    // FIXME(#124403): Once `repr_c_enums_larger_than_int` is a hard error, we could assert
+    // here that a repr(c) enum discriminant is never larger than a c_int.
 
     if !cfg!(debug_assertions) {
         // Stop here, the rest is kind of expensive.

tests/auxiliary/minicore.rs

@@ -177,6 +177,21 @@ impl Add<isize> for isize {
     }
 }
 
+#[lang = "neg"]
+pub trait Neg {
+    type Output;
+
+    fn neg(self) -> Self::Output;
+}
+
+impl Neg for isize {
+    type Output = isize;
+
+    fn neg(self) -> isize {
+        loop {} // Dummy impl, not actually used
+    }
+}
+
 #[lang = "sync"]
 trait Sync {}
 impl_marker_trait!(
@@ -231,6 +246,13 @@ pub mod mem {
     #[rustc_nounwind]
     #[rustc_intrinsic]
     pub unsafe fn transmute<Src, Dst>(src: Src) -> Dst;
+
+    #[rustc_nounwind]
+    #[rustc_intrinsic]
+    pub const fn size_of<T>() -> usize;
+    #[rustc_nounwind]
+    #[rustc_intrinsic]
+    pub const fn align_of<T>() -> usize;
 }
 
 #[lang = "c_void"]

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

@@ -0,0 +1,19 @@
+error: literal out of range for `isize`
+  --> $DIR/repr-c-big-discriminant1.rs:16:9
+   |
+LL |     A = 9223372036854775807, // i64::MAX
+   |         ^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the literal `9223372036854775807` does not fit into the type `isize` whose range is `-2147483648..=2147483647`
+   = note: `#[deny(overflowing_literals)]` on by default
+
+error: literal out of range for `isize`
+  --> $DIR/repr-c-big-discriminant1.rs:24:9
+   |
+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
+

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

@@ -0,0 +1,40 @@
+error: `repr(C)` enum discriminant does not fit into C `int`
+  --> $DIR/repr-c-big-discriminant1.rs:16:5
+   |
+LL |     A = 9223372036854775807, // i64::MAX
+   |     ^
+   |
+   = 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
+note: the lint level is defined here
+  --> $DIR/repr-c-big-discriminant1.rs:6:9
+   |
+LL | #![deny(repr_c_enums_larger_than_int)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: `repr(C)` enum discriminant does not fit into C `int`
+  --> $DIR/repr-c-big-discriminant1.rs:24:5
+   |
+LL |     A = -2147483649, // i32::MIN-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`
+  --> $DIR/repr-c-big-discriminant1.rs:34:5
+   |
+LL |     A = I64_MAX as isize,
+   |     ^
+   |
+   = 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: aborting due to 3 previous errors
+

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

@@ -0,0 +1,40 @@
+//@ revisions: ptr32 ptr64
+//@[ptr32] compile-flags: --target i686-unknown-linux-gnu
+//@[ptr32] needs-llvm-components: x86
+//@[ptr64] compile-flags: --target x86_64-unknown-linux-gnu
+//@[ptr64] needs-llvm-components: x86
+#![deny(repr_c_enums_larger_than_int)]
+
+//@ add-minicore
+#![feature(no_core)]
+#![no_core]
+extern crate minicore;
+use minicore::*;
+
+#[repr(C)]
+enum OverflowingEnum1 {
+    A = 9223372036854775807, // i64::MAX
+    //[ptr32]~^ ERROR: literal out of range
+    //[ptr64]~^^ ERROR: discriminant does not fit into C `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]~^^^ WARN: previously accepted
+}
+
+const I64_MAX: i64 = 9223372036854775807;
+
+#[repr(C)]
+enum OverflowingEnum3 {
+    A = I64_MAX as isize,
+    //[ptr64]~^ ERROR: discriminant does not fit into C `int`
+    //[ptr64]~^^ WARN: previously accepted
+    // No warning/error on 32bit targets, but the `as isize` hints that wrapping is occurring.
+}
+
+fn main() {}