-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Fix gce ICE when encountering ill-formed consts #119060
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
r? compiler-errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a better explanation for why this ICE occurs, since I think downgrading this span bug to a delayed span bug is a bit surprising solution to the ICE... so I'd rather have the other options exhausted first.
Specifically, why does it lead to an unevaluable const in the first place? Why is it not causing a validation error earlier?
As I understand it, a validation error does get created during wf_checking, but due to #117159, item_types_checking still occurs despite the errors, leading to |
Sorry for sitting on this PR for this long, and sorry for forgetting to update labels when I last replied. I've added a comment explaining why we delay the bug instead of ICEing immediately--I'm not sure if there's a less hacky solution to this ICE, though I'd hardly say I'm familiar with this part of the compiler. @rustbot review |
There's still no explanation why this is happening. All it does is link to #118545 and #117159. I expected this to have at least some type of PR description that explains why this is a valid solution. I've thought a bit about this, and I don't believe we should fix this. GCE is very unstable currently, and this PR, as far as I understand, simply hides the fact that we're calling Until we have a better story about not calling resolve on unevaluated consts that are not WF and valid, I think we should just leave this ICE. It's not possible to encounter it without the feature gate enabled. |
@rustbot author |
I looked into it a little more, and it seems like the root of the problem is us creating #![feature(generic_const_exprs)]
struct Checked<const F: fn(usize) -> bool>;
fn not_one(val: usize) -> bool {
val != 1
}
const _: Checked<not_one> = Checked::<not_one>; and #119731 ICEs when calling
That makes sense--the only "better" solution I could come up with is replacing the two occurrences of match self.selcx.infcx.const_eval_resolve(...) {
Ok(Some(val)) => Ok(ty::Const::new_value(tcx, val, c.ty())),
Ok(None) => {
let span = tcx.def_span(unevaluated.def);
let reported = tcx.dcx().create_err(UnableToConstructConstantValue {
span,
unevaluated,
}).delay_as_bug();
Err(ErrorHandled::Reported(reported.into(), span))
}
Err(e) => Err(e),
} I honestly have no clue how to prevent the creation of @rustbot review |
Given that this is only reachable with GCE, which is both incomplete and needing a large overhaul, I'm not comfortable merging more hacks to fix one-off ICEs. Sorry for delaying this for so long, but I'm just gonna close this. Thanks for the contribution anyways! |
Fixes #118545