Rollup merge of #144390 - oli-obk:arbitrary-enum-discrs, r=SparrowLii

Remove dead code and extend test coverage and diagnostics around it

I was staring a bit at the `dont_niche_optimize_enum` variable and figured out that part of it is dead code (at least today it is). I changed the diagnostic and test around the code that makes that part dead code, so everything that makes removing that code sound is visible in this PR

Author: matthiaskrgr

compiler/rustc_abi/src/layout.rs

@@ -313,7 +313,6 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
         scalar_valid_range: (Bound<u128>, Bound<u128>),
         discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool),
         discriminants: impl Iterator<Item = (VariantIdx, i128)>,
-        dont_niche_optimize_enum: bool,
         always_sized: bool,
     ) -> LayoutCalculatorResult<FieldIdx, VariantIdx, F> {
         let (present_first, present_second) = {
@@ -352,13 +351,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
             // structs. (We have also handled univariant enums
             // that allow representation optimization.)
             assert!(is_enum);
-            self.layout_of_enum(
-                repr,
-                variants,
-                discr_range_of_repr,
-                discriminants,
-                dont_niche_optimize_enum,
-            )
+            self.layout_of_enum(repr, variants, discr_range_of_repr, discriminants)
         }
     }
 
@@ -599,7 +592,6 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
         variants: &IndexSlice<VariantIdx, IndexVec<FieldIdx, F>>,
         discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool),
         discriminants: impl Iterator<Item = (VariantIdx, i128)>,
-        dont_niche_optimize_enum: bool,
     ) -> LayoutCalculatorResult<FieldIdx, VariantIdx, F> {
         // Until we've decided whether to use the tagged or
         // niche filling LayoutData, we don't want to intern the
@@ -618,7 +610,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
         }
 
         let calculate_niche_filling_layout = || -> Option<TmpLayout<FieldIdx, VariantIdx>> {
-            if dont_niche_optimize_enum {
+            if repr.inhibit_enum_layout_opt() {
                 return None;
             }
 

compiler/rustc_hir_analysis/src/check/check.rs

@@ -1642,20 +1642,40 @@ fn check_enum(tcx: TyCtxt<'_>, def_id: LocalDefId) {
 
     if def.repr().int.is_none() {
         let is_unit = |var: &ty::VariantDef| matches!(var.ctor_kind(), Some(CtorKind::Const));
-        let has_disr = |var: &ty::VariantDef| matches!(var.discr, ty::VariantDiscr::Explicit(_));
+        let get_disr = |var: &ty::VariantDef| match var.discr {
+            ty::VariantDiscr::Explicit(disr) => Some(disr),
+            ty::VariantDiscr::Relative(_) => None,
+        };
 
-        let has_non_units = def.variants().iter().any(|var| !is_unit(var));
-        let disr_units = def.variants().iter().any(|var| is_unit(var) && has_disr(var));
-        let disr_non_unit = def.variants().iter().any(|var| !is_unit(var) && has_disr(var));
+        let non_unit = def.variants().iter().find(|var| !is_unit(var));
+        let disr_unit =
+            def.variants().iter().filter(|var| is_unit(var)).find_map(|var| get_disr(var));
+        let disr_non_unit =
+            def.variants().iter().filter(|var| !is_unit(var)).find_map(|var| get_disr(var));
 
-        if disr_non_unit || (disr_units && has_non_units) {
-            struct_span_code_err!(
+        if disr_non_unit.is_some() || (disr_unit.is_some() && non_unit.is_some()) {
+            let mut err = struct_span_code_err!(
                 tcx.dcx(),
                 tcx.def_span(def_id),
                 E0732,
-                "`#[repr(inttype)]` must be specified"
-            )
-            .emit();
+                "`#[repr(inttype)]` must be specified for enums with explicit discriminants and non-unit variants"
+            );
+            if let Some(disr_non_unit) = disr_non_unit {
+                err.span_label(
+                    tcx.def_span(disr_non_unit),
+                    "explicit discriminant on non-unit variant specified here",
+                );
+            } else {
+                err.span_label(
+                    tcx.def_span(disr_unit.unwrap()),
+                    "explicit discriminant specified here",
+                );
+                err.span_label(
+                    tcx.def_span(non_unit.unwrap().def_id),
+                    "non-unit discriminant declared here",
+                );
+            }
+            err.emit();
         }
     }
 

compiler/rustc_ty_utils/src/layout.rs

@@ -603,12 +603,6 @@ fn layout_of_uncached<'tcx>(
                     .flatten()
             };
 
-            let dont_niche_optimize_enum = def.repr().inhibit_enum_layout_opt()
-                || def
-                    .variants()
-                    .iter_enumerated()
-                    .any(|(i, v)| v.discr != ty::VariantDiscr::Relative(i.as_u32()));
-
             let maybe_unsized = def.is_struct()
                 && def.non_enum_variant().tail_opt().is_some_and(|last_field| {
                     let typing_env = ty::TypingEnv::post_analysis(tcx, def.did());
@@ -625,7 +619,6 @@ fn layout_of_uncached<'tcx>(
                     tcx.layout_scalar_valid_range(def.did()),
                     get_discriminant_type,
                     discriminants_iter(),
-                    dont_niche_optimize_enum,
                     !maybe_unsized,
                 )
                 .map_err(|err| map_error(cx, ty, err))?;
@@ -651,7 +644,6 @@ fn layout_of_uncached<'tcx>(
                     tcx.layout_scalar_valid_range(def.did()),
                     get_discriminant_type,
                     discriminants_iter(),
-                    dont_niche_optimize_enum,
                     !maybe_unsized,
                 ) else {
                     bug!("failed to compute unsized layout of {ty:?}");

src/tools/rust-analyzer/crates/hir-ty/src/layout/adt.rs

@@ -3,19 +3,19 @@
 use std::{cmp, ops::Bound};
 
 use hir_def::{
-    AdtId, VariantId,
     layout::{Integer, ReprOptions, TargetDataLayout},
     signatures::{StructFlags, VariantFields},
+    AdtId, VariantId,
 };
 use intern::sym;
 use rustc_index::IndexVec;
 use smallvec::SmallVec;
 use triomphe::Arc;
 
 use crate::{
-    Substitution, TraitEnvironment,
     db::HirDatabase,
-    layout::{Layout, LayoutError, field_ty},
+    layout::{field_ty, Layout, LayoutError},
+    Substitution, TraitEnvironment,
 };
 
 use super::LayoutCx;
@@ -85,16 +85,6 @@ pub fn layout_of_adt_query(
                 let d = db.const_eval_discriminant(e.enum_variants(db).variants[id.0].0).ok()?;
                 Some((id, d))
             }),
-            // FIXME: The current code for niche-filling relies on variant indices
-            // instead of actual discriminants, so enums with
-            // explicit discriminants (RFC #2363) would misbehave and we should disable
-            // niche optimization for them.
-            // The code that do it in rustc:
-            // repr.inhibit_enum_layout_opt() || def
-            //     .variants()
-            //     .iter_enumerated()
-            //     .any(|(i, v)| v.discr != ty::VariantDiscr::Relative(i.as_u32()))
-            repr.inhibit_enum_layout_opt(),
             !matches!(def, AdtId::EnumId(..))
                 && variants
                     .iter()

tests/ui/enum-discriminant/arbitrary_enum_discriminant-no-repr.rs

@@ -1,8 +1,17 @@
-#![crate_type="lib"]
+#![crate_type = "lib"]
 
+// Test that if any variant is non-unit,
+// we need a repr.
 enum Enum {
-//~^ ERROR `#[repr(inttype)]` must be specified
-  Unit = 1,
-  Tuple() = 2,
-  Struct{} = 3,
+    //~^ ERROR `#[repr(inttype)]` must be specified
+    Unit = 1,
+    Tuple(),
+    Struct {},
+}
+
+// Test that if any non-unit variant has an explicit
+// discriminant we need a repr.
+enum Enum2 {
+    //~^ ERROR `#[repr(inttype)]` must be specified
+    Tuple() = 2,
 }

tests/ui/enum-discriminant/arbitrary_enum_discriminant-no-repr.stderr

@@ -1,9 +1,23 @@
-error[E0732]: `#[repr(inttype)]` must be specified
-  --> $DIR/arbitrary_enum_discriminant-no-repr.rs:3:1
+error[E0732]: `#[repr(inttype)]` must be specified for enums with explicit discriminants and non-unit variants
+  --> $DIR/arbitrary_enum_discriminant-no-repr.rs:5:1
    |
 LL | enum Enum {
    | ^^^^^^^^^
+LL |
+LL |     Unit = 1,
+   |            - explicit discriminant specified here
+LL |     Tuple(),
+   |     ----- non-unit discriminant declared here
 
-error: aborting due to 1 previous error
+error[E0732]: `#[repr(inttype)]` must be specified for enums with explicit discriminants and non-unit variants
+  --> $DIR/arbitrary_enum_discriminant-no-repr.rs:14:1
+   |
+LL | enum Enum2 {
+   | ^^^^^^^^^^
+LL |
+LL |     Tuple() = 2,
+   |               - explicit discriminant on non-unit variant specified here
+
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0732`.