Auto merge of #147017 - RalfJung:repr-c-big-discriminant, r=<try>

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

Author: rust-bors[bot]

compiler/rustc_abi/src/layout.rs

@@ -814,7 +814,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
         let (max, min) = largest_niche
             // We might have no inhabited variants, so pretend there's at least one.
             .unwrap_or((0, 0));
-        let (min_ity, signed) = discr_range_of_repr(min, max); //Integer::repr_discr(tcx, ty, &repr, min, max);
+        let (min_ity, signed) = discr_range_of_repr(min, max); //Integer::discr_range_of_repr(tcx, ty, &repr, min, max);
 
         let mut align = dl.aggregate_align;
         let mut max_repr_align = repr.align;

compiler/rustc_abi/src/lib.rs

@@ -183,6 +183,11 @@ impl ReprOptions {
 
     /// Returns the discriminant type, given these `repr` options.
     /// This must only be called on enums!
+    ///
+    /// This is the "typeck type" of the discriminant, which is effectively the maximum size:
+    /// discriminant values will be wrapped to fit (with a lint). Layout can later decide to use a
+    /// smaller type for the tag that stores the discriminant at runtime and that will work just
+    /// fine, it just induces casts when getting/setting the discriminant.
     pub fn discr_type(&self) -> IntegerType {
         self.int.unwrap_or(IntegerType::Pointer(true))
     }

compiler/rustc_hir_analysis/src/check/check.rs

@@ -784,7 +784,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,40 @@ 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 {
-                    span,
-                    discr: prev_discr.unwrap().to_string(),
-                    item_name: tcx.item_ident(variant.def_id),
-                    wrapped_discr: wrapped_discr.to_string(),
-                });
-                None
+                let mut d = tcx
+                    .dcx()
+                    .struct_span_err(span, "`repr(C)` enum discriminant does not fit into C `int`");
+                d.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");
+                d.emit();
             }
-            .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_EXTERNAL_PRIVATE_FIELDS,
         RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES,
         RUST_2021_INCOMPATIBLE_OR_PATTERNS,
@@ -5200,3 +5201,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

@@ -72,7 +72,10 @@ impl abi::Integer {
     /// signed discriminant range and `#[repr]` attribute.
     /// N.B.: `u128` values above `i128::MAX` will be treated as signed, but
     /// that shouldn't affect anything, other than maybe debuginfo.
-    fn repr_discr<'tcx>(
+    ///
+    /// This is the basis for computing the type of the *tag* of an enum (which can be smaller than
+    /// the type of the *discriminant*, which is determined by [`ReprOptions::discr_type`]).
+    fn discr_range_of_repr<'tcx>(
         tcx: TyCtxt<'tcx>,
         ty: Ty<'tcx>,
         repr: &ReprOptions,

compiler/rustc_ty_utils/src/layout.rs

@@ -613,8 +613,8 @@ fn layout_of_uncached<'tcx>(
             // UnsafeCell and UnsafePinned both disable niche optimizations
             let is_special_no_niche = def.is_unsafe_cell() || def.is_unsafe_pinned();
 
-            let get_discriminant_type =
-                |min, max| abi::Integer::repr_discr(tcx, ty, &def.repr(), min, max);
+            let discr_range_of_repr =
+                |min, max| abi::Integer::discr_range_of_repr(tcx, ty, &def.repr(), min, max);
 
             let discriminants_iter = || {
                 def.is_enum()
@@ -637,7 +637,7 @@ fn layout_of_uncached<'tcx>(
                     def.is_enum(),
                     is_special_no_niche,
                     tcx.layout_scalar_valid_range(def.did()),
-                    get_discriminant_type,
+                    discr_range_of_repr,
                     discriminants_iter(),
                     !maybe_unsized,
                 )
@@ -662,7 +662,7 @@ fn layout_of_uncached<'tcx>(
                     def.is_enum(),
                     is_special_no_niche,
                     tcx.layout_scalar_valid_range(def.did()),
-                    get_discriminant_type,
+                    discr_range_of_repr,
                     discriminants_iter(),
                     !maybe_unsized,
                 ) else {

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:23: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
+