Skip to content

Commit

Permalink
Auto merge of rust-lang#70825 - eddyb:enum-discr-correct-generics-par…
Browse files Browse the repository at this point in the history
…ent, r=nikomatsakis

typeck: always expose explicit enum discriminant `AnonConst`s' parent in `generics_of`.

This is similar to rust-lang#70452 but for explicit `enum` discriminant constant expressions.
However, unlike rust-lang#70452, this PR should have no effect on stable code, as while it alleviates rust-lang#43408 errors, there is no way to actually compile an `enum` with generic parameters *and* explicit discriminants, without `#![feature(arbitrary_enum_discriminant)]`, as explicit discriminant expression don't count as uses of parameters (if they did, they would count as invariant uses).

<hr/>

There's also 2 other commits here, both related to rust-lang#70453:
* "ty: use `delay_span_bug` in `ty::AdtDef::eval_explicit_discr`." - hides the ICEs demonstrated on rust-lang#70453, when there are other errors (which the next commit adds)
* "typeck/wfcheck: require that explicit enum discriminants const-evaluate succesfully." - closes rust-lang#70453 by picking alternative "2", i.e. erroring when a discriminant doesn't fully const-evaluate from the perspective of the `enum` definition

In the future, it might be possible to allow `enum` discriminants to actually depend on parameters, but that will likely require rust-lang#68436 + some way to restrict the values so no two variants can end up with overlapping discriminants.

As this PR would close rust-lang#70453, it shouldn't be merged until a decision is reached there.

r? @nikomatsakis
  • Loading branch information
bors committed May 3, 2020
2 parents d626e4d + 926c7a2 commit e5f35df
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 19 deletions.
23 changes: 8 additions & 15 deletions src/librustc_middle/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2388,21 +2388,14 @@ impl<'tcx> AdtDef {
None
}
}
Err(ErrorHandled::Reported(ErrorReported) | ErrorHandled::Linted) => {
if !expr_did.is_local() {
span_bug!(
tcx.def_span(expr_did),
"variant discriminant evaluation succeeded \
in its crate but failed locally"
);
}
None
}
Err(ErrorHandled::TooGeneric) => {
tcx.sess.delay_span_bug(
tcx.def_span(expr_did),
"enum discriminant depends on generic arguments",
);
Err(err) => {
let msg = match err {
ErrorHandled::Reported(ErrorReported) | ErrorHandled::Linted => {
"enum discriminant evaluation failed"
}
ErrorHandled::TooGeneric => "enum discriminant depends on generics",
};
tcx.sess.delay_span_bug(tcx.def_span(expr_did), msg);
None
}
}
Expand Down
37 changes: 35 additions & 2 deletions src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,23 @@ fn check_type_defn<'tcx, F>(
ObligationCauseCode::MiscObligation,
)
}

// Explicit `enum` discriminant values must const-evaluate successfully.
if let Some(discr_def_id) = variant.explicit_discr {
let discr_substs =
InternalSubsts::identity_for_item(fcx.tcx, discr_def_id.to_def_id());

let cause = traits::ObligationCause::new(
fcx.tcx.def_span(discr_def_id),
fcx.body_id,
traits::MiscObligation,
);
fcx.register_predicate(traits::Obligation::new(
cause,
fcx.param_env,
ty::Predicate::ConstEvaluatable(discr_def_id.to_def_id(), discr_substs),
));
}
}

check_where_clauses(tcx, fcx, item.span, def_id.to_def_id(), None);
Expand Down Expand Up @@ -1287,8 +1304,14 @@ impl ParItemLikeVisitor<'tcx> for CheckTypeWellFormedVisitor<'tcx> {
///////////////////////////////////////////////////////////////////////////
// ADT

// FIXME(eddyb) replace this with getting fields/discriminants through `ty::AdtDef`.
struct AdtVariant<'tcx> {
/// Types of fields in the variant, that must be well-formed.
fields: Vec<AdtField<'tcx>>,

/// Explicit discriminant of this variant (e.g. `A = 123`),
/// that must evaluate to a constant value.
explicit_discr: Option<LocalDefId>,
}

struct AdtField<'tcx> {
Expand All @@ -1297,6 +1320,7 @@ struct AdtField<'tcx> {
}

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// FIXME(eddyb) replace this with getting fields through `ty::AdtDef`.
fn non_enum_variant(&self, struct_def: &hir::VariantData<'_>) -> AdtVariant<'tcx> {
let fields = struct_def
.fields()
Expand All @@ -1309,11 +1333,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
AdtField { ty: field_ty, span: field.span }
})
.collect();
AdtVariant { fields }
AdtVariant { fields, explicit_discr: None }
}

fn enum_variants(&self, enum_def: &hir::EnumDef<'_>) -> Vec<AdtVariant<'tcx>> {
enum_def.variants.iter().map(|variant| self.non_enum_variant(&variant.data)).collect()
enum_def
.variants
.iter()
.map(|variant| AdtVariant {
fields: self.non_enum_variant(&variant.data).fields,
explicit_discr: variant
.disr_expr
.map(|explicit_discr| self.tcx.hir().local_def_id(explicit_discr.hir_id)),
})
.collect()
}

fn impl_implied_bounds(&self, impl_def_id: DefId, span: Span) -> Vec<Ty<'tcx>> {
Expand Down
6 changes: 4 additions & 2 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1178,9 +1178,11 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::Generics {
let parent_node = tcx.hir().get(tcx.hir().get_parent_node(hir_id));
match parent_node {
// HACK(eddyb) this provides the correct generics for repeat
// expressions' count (i.e. `N` in `[x; N]`), as they shouldn't
// be able to cause query cycle errors.
// expressions' count (i.e. `N` in `[x; N]`), and explicit
// `enum` discriminants (i.e. `D` in `enum Foo { Bar = D }`),
// as they shouldn't be able to cause query cycle errors.
Node::Expr(&Expr { kind: ExprKind::Repeat(_, ref constant), .. })
| Node::Variant(Variant { disr_expr: Some(ref constant), .. })
if constant.hir_id == hir_id =>
{
Some(parent_def_id.to_def_id())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#![feature(arbitrary_enum_discriminant, core_intrinsics)]

extern crate core;
use core::intrinsics::discriminant_value;

#[repr(usize)]
enum MyWeirdOption<T> {
None = 0,
Some(T) = std::mem::size_of::<T>(),
//~^ ERROR constant expression depends on a generic parameter
}

fn main() {
assert_eq!(discriminant_value(&MyWeirdOption::<u8>::None), 0);
assert_eq!(discriminant_value(&MyWeirdOption::Some(0u8)), 1);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: constant expression depends on a generic parameter
--> $DIR/issue-70453-generics-in-discr-ice-2.rs:9:15
|
LL | Some(T) = std::mem::size_of::<T>(),
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this may fail depending on what value the parameter takes

error: aborting due to previous error

17 changes: 17 additions & 0 deletions src/test/ui/enum-discriminant/issue-70453-generics-in-discr-ice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![feature(core_intrinsics)]

extern crate core;
use core::intrinsics::discriminant_value;

#[repr(usize)]
enum MyWeirdOption<T> {
//~^ ERROR parameter `T` is never used
None = 0,
Some = std::mem::size_of::<T>(),
//~^ ERROR constant expression depends on a generic parameter
}

fn main() {
assert_eq!(discriminant_value(&MyWeirdOption::<u8>::None), 0);
assert_eq!(discriminant_value(&MyWeirdOption::<u8>::Some), 1);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error: constant expression depends on a generic parameter
--> $DIR/issue-70453-generics-in-discr-ice.rs:10:12
|
LL | Some = std::mem::size_of::<T>(),
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this may fail depending on what value the parameter takes

error[E0392]: parameter `T` is never used
--> $DIR/issue-70453-generics-in-discr-ice.rs:7:20
|
LL | enum MyWeirdOption<T> {
| ^ unused parameter
|
= help: consider removing `T`, referring to it in a field, or using a marker such as `std::marker::PhantomData`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0392`.
17 changes: 17 additions & 0 deletions src/test/ui/enum-discriminant/issue-70453-polymorphic-ctfe.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// run-pass

#![feature(arbitrary_enum_discriminant, core_intrinsics)]

extern crate core;
use core::intrinsics::discriminant_value;

#[repr(usize)]
enum MyWeirdOption<T> {
None = 0,
Some(T) = core::mem::size_of::<*mut T>(),
}

fn main() {
assert_eq!(discriminant_value(&MyWeirdOption::<()>::None), 0);
assert_eq!(discriminant_value(&MyWeirdOption::Some(())), core::mem::size_of::<usize>() as u64);
}

0 comments on commit e5f35df

Please sign in to comment.