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 missing lifetime in opaque and trait object return types #72543

Merged
merged 10 commits into from
May 31, 2020

Conversation

estebank
Copy link
Contributor

When encountering an opaque closure return type that needs to bound a
lifetime to the function's arguments, including borrows and type params,
provide appropriate suggestions that lead to working code.

Get the user from

fn foo<G, T>(g: G, dest: &mut T) -> impl FnOnce()
where
    G: Get<T>
{
    move || {
        *dest = g.get();
    }
}

to

fn foo<'a, G: 'a, T>(g: G, dest: &'a mut T) -> impl FnOnce() +'a
where
    G: Get<T>
{
    move || {
        *dest = g.get();
    }
}

@rust-highfive

This comment has been minimized.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 24, 2020
@estebank estebank force-pushed the opaque-missing-lts-in-fn branch from 9e787bc to e1382d8 Compare May 24, 2020 19:32
@estebank
Copy link
Contributor Author

The nll output is as good as the new regular output. I'll take a look at that later.

@estebank estebank force-pushed the opaque-missing-lts-in-fn branch from e1382d8 to c423748 Compare May 24, 2020 20:42
@eddyb

This comment has been minimized.

@rust-highfive rust-highfive assigned nikomatsakis and unassigned eddyb May 25, 2020
@estebank estebank force-pushed the opaque-missing-lts-in-fn branch from d45607e to 4ff2398 Compare May 26, 2020 20:27
@estebank estebank changed the title Account for missing lifetime in opaque return type Account for missing lifetime in opaque and trait object return types May 26, 2020
| ---------^^^^^-
| | |
| | ...but this borrow...
| this evaluates to the `'static` lifetime...
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, this kind of prints like a "jumble" to me...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer to keep output closer to E0621 but have trouble not "hiding" conceptual information that is actually necessary to ever learn why the + '_ fixes it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we could print this?

error[E0621]: non-static reference captured in `dyn Any` type without explicit bound
  --> $DIR/issue-16922.rs:4:5
   |
LL | fn foo<T: Any>(value: &T) -> Box<dyn Any> {
   |                       -- data with this lifetime...
LL |     Box::new(value) as Box<dyn Any>
   |     ^^^^^^^^^^^^^^^ is captured in a `dyn Any` type here with the default `'static` bound
help: for a `dyn` type to capture a non-static reference, it needs an explicit bound, as shown
   |
LL | fn foo<T: Any>(value: &T) -> Box<dyn Any + '_> {
   |                             

It seems like we might know everything we need to know to "special case" this scenario, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it should be possible, yes, probably as a new nice_region_error impl.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about the following?

error: cannot infer an appropriate lifetime
 --> src/test/ui/issues/issue-16922.rs:4:14
  |
3 | fn foo<T: Any>(value: &T) -> Box<dyn Any> {
  |                       -- data with this lifetime...
4 |     Box::new(value) as Box<dyn Any>
  |     ---------^^^^^-
  |     |        |
  |     |        ...is captured here...
  |     ...and required to be `'static` by this
  |
help: to permit non-static references in a `dyn Trait` value, you can add an explicit `the anonymous lifetime #1 defined on the function body at 3:1` bound
  |
3 | fn foo<T: Any>(value: &T) -> Box<dyn Any + '_> {
  |                                          ^^^^

Copy link
Contributor

Choose a reason for hiding this comment

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

this looks good too, though it still has the "jumbled" look I was hoping to avoid

Copy link
Contributor

Choose a reason for hiding this comment

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

it does give more information

Copy link
Contributor Author

@estebank estebank May 28, 2020

Choose a reason for hiding this comment

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

I think it looks good enough
Screen Shot 2020-05-28 at 3 49 18 PM
but I could further tweak it to be

error: cannot infer an appropriate lifetime
 --> src/test/ui/issues/issue-16922.rs:4:14
  |
3 | fn foo<T: Any>(value: &T) -> Box<dyn Any> {
  |                       -- data with this lifetime...
4 |     Box::new(value) as Box<dyn Any>
  |              ^^^^^ ...is captured here...
note: ...and required to be `'static` by this
  |
4 |     Box::new(value) as Box<dyn Any>
  |     ^^^^^^^^^^^^^^^
help: to permit non-static references in a `dyn Trait` value, you can add an explicit `the anonymous lifetime #1 defined on the function body at 3:1` bound
  |
3 | fn foo<T: Any>(value: &T) -> Box<dyn Any + '_> {
  |                                          ^^^^

src/test/ui/issues/issue-16922.stderr Outdated Show resolved Hide resolved
src/librustc_infer/infer/error_reporting/mod.rs Outdated Show resolved Hide resolved
src/librustc_infer/infer/error_reporting/mod.rs Outdated Show resolved Hide resolved
Comment on lines 35 to 32
help: you can add a bound to the returned `dyn Trait` to make it last less than `'static` and match the anonymous lifetime #1 defined on the function body at 17:1
|
LL | fn c(v: &[u8]) -> Box<dyn Foo + '_> {
| ^^^^
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I change this to be "to make it require less than 'static" instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I might say

help: to permit non-static references in a dyn Trait value, you need to add an explicit bound, as shown

or

help: consider adding an explicit bound to the dyn Trait as shown, which is used to declare that the dyn Trait may capture non-static references

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the "as shown" something you'd like to add to all suggestions? In my mind it is implied always, and might not work nicely with the presentation in 3rd party tools (non-CLI uses).

I'm cooking up a commit to make this be "to permit non-static references in a dyn Trait value, you can add an explicit bound". WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

you can leave out the "as shown", I'm not sure how implicit it really is, but I can believe it

@estebank
Copy link
Contributor Author

Check the last commit with some of your review comments addressed.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 29, 2020

📌 Commit ff82af1b25443de4e3af42403b0d89c770126169 has been approved by nikomatsakis

@bors bors added 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-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2020
@bors
Copy link
Contributor

bors commented May 30, 2020

☔ The latest upstream changes (presumably #72756) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 30, 2020
estebank added 5 commits May 30, 2020 10:19
When encountering an opaque closure return type that needs to bound a
lifetime to the function's arguments, including borrows and type params,
provide appropriate suggestions that lead to working code.

Get the user from

```rust
fn foo<G, T>(g: G, dest: &mut T) -> impl FnOnce()
where
    G: Get<T>
{
    move || {
        *dest = g.get();
    }
}
```

to

```rust
fn foo<'a, G: 'a, T>(g: G, dest: &'a mut T) -> impl FnOnce() +'a
where
    G: Get<T>
{
    move || {
        *dest = g.get();
    }
}
```
…ing opaque return type

Go from

```
error[E0495]: cannot infer an appropriate lifetime due to conflicting requirements
  --> file8.rs:22:5
   |
22 | /     move || {
23 | |         *dest = g.get();
24 | |     }
   | |_____^
   |
note: first, the lifetime cannot outlive the anonymous lifetime rust-lang#1 defined on the function body at 18:1...
  --> file8.rs:18:1
   |
18 | / fn bat<'a, G: 'a, T>(g: G, dest: &mut T) -> impl FnOnce() + '_ + 'a
19 | | where
20 | |     G: Get<T>
21 | | {
...  |
24 | |     }
25 | | }
   | |_^
note: ...so that the types are compatible
  --> file8.rs:22:5
   |
22 | /     move || { //~ ERROR cannot infer an appropriate lifetime
23 | |         *dest = g.get();
24 | |     }
   | |_____^
   = note: expected  `&mut T`
              found  `&mut T`
note: but, the lifetime must be valid for the lifetime `'a` as defined on the function body at 18:8...
  --> file8.rs:18:8
   |
18 | fn bat<'a, G: 'a, T>(g: G, dest: &mut T) -> impl FnOnce() + '_ + 'a
   |        ^^
note: ...so that return value is valid for the call
  --> file8.rs:18:45
   |
18 | fn bat<'a, G: 'a, T>(g: G, dest: &mut T) -> impl FnOnce() + '_ + 'a
   |                                             ^^^^^^^^^^^^^^^^^^^^^^^
```

to

```
error[E0621]: explicit lifetime required in the type of `dest`
  --> file8.rs:18:45
   |
18 | fn bat<'a, G: 'a, T>(g: G, dest: &mut T) -> impl FnOnce() + '_ + 'a
   |                                  ------     ^^^^^^^^^^^^^^^^^^^^^^^ lifetime `'a` required
   |                                  |
   |                                  help: add explicit lifetime `'a` to the type of `dest`: `&'a mut T`
   ```
Provide a suggestion for `dyn Trait + '_` when possible.
@estebank estebank force-pushed the opaque-missing-lts-in-fn branch from ff82af1 to 83f6f22 Compare May 30, 2020 17:22
@estebank
Copy link
Contributor Author

@bors r=nikomatsakis

rebased

@bors
Copy link
Contributor

bors commented May 30, 2020

📌 Commit 83f6f22 has been approved by nikomatsakis

@bors bors added 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 30, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request May 30, 2020
Rollup of 13 pull requests

Successful merges:

 - rust-lang#72543 (Account for missing lifetime in opaque and trait object return types)
 - rust-lang#72625 (Improve inline asm error diagnostics)
 - rust-lang#72637 (expand `env!` with def-site context)
 - rust-lang#72650 (Sort sidebar elements)
 - rust-lang#72657 (Allow types (with lifetimes/generics) in impl_lint_pass)
 - rust-lang#72666 (Add -Z profile-emit=<path> for Gcov gcda output.)
 - rust-lang#72668 (Fix missing parentheses Fn notation error)
 - rust-lang#72669 (rustc_session: Cleanup session creation)
 - rust-lang#72728 (Make bootstrap aware of relative libdir in stage0 compiler)
 - rust-lang#72757 (rustc_lexer: Optimize shebang detection slightly)
 - rust-lang#72772 (miri validation: clarify valid values of 'char')
 - rust-lang#72773 (Fix is_char_boundary documentation)
 - rust-lang#72777 (rustdoc: remove calls to `local_def_id_from_node_id`)

Failed merges:

r? @ghost
@bors bors merged commit f1661d2 into rust-lang:master May 31, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 11, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 12, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 12, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 13, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 13, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 13, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 13, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 13, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 13, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 15, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 16, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 16, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 16, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 16, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 17, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
tmandry added a commit to tmandry/rust that referenced this pull request Jun 17, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 18, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 18, 2020
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
@estebank estebank deleted the opaque-missing-lts-in-fn branch November 9, 2023 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants