Account for HR lifetimes when suggesting introduction of named lifetime by estebank · Pull Request #68583 · rust-lang/rust

estebank · 2020-01-27T20:55:37Z

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 · 2020-01-27T20:55:41Z

r? @petrochenkov

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

src/librustc_hir/hir.rs

src/librustc_resolve/diagnostics.rs

eddyb · 2020-01-27T21:45:06Z

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?

src/librustc_resolve/diagnostics.rs

estebank · 2020-01-27T22:13:17Z

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

src/librustc_resolve/diagnostics.rs

src/librustc_resolve/lifetimes.rs

src/librustc_resolve/diagnostics.rs

src/librustc_resolve/lifetimes.rs

src/test/ui/underscore-lifetime/in-fn-return-illegal.stderr

Centril · 2020-01-28T23:27:44Z

src/test/ui/generic/generic-extern-lifetime.stderr

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.

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 · 2020-01-29T20:14:07Z

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

estebank · 2020-01-31T18:28:47Z

Would you mind to r? @oli-obk

oli-obk · 2020-02-04T17:41:55Z

@bors r+

bors · 2020-02-04T17:41:56Z

📌 Commit 4310b74 has been approved by oli-obk

rust-highfive · 2020-02-05T06:48:30Z

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 · 2020-02-05T06:58:43Z

@bors retry

bors · 2020-02-05T08:39:03Z

⌛ Testing commit 4310b74 with merge b4097ad7cc9693bf7845908726213028ead63854...

Centril · 2020-02-05T08:40:58Z

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

…uments

Address rust-lang#49287

estebank · 2020-02-05T18:59:48Z

Rebased and fixed test.

@bors r=oli-obk

bors · 2020-02-05T18:59:50Z

📌 Commit 609a374 has been approved by oli-obk

bors · 2020-02-06T05:19:58Z

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

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 · 2020-02-06T08:47:52Z

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

rust-highfive assigned petrochenkov Jan 27, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 27, 2020

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

estebank mentioned this pull request Jan 27, 2020

Tweak lifetime definition errors #68267

Merged

eddyb reviewed Jan 27, 2020

View reviewed changes

src/librustc_hir/hir.rs Outdated Show resolved Hide resolved

eddyb reviewed Jan 27, 2020

View reviewed changes

src/librustc_resolve/diagnostics.rs Outdated Show resolved Hide resolved

varkor reviewed Jan 27, 2020

View reviewed changes

src/librustc_resolve/diagnostics.rs Outdated Show resolved Hide resolved

Centril reviewed Jan 27, 2020

View reviewed changes

src/librustc_resolve/diagnostics.rs Outdated Show resolved Hide resolved

src/librustc_resolve/lifetimes.rs Outdated Show resolved Hide resolved

Centril reviewed Jan 28, 2020

View reviewed changes

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

Centril reviewed Jan 28, 2020

View reviewed changes

src/librustc_resolve/diagnostics.rs Outdated Show resolved Hide resolved

src/librustc_resolve/lifetimes.rs Outdated Show resolved Hide resolved

petrochenkov reviewed Jan 28, 2020

View reviewed changes

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 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

eddyb reviewed Jan 28, 2020

View reviewed changes

src/test/ui/underscore-lifetime/in-fn-return-illegal.stderr Outdated Show resolved Hide resolved

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

This comment has been minimized.

Sign in to view

Centril reviewed Jan 28, 2020

View reviewed changes

estebank force-pushed the hrlt branch from ed60d76 to c2f0829 Compare January 29, 2020 01:54

rust-highfive unassigned petrochenkov Jan 29, 2020

estebank mentioned this pull request Jan 30, 2020

Argument-position impl Trait requires a named lifetime #49287

Open

estebank force-pushed the hrlt branch from c2f0829 to 4310b74 Compare January 30, 2020 19:54

rust-highfive assigned oli-obk Jan 31, 2020

estebank added 12 commits February 5, 2020 10:32

Account for HKTB when suggesting introduction of named lifetime

183dfac

Use spans for input borrowed types unrelated to return type

70dbf55

When suggesting lifetimes, propose adding the new lifetime to all arg…

7e14643

…uments

Suggest 'r instead of 'lifetime

fa45941

review comments

2100b31

Account for '_ in suggestions

ba3b44c

Account for fn() types in lifetime suggestions

92505df

review comments: wording

30d9278

review comments

49f9bf8

Move code to diagnostics.rs

1beac2b

Account for impl Trait

96bbb0d

Address rust-lang#49287

Fix test

609a374

estebank force-pushed the hrlt branch from 4310b74 to 609a374 Compare February 5, 2020 18:59

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 6, 2020

bors merged commit 609a374 into rust-lang:master Feb 6, 2020

estebank mentioned this pull request Feb 6, 2020

Invalid suggestion on missing lifetime in impl trait associated type #68904

Closed

Uh oh!

Conversation

estebank commented Jan 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Jan 27, 2020

Uh oh!

Uh oh!

Uh oh!

eddyb commented Jan 27, 2020

Uh oh!

Uh oh!

estebank commented Jan 27, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Centril Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

estebank Jan 29, 2020

Choose a reason for hiding this comment

Uh oh!

petrochenkov commented Jan 29, 2020

Uh oh!

estebank commented Jan 31, 2020

Uh oh!

oli-obk commented Feb 4, 2020

Uh oh!

bors commented Feb 4, 2020

Uh oh!

rust-highfive commented Feb 5, 2020

Uh oh!

estebank commented Feb 5, 2020

Uh oh!

bors commented Feb 5, 2020

Uh oh!

Centril commented Feb 5, 2020

Uh oh!

estebank commented Feb 5, 2020

Uh oh!

bors commented Feb 5, 2020

Uh oh!

bors commented Feb 6, 2020

Uh oh!

bors commented Feb 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

estebank commented Jan 27, 2020 •

edited

Loading