-
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 printing impl trait
under binders
#98371
Fix printing impl trait
under binders
#98371
Conversation
r? @oli-obk (rust-highfive has picked a reviewer for you, use r? to override) |
f34687b
to
a1e0ef4
Compare
@bors r+ |
📌 Commit a1e0ef43bb404a27d5651f3b8a03b19acede7a8d 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. |
@bors r- failed to merge apparently |
☔ The latest upstream changes (presumably #98447) made this pull request unmergeable. Please resolve the merge conflicts. |
a1e0ef4
to
e80cced
Compare
@bors r=oli-obk |
📌 Commit e80cced has been approved by |
…ing, r=oli-obk Fix printing `impl trait` under binders Before, we would render `impl for<'a> Trait<'a>` like `impl Trait<for<'a> 'a>`, lol.
…ing, r=oli-obk Fix printing `impl trait` under binders Before, we would render `impl for<'a> Trait<'a>` like `impl Trait<for<'a> 'a>`, lol.
…ing, r=oli-obk Fix printing `impl trait` under binders Before, we would render `impl for<'a> Trait<'a>` like `impl Trait<for<'a> 'a>`, lol.
Rollup of 8 pull requests Successful merges: - rust-lang#98371 (Fix printing `impl trait` under binders) - rust-lang#98385 (Work around llvm 12's memory ordering restrictions.) - rust-lang#98474 (x.py: Support systems with only `python3` not `python`) - rust-lang#98488 (Bump RLS to latest master on rust-lang/rls) - rust-lang#98491 (Fix backtrace UI test when panic=abort is used) - rust-lang#98502 (Fix source sidebar hover in ayu theme) - rust-lang#98509 (diagnostics: consider parameter count when suggesting smart pointers) - rust-lang#98513 (Fix LLVM rebuild with download-ci-llvm.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
@@ -22,7 +22,7 @@ LL | async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> { | |||
LL | | | |||
LL | | } | |||
| |_^ | |||
= note: required because it captures the following types: `ResumeTy`, `impl Future<Output = ()>`, `()` | |||
= note: required because it captures the following types: `ResumeTy`, `impl for<'r, 's, 't0> Future<Output = ()>`, `()` |
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.
hmm, is this change expected? I guess it doesn't really matter since I'm going to remove it in #98754, but I wonder if it can come up in diagnostics for generators or something like that?
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.
well so the only reason we weren't printing this in the first place was because of another bug... the bound lifetimes are there though
i mean i guess we could filter them out or something 🤷
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.
hmm, sure - I think what's confusing is that the lifetimes don't appear in the impl Future
pretty printing, only the for
binder.
maybe that's ok though and this gives a hint of which lifetimes are captured? I don't feel super strongly either way
Before, we would render
impl for<'a> Trait<'a>
likeimpl Trait<for<'a> 'a>
, lol.