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

tracking issue for lifetime inference error work (E0495) #42516

Closed
12 of 19 tasks
nikomatsakis opened this issue Jun 7, 2017 · 13 comments
Closed
12 of 19 tasks

tracking issue for lifetime inference error work (E0495) #42516

nikomatsakis opened this issue Jun 7, 2017 · 13 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. metabug Issues about issues themselves ("bugs about bugs") P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-diagnostics Working group: Diagnostics

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jun 7, 2017

This is a general tracking issue for the work on improving lifetime errors (in particular the "cannot infer lifetime" error E0495, but also including other errors). This is a rather large project, since there are many different classes of errors, so part of the work is finding scenarios and making more targeted error messages for them.

In general, I am trying to track errors and brainstorm here. There is also an etherpad where I have done some note-taking.

Current road-map

The plan is to have three major kinds of errors, delivered in order of preference. The basic work here is done, but there remain gaps to be covered. If you're interested in helping out, check out the issues associated with the unchecked check boxes!

Examples in need of analysis

Future plans

  • Improve how lifetime inference selects which edges in the region inference graph are at fault. The code for finding the regions that conflict seems to work quite well -- but the code to select which edges to blame is hopeless. I think we need to look specifically for cases where the user "puts things in tension" (e.g., foo(a) puts the formal declarations of foo in tension with the type of a). I'll try to write up these thoughts in a bit more detail later.

Other issues

Here are various issues that I have subsumed into this one. These are a checklist because I want to go over them and extract any key examples into this issue and then close them. Feel free to do so (in the comments).

@nikomatsakis nikomatsakis added A-diagnostics Area: Messages for errors, warnings, and lints metabug Issues about issues themselves ("bugs about bugs") T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 7, 2017
frewsxcv added a commit to frewsxcv/rust that referenced this issue Jun 29, 2017
Adding diagnostic code 0611 for lifetime errors with one named, one anonymous lifetime parameter

This is a fix for rust-lang#42517
Note that this only handles the above case for **function declarations** and **traits**.
`impl items` and `closures` will be handled in a later PR.
Example
```
fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
    if x > y { x } else { y }
}
```
now displays the following error message. ui tests have been added for the same.
```
error[E0611]: explicit lifetime required in the type of `x`
11 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
   |                     ^ consider changing the type of `x` to `&'a i32`
12 |     if x > y { x } else { y }
   |                  - lifetime `'a` required
```
rust-lang#42516
r? @nikomatsakis
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 30, 2017
Adding diagnostic code 0611 for lifetime errors with one named, one anonymous lifetime parameter

This is a fix for rust-lang#42517
Note that this only handles the above case for **function declarations** and **traits**.
`impl items` and `closures` will be handled in a later PR.
Example
```
fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
    if x > y { x } else { y }
}
```
now displays the following error message. ui tests have been added for the same.
```
error[E0611]: explicit lifetime required in the type of `x`
11 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
   |                     ^ consider changing the type of `x` to `&'a i32`
12 |     if x > y { x } else { y }
   |                  - lifetime `'a` required
```
rust-lang#42516
r? @nikomatsakis
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 30, 2017
Adding diagnostic code 0611 for lifetime errors with one named, one anonymous lifetime parameter

This is a fix for rust-lang#42517
Note that this only handles the above case for **function declarations** and **traits**.
`impl items` and `closures` will be handled in a later PR.
Example
```
fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
    if x > y { x } else { y }
}
```
now displays the following error message. ui tests have been added for the same.
```
error[E0611]: explicit lifetime required in the type of `x`
11 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
   |                     ^ consider changing the type of `x` to `&'a i32`
12 |     if x > y { x } else { y }
   |                  - lifetime `'a` required
```
rust-lang#42516
r? @nikomatsakis
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 30, 2017
Adding diagnostic code 0611 for lifetime errors with one named, one anonymous lifetime parameter

This is a fix for rust-lang#42517
Note that this only handles the above case for **function declarations** and **traits**.
`impl items` and `closures` will be handled in a later PR.
Example
```
fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
    if x > y { x } else { y }
}
```
now displays the following error message. ui tests have been added for the same.
```
error[E0611]: explicit lifetime required in the type of `x`
11 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
   |                     ^ consider changing the type of `x` to `&'a i32`
12 |     if x > y { x } else { y }
   |                  - lifetime `'a` required
```
rust-lang#42516
r? @nikomatsakis
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 30, 2017
Adding diagnostic code 0611 for lifetime errors with one named, one anonymous lifetime parameter

This is a fix for rust-lang#42517
Note that this only handles the above case for **function declarations** and **traits**.
`impl items` and `closures` will be handled in a later PR.
Example
```
fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
    if x > y { x } else { y }
}
```
now displays the following error message. ui tests have been added for the same.
```
error[E0611]: explicit lifetime required in the type of `x`
11 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
   |                     ^ consider changing the type of `x` to `&'a i32`
12 |     if x > y { x } else { y }
   |                  - lifetime `'a` required
```
rust-lang#42516
r? @nikomatsakis
@cengiz-io
Copy link
Contributor

@nikomatsakis

I'd like to take at least one of these and help!

@gaurikholkar-zz
Copy link

#43288

@gaurikholkar-zz
Copy link

#43433

@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 26, 2017
@eira-fransham
Copy link

I was referred here for a comment I made about how lifetime errors involving complex elision should print the fully elided parameters. By complex elision I mean elision that doesn't just result in the same lifetime for every param, so that's elision of some parameters when you have other parameters like fn foo<'a>(_: &&'a Foo) and elision involving &self like fn foo(&self, _: &Foo) -> &Foo. For these, respectively, it should print something along the lines of:

note: fully inferred function definition
      fn foo<'a, 'anon0>(_: &'anon0 &'a Foo) { ... }

and

note: fully inferred function definition
      fn foo<'anon0, 'anon1>(&'anon0 self, _: &'anon1 Foo) -> &'anon0 Foo { ... }

@nikomatsakis
Copy link
Contributor Author

@Vurich it'd be good to see an example of a case where you think that'd be useful. We've been trying so far to make more targeted suggestions -- see the updated game plan above -- but I could see that it might be useful to show the "fully inferred" function definition in some cases.

@PieterPenninckx
Copy link
Contributor

@nikomatsakis I think Vurich's original example was the following (taken from his comment)

struct Bar<'a, T: 'a>(&'a mut T);
struct Foo<'a, T: 'a>(&'a mut Bar<'a, T>);

fn make_foo<'a, T>(m: &'a mut Bar<T>) -> Foo<'a, T> {
    Foo(m)
}

When I try to compile this, the error message already mentions an anonymous lifetime:

note: but, the lifetime must be valid for the anonymous lifetime #1 defined on the function body at 4:1...
 --> test.rs:4:1
  |
4 | / fn make_foo<'a, T>(m: &'a mut Bar<T>) -> Foo<'a, T> {
5 | |     Foo(m)
6 | | }
  | |_^

I think this would be the perfect place to show the fully inferred function definition ("anonymous lifetime #1" is a little vague):

note: but, the lifetime must be valid for the anonymous lifetime 'anon1 defined on the function body at 4:1...
 --> test.rs:4:1
  |
4 | / fn make_foo<'a, T>(m: &'a mut Bar<T>) -> Foo<'a, T> {
5 | |     Foo(m)
6 | | }
  | |_^
note: fully inferred function definition of test.rs:4:1
  | fn make_foo<'a, 'anon0, T>(m: &'a mut Bar<'anon0, T>) -> Foo<'a, T>

@PieterPenninckx
Copy link
Contributor

I had a problem similar to @Vurich 's example when I did my first experiments with trait objects. A simplified version of the code is the following:

trait T {}

struct TT<'a> { x: &'a i8 }

impl<'a> T for TT<'a> {}

struct S { data: Vec<Box<T>> }

impl S {
    fn new() -> Self { S{data: Vec::new()} }
    fn push(&mut self, t: Box<T>) { self.data.push(t); }
}

fn boxed_trait_object() {
    let i = 4;
    let t = TT{x: &i};
    let mut s = S::new();
    s.push(Box::new(t)); // Error: `i` does not live long enough
                         // Note: borrowed value must be valid 
                         //   for the static lifetime...
}

I started searching my code for the 'static lifetime to find out how why I got this error. After some unsuccessful attempts, I switched strategy and tried to figure out what potential bug the compiler stopped me from making. Eventually I figured out that the type S should have a lifetime parameter because s borrows i and that hence the trait object should have a lifetime parameter. I did an internet search for "rust how to add a lifetime parameter to a trait object" or similar because I didn't know that the proper terminology is "lifetime bound for trait object". I added the lifetime bound to the trait object and made the compiler happy. It took me about a whole afternoon to fix this compiler error.

The only solution I can think of to make it clearer what happens in situations like this is to warn on default lifetimes for trait objects. I know that RFC rust-lang/rfcs#2115 proposes to warn on leaving off lifetime parameters on structs. Is a similar warning planned for leaving off lifetime bounds on trait objects?

@nikomatsakis nikomatsakis added the WG-diagnostics Working group: Diagnostics label Sep 20, 2017
@estebank
Copy link
Contributor

I believe #44684 might be a duplicate of #42703. Leaving open for now until verifying that this is the case.

@vext01
Copy link
Contributor

vext01 commented Nov 30, 2017

Thanks for working on this :)

I think the lifetime errors are the biggest usability hurdle with Rust.

/me subscribes.

@estebank
Copy link
Contributor

estebank commented Feb 10, 2018

CC #18759, #30257, #37879

@pnkfelix
Copy link
Member

It would be good to try to provide a catalogue/summary of the different cases here, and perhaps check how each one changes, if at all, when you switch on "full" NLL mode (as opposed to the NLL migrate mode that is the current default in nightly for all editions).

(One switches on "full" NLL via #![feature(nll)] today, or alternatively via -Z borrowck=mir if you want to do it via RUSTFLAGS)

@pnkfelix pnkfelix self-assigned this Jun 21, 2019
@pnkfelix
Copy link
Member

triage: realistically we are not (and cannot) prioritize this effort (beyond what we have already done that is). Downgrading from P-high to P-medium.

@pnkfelix pnkfelix added P-medium Medium priority and removed P-high High priority labels Sep 19, 2019
@pnkfelix
Copy link
Member

Discussed at T-compiler backlog bonanza.

We think C-tracking-issue is meant to track issues with some concrete feature or deliverable. This issue seems like its tracking something more abstract, a grab bag of diagnostic changes all related to lifetime inference.

The listed work does look like something that should be done, but we don't think there's benefit in leaving this issue open, because there will always be ways to improve lifetime diagnostics, so its not clear what criteria we would use for saying that this is "done"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. metabug Issues about issues themselves ("bugs about bugs") P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-diagnostics Working group: Diagnostics
Projects
None yet
Development

No branches or pull requests

9 participants