Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typeck: always expose explicit enum discriminant AnonConsts' parent in generics_of. #70825

Merged
merged 3 commits into from
May 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment could use some elaboration regarding the "why" as well as a link towards #70453.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure I'm following along, this concept of "must be evaluatable" is going to ultimately be true for any const expression, right? I think this is the basic WF criteria we are working towards?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, at the minimum any ty::Const used in the typesystem (array lengths and args to const params). While enum discrminants aren't that, they do require strict checks (no overlap), for which successful evaluatability is a bare minimum.

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);
}