-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Clean up type alias impl trait implementation #72080
Conversation
r? @davidtwco (rust_highfive has picked a reviewer for you, use r? to override) |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
c808d4f
to
8be87df
Compare
r? @eddyb |
☔ The latest upstream changes (presumably #70705) made this pull request unmergeable. Please resolve the merge conflicts. |
Sorry for not seeing this earlier. Also, I'm not the best person to review this (but I will go through it). |
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.
Seems pretty good, overall, especially all the removed code. I'll let @nikomatsakis take a closer look though.
src/librustc_ast_lowering/lib.rs
Outdated
} else { | ||
// Non return-position impl trait captures all of the lifetimes of | ||
// the parent item. | ||
(&[], &[]) |
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 to include the bound regions that the type captures.
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.
Is your intent to patch this code?
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.
Yes, although it ended up being a somewhat large change.
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 all looks good. I left various comments and nits.
src/librustc_ast_lowering/lib.rs
Outdated
} else { | ||
// Non return-position impl trait captures all of the lifetimes of | ||
// the parent item. | ||
(&[], &[]) |
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.
Is your intent to patch this code?
Since this also fixes #73061, it'd be good to add a regression test for that + add it to the "closes" list. |
I wonder if this change is also going to make it easier to fix #65863. |
7245c19
to
51a6550
Compare
I don't think this helps with #65863, because this doesn't change the AST representation of impl trait types. |
@bors r+ |
📌 Commit 51a6550 has been approved by |
🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened |
e9caf48
to
985f3a6
Compare
@matthewjasper Oh, I'm sorry, I scrolled through looking for it, but it must have been collapsed by GitHub somehow! Sorry for the noise ❤️ |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
985f3a6
to
8b10d42
Compare
@bors r=nikomatsakis |
📌 Commit 8b10d42 has been approved by |
Somehow Windows x86_64 msvc-2 is almost at 4h now (and will likely timeout) even though it usually takes <3h. |
☀️ Test successful - checks-azure |
Oh wow, it just barely made it. Nice :D |
…ikomatsakis Clean up type alias impl trait implementation - Removes special case for top-level impl trait - Removes associated opaque types - Forbid lifetime elision in let position impl trait. This is consistent with the behavior for inferred types. - Handle lifetimes in type alias impl trait more uniformly with other parameters cc rust-lang#69323 cc rust-lang#63063 Closes rust-lang#57188 Closes rust-lang#62988 Closes rust-lang#69136 Closes rust-lang#73061
Fix breakage due to rust-lang/rust#72080 See rust-lang/rust#72080.
cc #69323
cc #63063
Closes #57188
Closes #62988
Closes #69136
Closes #73061