Fix ICE with explicit late-bound lifetimes#72441
Fix ICE with explicit late-bound lifetimes#72441bors merged 1 commit intorust-lang:masterfrom cofibrant:late-bound-lifetime-ice
Conversation
rust-highfive
added
the
S-waiting-on-review
src/librustc_typeck/astconv.rs
Outdated
There was a problem hiding this comment.
Couldn't this information go into the GenericArgCountMismatch?
There was a problem hiding this comment.
The problem is we want to warn not error at the moment (introducing an error would be a breaking change - see #42868), so basically we end up with situations where we might not have a generic arg count mismatch but we still have explicit late bounds. That's part of what's causing the ICE. (If you #[deny(warnings)] the ICE disappears.)
There was a problem hiding this comment.
Can we consider trying to close #42868? I guess we still need to make some expressiveness improvements to do so?
There was a problem hiding this comment.
So #42868 lists two future PRs, one to make the lint deny-by-default, and the other to turn the warning into a hard error. Making the lint deny-by-default would still require this patch, but if we jumped straight to a hard error, then none of this work is necessary. It doesn't resolve the issue that this is still a breaking change though, but I guess that might not matter so much seeing as it's already broken?
|
r? @nikomatsakis (maybe we should discuss how to get rid of the early/late-bound lifetime split...) |
|
Reading, I think @nikomatsakis's suggestion here is probably close to the right answer, but we'd definitely have to wait for a new edition before it could be integrated. |
src/librustc_typeck/astconv.rs
Outdated
There was a problem hiding this comment.
Can we consider trying to close #42868? I guess we still need to make some expressiveness improvements to do so?
src/test/ui/issues/issue-72278.rs
Outdated
There was a problem hiding this comment.
Do we have tests for all the other random scenarios that can occur? It seems like we should try to test them all -- in particular the case of "got a lifetime but expected something else"...
There was a problem hiding this comment.
I was under the impression that
- src/test/ui/methods/method-call-lifetime-args.rs
- src/test/ui/methods/method-call-lifetime-args-fail.rs
cover pretty much everything but I could be wrong
There was a problem hiding this comment.
Those tests look pretty good. I guess they don't cover lifetimes + types intermixed. But I'm satisfied.
|
So I guess r=me with comment applied |
|
@bors r+ |
|
📌 Commit fa351ee has been approved by |
bors
added
S-waiting-on-bors
Rollup of 9 pull requests Successful merges: - rust-lang#72299 (more `LocalDefId`s) - rust-lang#72368 (Resolve overflow behavior for RangeFrom) - rust-lang#72441 (Fix ICE with explicit late-bound lifetimes) - rust-lang#72499 (Override Box::<[T]>::clone_from) - rust-lang#72521 (Properly handle InlineAsmOperand::SymFn when collecting monomorphized items) - rust-lang#72540 (mir: adjust conditional in recursion limit check) - rust-lang#72563 (multiple Return terminators are possible) - rust-lang#72585 (Only capture tokens for items with outer attributes) - rust-lang#72607 (Eagerly lower asm sub-expressions to HIR even if there is an error) Failed merges: r? @ghost
6 participants
Rather than returning an explicit late-bound lifetime as a generic argument count mismatch (which is not necessarily true), this PR propagates the presence of explicit late-bound lifetimes.
This avoids an ICE that can occur due to the presence of explicit late-bound lifetimes when building generic substitutions by explicitly ignoring them.
r? @varkor
cc @davidtwco (this removes a check you introduced in #60892)
Resolves #72278