Skip to content

Commit d6a149d

Browse files
committed
Remove dead code and extend test coverage and diagnostics around it
We lost the following comment during refactorings: The current code for niche-filling relies on variant indices instead of actual discriminants, so enums with explicit discriminants (RFC #2363) would misbehave.
1 parent 3c30dbb commit d6a149d

File tree

5 files changed

+62
-35
lines changed

5 files changed

+62
-35
lines changed

compiler/rustc_abi/src/layout.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,6 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
313313
scalar_valid_range: (Bound<u128>, Bound<u128>),
314314
discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool),
315315
discriminants: impl Iterator<Item = (VariantIdx, i128)>,
316-
dont_niche_optimize_enum: bool,
317316
always_sized: bool,
318317
) -> LayoutCalculatorResult<FieldIdx, VariantIdx, F> {
319318
let (present_first, present_second) = {
@@ -352,13 +351,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
352351
// structs. (We have also handled univariant enums
353352
// that allow representation optimization.)
354353
assert!(is_enum);
355-
self.layout_of_enum(
356-
repr,
357-
variants,
358-
discr_range_of_repr,
359-
discriminants,
360-
dont_niche_optimize_enum,
361-
)
354+
self.layout_of_enum(repr, variants, discr_range_of_repr, discriminants)
362355
}
363356
}
364357

@@ -599,7 +592,6 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
599592
variants: &IndexSlice<VariantIdx, IndexVec<FieldIdx, F>>,
600593
discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool),
601594
discriminants: impl Iterator<Item = (VariantIdx, i128)>,
602-
dont_niche_optimize_enum: bool,
603595
) -> LayoutCalculatorResult<FieldIdx, VariantIdx, F> {
604596
// Until we've decided whether to use the tagged or
605597
// niche filling LayoutData, we don't want to intern the
@@ -618,7 +610,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
618610
}
619611

620612
let calculate_niche_filling_layout = || -> Option<TmpLayout<FieldIdx, VariantIdx>> {
621-
if dont_niche_optimize_enum {
613+
if repr.inhibit_enum_layout_opt() {
622614
return None;
623615
}
624616

compiler/rustc_hir_analysis/src/check/check.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1642,20 +1642,40 @@ fn check_enum(tcx: TyCtxt<'_>, def_id: LocalDefId) {
16421642

16431643
if def.repr().int.is_none() {
16441644
let is_unit = |var: &ty::VariantDef| matches!(var.ctor_kind(), Some(CtorKind::Const));
1645-
let has_disr = |var: &ty::VariantDef| matches!(var.discr, ty::VariantDiscr::Explicit(_));
1645+
let get_disr = |var: &ty::VariantDef| match var.discr {
1646+
ty::VariantDiscr::Explicit(disr) => Some(disr),
1647+
ty::VariantDiscr::Relative(_) => None,
1648+
};
16461649

1647-
let has_non_units = def.variants().iter().any(|var| !is_unit(var));
1648-
let disr_units = def.variants().iter().any(|var| is_unit(var) && has_disr(var));
1649-
let disr_non_unit = def.variants().iter().any(|var| !is_unit(var) && has_disr(var));
1650+
let non_unit = def.variants().iter().find(|var| !is_unit(var));
1651+
let disr_unit =
1652+
def.variants().iter().filter(|var| is_unit(var)).find_map(|var| get_disr(var));
1653+
let disr_non_unit =
1654+
def.variants().iter().filter(|var| !is_unit(var)).find_map(|var| get_disr(var));
16501655

1651-
if disr_non_unit || (disr_units && has_non_units) {
1652-
struct_span_code_err!(
1656+
if disr_non_unit.is_some() || (disr_unit.is_some() && non_unit.is_some()) {
1657+
let mut err = struct_span_code_err!(
16531658
tcx.dcx(),
16541659
tcx.def_span(def_id),
16551660
E0732,
1656-
"`#[repr(inttype)]` must be specified"
1657-
)
1658-
.emit();
1661+
"`#[repr(inttype)]` must be specified for enums with explicit discriminants and non-unit variants"
1662+
);
1663+
if let Some(disr_non_unit) = disr_non_unit {
1664+
err.span_label(
1665+
tcx.def_span(disr_non_unit),
1666+
"explicit discriminant on non-unit variant specified here",
1667+
);
1668+
} else {
1669+
err.span_label(
1670+
tcx.def_span(disr_unit.unwrap()),
1671+
"explicit discriminant specified here",
1672+
);
1673+
err.span_label(
1674+
tcx.def_span(non_unit.unwrap().def_id),
1675+
"non-unit discriminant declared here",
1676+
);
1677+
}
1678+
err.emit();
16591679
}
16601680
}
16611681

compiler/rustc_ty_utils/src/layout.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -603,12 +603,6 @@ fn layout_of_uncached<'tcx>(
603603
.flatten()
604604
};
605605

606-
let dont_niche_optimize_enum = def.repr().inhibit_enum_layout_opt()
607-
|| def
608-
.variants()
609-
.iter_enumerated()
610-
.any(|(i, v)| v.discr != ty::VariantDiscr::Relative(i.as_u32()));
611-
612606
let maybe_unsized = def.is_struct()
613607
&& def.non_enum_variant().tail_opt().is_some_and(|last_field| {
614608
let typing_env = ty::TypingEnv::post_analysis(tcx, def.did());
@@ -625,7 +619,6 @@ fn layout_of_uncached<'tcx>(
625619
tcx.layout_scalar_valid_range(def.did()),
626620
get_discriminant_type,
627621
discriminants_iter(),
628-
dont_niche_optimize_enum,
629622
!maybe_unsized,
630623
)
631624
.map_err(|err| map_error(cx, ty, err))?;
@@ -651,7 +644,6 @@ fn layout_of_uncached<'tcx>(
651644
tcx.layout_scalar_valid_range(def.did()),
652645
get_discriminant_type,
653646
discriminants_iter(),
654-
dont_niche_optimize_enum,
655647
!maybe_unsized,
656648
) else {
657649
bug!("failed to compute unsized layout of {ty:?}");
Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,17 @@
1-
#![crate_type="lib"]
1+
#![crate_type = "lib"]
22

3+
// Test that if any variant is non-unit,
4+
// we need a repr.
35
enum Enum {
4-
//~^ ERROR `#[repr(inttype)]` must be specified
5-
Unit = 1,
6-
Tuple() = 2,
7-
Struct{} = 3,
6+
//~^ ERROR `#[repr(inttype)]` must be specified
7+
Unit = 1,
8+
Tuple(),
9+
Struct {},
10+
}
11+
12+
// Test that if any non-unit variant has an explicit
13+
// discriminant we need a repr.
14+
enum Enum2 {
15+
//~^ ERROR `#[repr(inttype)]` must be specified
16+
Tuple() = 2,
817
}
Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,23 @@
1-
error[E0732]: `#[repr(inttype)]` must be specified
2-
--> $DIR/arbitrary_enum_discriminant-no-repr.rs:3:1
1+
error[E0732]: `#[repr(inttype)]` must be specified for enums with explicit discriminants and non-unit variants
2+
--> $DIR/arbitrary_enum_discriminant-no-repr.rs:5:1
33
|
44
LL | enum Enum {
55
| ^^^^^^^^^
6+
LL |
7+
LL | Unit = 1,
8+
| - explicit discriminant specified here
9+
LL | Tuple(),
10+
| ----- non-unit discriminant declared here
611

7-
error: aborting due to 1 previous error
12+
error[E0732]: `#[repr(inttype)]` must be specified for enums with explicit discriminants and non-unit variants
13+
--> $DIR/arbitrary_enum_discriminant-no-repr.rs:14:1
14+
|
15+
LL | enum Enum2 {
16+
| ^^^^^^^^^^
17+
LL |
18+
LL | Tuple() = 2,
19+
| - explicit discriminant on non-unit variant specified here
20+
21+
error: aborting due to 2 previous errors
822

923
For more information about this error, try `rustc --explain E0732`.

0 commit comments

Comments
 (0)