-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Allow inference regions when relating consts #73225
Conversation
Nominating for backport as this fixes a stable-stable regression. |
@bors r+ |
|
📌 Commit d794313 has been approved by |
Rollup of 5 pull requests Successful merges: - rust-lang#72906 (Migrate to numeric associated consts) - rust-lang#73178 (expand: More precise locations for expansion-time lints) - rust-lang#73225 (Allow inference regions when relating consts) - rust-lang#73236 (Clean up E0666 explanation) - rust-lang#73273 (Fix Zulip pings) Failed merges: r? @ghost
Discussed at T-compiler meeting; approved for beta backport. |
async fn foo<'a>() { | ||
let _data = &mut [0u8; { 1 + 4 }]; | ||
bar().await | ||
} |
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.
Weird that this would need async
, presumably fn foo<'a: 'a>
(forcing lifetime parameters aka "early-bound" as opposed to only the signature being lifetime-polymorphic aka "late-bound) would also work?
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.
I can't get it to ICE: playground
The ICE happened when unifying the generator witness with the generator type (IIRC?), so maybe there's something special about that operation.
let eagerly_eval = |x: &'tcx ty::Const<'tcx>| { | ||
// FIXME(eddyb) this doesn't account for lifetime inference variables | ||
// being erased by `eval`, *nor* for the polymorphic aspect of `eval`. | ||
// That is, we could always use `eval` and it will just return the | ||
// old value back if it doesn't succeed. | ||
if !x.val.needs_infer() { | ||
return x.eval(tcx, relation.param_env()).val; | ||
} | ||
x.val | ||
}; | ||
let eagerly_eval = |x: &'tcx ty::Const<'tcx>| x.eval(tcx, relation.param_env()).val; |
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.
I think this makes sense. I left that FIXME comment (and the one below) while working on an unrelated PR #71049 (and glancing at code next to what I was touching). I should've probably filed issues.
At least I don't think this FIXME was a soundness problem, the overzealous check could only lead to types failing to equate (which could mean ICEs, clearly it happened here so it's possible).
This is reminiscent of #70773, where missing normalizations started showing up after we stopped ignoring generics in scope, in #70452.
I wonder if changing this one piece of code in relate
makes the extra normalizations added to fix #70773 unnecessary.
…ulacrum [beta] backports This PR backports the following: * Make novel structural match violations not a `bug` rust-lang#73446 * linker: Never pass `-no-pie` to non-gnu linkers rust-lang#73384 * Disable the `SimplifyArmIdentity` pass rust-lang#73262 * Allow inference regions when relating consts rust-lang#73225 * Fix link error with #[thread_local] introduced by rust-lang#71192 rust-lang#73065 * Ensure stack when building MIR for matches rust-lang#72941 * Don't create impl candidates when obligation contains errors rust-lang#73005 r? @ghost
As first noticed by @eddyb,
super_relate_consts
doesn't need to check for inference vars sinceeval
does it already (and handles lifetimes correctly by erasing them).Fixes #73050
r? @oli-obk