Skip to content
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

Account for HR lifetimes when suggesting introduction of named lifetime #68583

Merged
merged 12 commits into from
Feb 6, 2020

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jan 27, 2020

error[E0106]: missing lifetime specifier
 --> src/test/ui/suggestions/fn-missing-lifetime-in-item.rs:2:32
  |
2 | struct S2<F: Fn(&i32, &i32) -> &i32>(F);
  |                 ----  ----     ^ expected named lifetime parameter
  |
  = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from argument 1 or argument 2
  = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
help: consider making the bound lifetime-generic with a new `'a` lifetime
  |
2 | struct S2<F: for<'a> Fn(&'a i32, &'a i32) -> &'a i32>(F);
  |              ^^^^^^^    ^^^^^^^  ^^^^^^^     ^^^
help: consider introducing a named lifetime parameter
  |
2 | struct S2<'a, F: Fn(&'a i32, &'a i32) -> &'a i32>(F);=
  |           ^^^       ^^^^^^^  ^^^^^^^     ^^^

Follow up to #68267. Addresses the diagnostics part of #49287.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 27, 2020
@estebank estebank changed the title Account for HRTB when suggesting introduction of named lifetime Account for HR lifetimes when suggesting introduction of named lifetime Jan 27, 2020
src/librustc_hir/hir.rs Outdated Show resolved Hide resolved
@eddyb
Copy link
Member

eddyb commented Jan 27, 2020

cc @rust-lang/compiler I know we removed some lifetime-related automatic suggestions, are these related? Also, do people have opinions on what we should be showing?

@estebank
Copy link
Contributor Author

cc @rust-lang/wg-diagnostics as well.

src/librustc_resolve/diagnostics.rs Outdated Show resolved Hide resolved
src/librustc_resolve/lifetimes.rs Outdated Show resolved Hide resolved
src/librustc_resolve/lifetimes.rs Outdated Show resolved Hide resolved
src/librustc_resolve/lifetimes.rs Outdated Show resolved Hide resolved
src/librustc_resolve/lifetimes.rs Outdated Show resolved Hide resolved
src/librustc_resolve/diagnostics.rs Outdated Show resolved Hide resolved
src/librustc_resolve/lifetimes.rs Outdated Show resolved Hide resolved
src/librustc_resolve/lifetimes.rs Outdated Show resolved Hide resolved
src/librustc_resolve/lifetimes.rs Outdated Show resolved Hide resolved
src/librustc_resolve/lifetimes.rs Outdated Show resolved Hide resolved
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 28, 2020
@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 28, 2020
@rust-highfive

This comment has been minimized.

@@ -9,12 +9,24 @@ error[E0261]: use of undeclared lifetime name `'a`
|
LL | pub fn life4<'b>(x: for<'c> fn(&'a i32));
| ^^ undeclared lifetime
|
= note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, can this note be moved to after the help:? The blog post analogy to this would be having the "read more" link before the introduction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is part of a separate bug where all suggestions come always at the end. I would rather tackle that in a separate PR, ideally after fully migrating to annotate_snippets.

@petrochenkov
Copy link
Contributor

r? @rust-lang/wg-diagnostics
Could someone review the actual logic?

@estebank
Copy link
Contributor Author

Would you mind to r? @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Feb 4, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Feb 4, 2020

📌 Commit 4310b74 has been approved by oli-obk

@rust-highfive
Copy link
Collaborator

Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

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 @TimNN. (Feature Requests)

@estebank
Copy link
Contributor Author

estebank commented Feb 5, 2020

@bors retry

@bors
Copy link
Contributor

bors commented Feb 5, 2020

⌛ Testing commit 4310b74 with merge b4097ad7cc9693bf7845908726213028ead63854...

@Centril
Copy link
Contributor

Centril commented Feb 5, 2020

Failed in #68852 (comment), @bors r- retry

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 5, 2020
@estebank
Copy link
Contributor Author

estebank commented Feb 5, 2020

Rebased and fixed test.

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Feb 5, 2020

📌 Commit 609a374 has been approved by oli-obk

@bors
Copy link
Contributor

bors commented Feb 6, 2020

⌛ Testing commit 609a374 with merge 333c32a...

bors added a commit that referenced this pull request Feb 6, 2020
Account for HR lifetimes when suggesting introduction of named lifetime

```
error[E0106]: missing lifetime specifier
 --> src/test/ui/suggestions/fn-missing-lifetime-in-item.rs:2:32
  |
2 | struct S2<F: Fn(&i32, &i32) -> &i32>(F);
  |                 ----  ----     ^ expected named lifetime parameter
  |
  = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from argument 1 or argument 2
  = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
help: consider making the bound lifetime-generic with a new `'a` lifetime
  |
2 | struct S2<F: for<'a> Fn(&'a i32, &'a i32) -> &'a i32>(F);
  |              ^^^^^^^    ^^^^^^^  ^^^^^^^     ^^^
help: consider introducing a named lifetime parameter
  |
2 | struct S2<'a, F: Fn(&'a i32, &'a i32) -> &'a i32>(F);=
  |           ^^^       ^^^^^^^  ^^^^^^^     ^^^
```

Follow up to #68267. Addresses the diagnostics part of #49287.
@bors
Copy link
Contributor

bors commented Feb 6, 2020

☀️ Test successful - checks-azure
Approved by: oli-obk
Pushing 333c32a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 6, 2020
@bors bors merged commit 609a374 into rust-lang:master Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants