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

Make TooGeneric error in WF checking a proper error #68388

Merged
merged 1 commit into from
Jan 23, 2020

Conversation

varkor
Copy link
Member

@varkor varkor commented Jan 20, 2020

TooGeneric is encountered during WF checking when we cannot determine that a constant involving a generic parameter will always be evaluated successfully (rather than resulting in an error). In these cases, the burden of proof should be with the caller, so that we can avoid post-monomorphisation time errors (which was the previous previous behaviour). This commit ensures that this situation produces a proper compiler error, rather than silently ignoring it or ICEing.

Fixes #66962.

r? @eddyb

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 20, 2020
@varkor varkor added the F-const_generics `#![feature(const_generics)]` label Jan 20, 2020
// prove are well-formed.
let mut err = self.tcx.sess.struct_span_err(
span,
"cannot determine that constant is always well-formed",
Copy link
Member

Choose a reason for hiding this comment

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

We probably shouldn't talk about WF in diagnostics.

The way I was initially approaching this (see variant names, like ConstEvaluatable in Predicate and ConstEvalFailure here) would suggest something more like:

constant expression depends on generic parameters

followed by explaining that it may fail to evaluate depending on the choice of generics.

Copy link
Member

Choose a reason for hiding this comment

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

Oh and you can bypass the problem of explaining to the user how to make it work, by linking to a rust-lang/rust issue (ideally a new one without any preexisting discussion) titled something like "Constant expressions using generic params require weird where clauses." with a "canonical" example.

Note that the "canonical" example doesn't work yet, because ConstEvaluatable requirements from bounds aren't implied in the body (maybe they need to be added to ParamEnv, and then looked up in there?).

I'm not even sure we can (ab)use the where TyAlias<T, {N}>:, pattern like that, usually WF semantics would require that the definition that where is attached to proves the WF, instead of deferring that to users of that definition.
I would want a @rust-lang/wg-traits confirmation that we want to implement that behavior (as it's not limited to const generics, it applies to any constant expressions using generic parameters, e.g. [u8; size_of::<T>()], so it might impact stable).

@varkor
Copy link
Member Author

varkor commented Jan 20, 2020

Fixed.

@rust-highfive

This comment has been minimized.

`TooGeneric` is encountered during WF checking when we cannot determine that a constant involving a generic parameter will always be evaluated successfully (rather than resulting in an error). In these cases, the burden of proof should be with the caller, so that we can avoid post-monomorphisation tim errors (which was the previous previous behaviour). This commit ensures that this situation produces a proper compiler error, rather than silently ignoring it or ICEing.
Copy link
Member

@eddyb eddyb left a comment

Choose a reason for hiding this comment

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

I don't like leaving this an error with no workaround, but I think it's better to properly decide how to approach it.

@eddyb
Copy link
Member

eddyb commented Jan 21, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jan 21, 2020

📌 Commit dd0507c has been approved by eddyb

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 21, 2020
tmandry added a commit to tmandry/rust that referenced this pull request Jan 22, 2020
Make `TooGeneric` error in WF checking a proper error

`TooGeneric` is encountered during WF checking when we cannot determine that a constant involving a generic parameter will always be evaluated successfully (rather than resulting in an error). In these cases, the burden of proof should be with the caller, so that we can avoid post-monomorphisation tim errors (which was the previous previous behaviour). This commit ensures that this situation produces a proper compiler error, rather than silently ignoring it or ICEing.

Fixes rust-lang#66962.

r? @eddyb
tmandry added a commit to tmandry/rust that referenced this pull request Jan 22, 2020
Make `TooGeneric` error in WF checking a proper error

`TooGeneric` is encountered during WF checking when we cannot determine that a constant involving a generic parameter will always be evaluated successfully (rather than resulting in an error). In these cases, the burden of proof should be with the caller, so that we can avoid post-monomorphisation tim errors (which was the previous previous behaviour). This commit ensures that this situation produces a proper compiler error, rather than silently ignoring it or ICEing.

Fixes rust-lang#66962.

r? @eddyb
tmandry added a commit to tmandry/rust that referenced this pull request Jan 22, 2020
Make `TooGeneric` error in WF checking a proper error

`TooGeneric` is encountered during WF checking when we cannot determine that a constant involving a generic parameter will always be evaluated successfully (rather than resulting in an error). In these cases, the burden of proof should be with the caller, so that we can avoid post-monomorphisation tim errors (which was the previous previous behaviour). This commit ensures that this situation produces a proper compiler error, rather than silently ignoring it or ICEing.

Fixes rust-lang#66962.

r? @eddyb
tmandry added a commit to tmandry/rust that referenced this pull request Jan 23, 2020
Make `TooGeneric` error in WF checking a proper error

`TooGeneric` is encountered during WF checking when we cannot determine that a constant involving a generic parameter will always be evaluated successfully (rather than resulting in an error). In these cases, the burden of proof should be with the caller, so that we can avoid post-monomorphisation tim errors (which was the previous previous behaviour). This commit ensures that this situation produces a proper compiler error, rather than silently ignoring it or ICEing.

Fixes rust-lang#66962.

r? @eddyb
bors added a commit that referenced this pull request Jan 23, 2020
Rollup of 10 pull requests

Successful merges:

 - #67195 ([experiment] Add `-Z no-link` flag)
 - #68253 (add bare metal ARM Cortex-A targets to rustc)
 - #68361 (Unbreak linking with lld 9 on FreeBSD 13.0-CURRENT i386)
 - #68388 (Make `TooGeneric` error in WF checking a proper error)
 - #68409 (Micro-optimize OutputFilenames)
 - #68410 (Export weak symbols used by MemorySanitizer)
 - #68425 (Fix try-op diagnostic in E0277 for methods)
 - #68440 (bootstrap: update clippy subcmd decription)
 - #68441 (pprust: use as_deref)
 - #68462 (librustc_mir: don't allocate vectors where slices will do.)

Failed merges:

r? @ghost
@bors bors merged commit dd0507c into rust-lang:master Jan 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-const_generics `#![feature(const_generics)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Const generic ICE: constant in type had an ignored error: TooGeneric
4 participants