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

@@ -812,7 +812,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,32 +605,64 @@ 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);
     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() {
         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() {
+            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);
+
+            // 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);
-                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 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`"
+                };
+                let mut d = tcx.dcx().struct_span_err(span, msg);
+                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,
@@ -5197,3 +5198,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/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]);