Presenting "lifetime inference" errors better in NLL

[nikomatsakis]

So, one of the big topics in NLL is how to handle lifetime errors – specifically, I am taking aim at that class of errors that currently show up as “unable to infer lifetime for foo”. I want to see if, with NLL, we can do a better job of presenting that information. We’ve been working on a template for such errors and I’d like to get some feedback.

Here is an example test:

fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
    Box::new(items.iter())
}

Here is the current error message that you get when you try to compile this test:

error[E0495]: cannot infer an appropriate lifetime for autoref due to conflicting requirements
  --> underscore-lifetime/dyn-trait-underscore.rs:18:20
   |
17 |     Box::new(items.iter())
   |                    ^^^^
   |
note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the function body at 16:1...
  --> underscore-lifetime/dyn-trait-underscore.rs:16:1
   |
16 | / fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
17 | |     Box::new(items.iter())
17 | | }
   | |_^
note: ...so that reference does not outlive borrowed content
  --> underscore-lifetime/dyn-trait-underscore.rs:18:14
   |
17 |     Box::new(items.iter())
   |              ^^^^^
   = note: but, the lifetime must be valid for the static lifetime...
   = note: ...so that the expression is assignable:
           expected std::boxed::Box<(dyn std::iter::Iterator<Item=&T> + 'static)>
              found std::boxed::Box<dyn std::iter::Iterator<Item=&T>>

My experience has been that most people just have their eyes glaze over when confronted with all this stuff.

General plan

In general, my plan for region errors is two-fold:

• First, we should identify as many special cases as we can and give highly tailored messages.
• But we also need a general fallback plan.

It’s the latter point I want to talk about here: a kind of general fallback for region errors when we don’t have a specific error message at hand. I think this “general plan” probably can also have a number of smaller helpful notes that we add from time to time (in fact, my example below has one).

A new template for general errors

Before I explain how my new template for “general errors” works, let me show you how it might look on the error above. I want to give you this first so as not to bias you:

error: unsatisfied lifetime constraints
  --> underscore-lifetime/dyn-trait-underscore.rs:18:5
   |
16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |         -----           -------------------------- defaults to: `Box<dyn Iterator<Item = &T> + 'static>`
   |         |
   |         the fully elaborated type of `items` is `&'1 [T]`
17 |     Box::new(items.iter())
   |     ^^^^^^^^^^^^^^^^^^^^^^ casting this value to `Box<dyn Iterator<Item = &T>>` requires `'1` to be `'static`

The basic plan here:

• first, to first give names to anonymous lifetimes via “fully elaborated types”. So here the lifetime of the items argument is “elaborated” and named '1 – this is intentionally not valid syntax, for a variety of reasons.
• second, to identify key points within the function and talk about the constraints they introduce. In this case, the point we identify is the cast to Box<dyn Iterator<Item = &T>>, but in general there may be more than one such case
• third, try to add add’l helpful tips for defaults and things that may not be obvious, such as the fact that Box<dyn Iterator> defaults to Box<dyn Iterator + 'static>

This error message feels much better than the original, but still ungreat. It feels like a system that one could learn to understand as you become more experienced, but is probably not very good for newbies. That’s progress – and I’ll take it – but it’d be nice if we could do better.

Anyway, I wanted to open up this thread to chat with folks (cc @davidtwco, @ekuber) and try to brainstorm how we might do better here.

Implementation plan

In terms of implementation, I am hoping to land https://github.com/rust-lang/rust/pull/52021 ASAP, which lays some of the foundation here. Then I thought I would open up some issues and try to keep things moving towards the error I gave above. In the meantime, maybe we can discuss come up with a better overall system for talking about these sorts of errors, and we can adjust things to match.

[nikomatsakis]

One immediate thing here is that we might special case regions, like the one in this example, that are the lifetime of the “main reference”. i.e., if we have an argument foo: &T and we are talking about the lifetime of that reference, we can maybe just talk about "lifetime of foo" instead of giving it a numbered name.

Example:

error: unsatisfied lifetime constraints
  --> underscore-lifetime/dyn-trait-underscore.rs:18:5
   |
16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |                         -------------------------- defaults to: `Box<dyn Iterator<Item = &T> + 'static>`
17 |     Box::new(items.iter())
   |     ^^^^^^^^^^^^^^^^^^^^^^ casting this value to `Box<dyn Iterator<Item=&T>>` requires  `items` to have `'static` lifetime

[nikomatsakis]

Some things I am not keen on:

• “unsatisfied lifetime constraints” as the header
  • probably this should be drawn instead from the “primary” error
• the phrase “fully elaborated type” – I like the idea, but it feels jargon-y. I can’t think of a better way to present such types though.
  • I do wonder if the --explain can explain “how to read” these errors, and if that might help a bit?

[ekuber]

• “unsatisfied lifetime constraints” as the header
  • probably this should be drawn instead from the “primary” error

I feel the best option we have is to land the PR and iterate over it, the foundation there is good enough to work with.

• the phrase “fully elaborated type” – I like the idea, but it feels jargon-y. I can’t think of a better way to present such types though.

This is a problem throughout a lot of diagnostics… We have to unify the messaging. I’m trying to take inspiration from the book, but this is probably we should sit down with Steve, Oli and Jonathan to brainstorm how to refer to these concepts without using jargon.

• I do wonder if the --explain can explain “how to read” these errors, and if that might help a bit?

The only problem I see is the explosion of variations of lifetime errors we give. They are great in that the well trodden path usually has great, very specific readable diagnostics, but it does make it harder to write documentation for them. That said, at least we can refer to the diagnostic code to see all the variations that we’re aware of, so maybe that’s a silver lining?

• first, to first give names to anonymous lifetimes via “fully elaborated types”. So here the lifetime of the items argument is “elaborated” and named '1 – this is intentionally not valid syntax, for a variety of reasons.

I’m not sure what I think of this. Giving them a name to be able to refer to them seems like an easy solution to the “how do we talk about anonymous lifetimes” problem, but I feel that it’ll pay off to search for wording that will not require naming them. (This presumes that such a wording can be found.)

• second, to identify key points within the function and talk about the constraints they introduce. In this case, the point we identify is the cast to Box<dyn Iterator<Item = &T>> , but in general there may be more than one such case
• third, try to add add’l helpful tips for defaults and things that may not be obvious, such as the fact that Box<dyn Iterator> defaults to Box<dyn Iterator + 'static>

I feel that that is something that can be done through suggestions, but I also question the possibility of changing the ergonomics of the language to assume + '_ instead of 'static for the snippet shown.

[nikomatsakis]

Another few attempts.

The first one is more generic, the second more specialized:

error: `'static` lifetime required
  --> underscore-lifetime/dyn-trait-underscore.rs:18:5
   |
16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |         -----           -------------------------- defaults to: `Box<dyn Iterator<Item = &T> + 'static>`
   |         |
   |         fully elaborated type of `items` is `&'1 [T]`
17 |     Box::new(items.iter())
   |              ^^^^^ `'1` is cast into to a `dyn Iterator` type with a `'static` bound

A more specialized attempt follows. This one relies on us observing that '1 is the argument of a reference and then choosing to conflate items with the name of the lifetime:

error: `items` required to have `'static` lifetime
  --> underscore-lifetime/dyn-trait-underscore.rs:18:5
   |
16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |         -----           -------------------------- defaults to: `Box<dyn Iterator<Item = &T> + 'static>`
   |         |
   |         `items` declared here
17 |     Box::new(items.iter())
   |              ^^^^^ `items` is cast into a `dyn Iterator` type with a `'static` bound

Some thoughts about how we might generate this:

• First, you see that I am highlighting the use of items in both cases, instead of the entire Box::new(items.iter()) expression:
  • I think we can use the fact that '1 (the error region) appears in items to cause us to consider uses of that same variable in MIR as more significant.
• Second, I have suppressed the highlighting of the “implicit cast” that is occurring at the return:
  • This seems like just noise.
• Third, I am identifying arguments of &T type and printing the argument name instead of a synthetic variable
• Fourth, I changed the message around casts (there are many other cases, we should try to come up with a more comprehensive set of examples) to “R1 is cast into a dyn Foo type with a R2 bound”

[nikomatsakis]

This is a problem throughout a lot of diagnostics… We have to unify the messaging. I’m trying to take inspiration from the book, but this is probably we should sit down with Steve, Oli and Jonathan to brainstorm how to refer to these concepts without using jargon.

Yes…I have been wanting to go over the NLL errors in particular and try to whittle down and reduce the amount of terminology involved.

The only problem I see is the explosion of variations of lifetime errors we give.

One thing I have noticed in the compiler is that when the smart errors are wrong, they give too little information. For example, @tmandry was recently showing me this error:

error[E0621]: explicit lifetime required in the type of `tcx`
  --> librustc_traits/implied_outlives_bounds.rs:43:5
   |
37 |       tcx: TyCtxt<'_, 'tcx, 'tcx>,
   |       --- consider changing the type of `tcx` to `rustc::ty::TyCtxt<'tcx, 'tcx, 'tcx>`
...
43 | /     tcx.infer_ctxt()
44 | |        .enter_canonical_trait_query(&goal, |infcx, fulfill_cx, key| {
45 | |            let (param_env, ty) = key.into_parts();
46 | |            compute_implied_outlives_bounds(&infcx, param_env, ty)
47 | |        })
   | |_________^ lifetime `'tcx` required

the problem here was that compute_implied_outlives_bounds had the wrong type (it was InferCtxt<'_, 'tcx, 'tcx> and should have been InferCtxt<'_, 'gcx, 'tcx>).

I think the compiler’s suggestion was good, but it would have been better if it could’ve included more tips on how it came to its conclusions.

This suggests to me that my premise of “specialized errors” is a bit flawed – that is, the ideal might be if more and more we can incorporate the specialized errors into the same basic framework for reporting region errors, so that we can also have information about how the compiler came to its conclusions.

I’m not sure what I think of this. Giving them a name to be able to refer to them seems like an easy solution to the “how do we talk about anonymous lifetimes” problem, but I feel that it’ll pay off to search for wording that will not require naming them. (This presumes that such a wording can be found.)

Yeah, I’ve been going back and forth. I think my conclusion is sort of the same but I look at it differently. That is, giving them names is very hard, because it’s hard to find a wording for it. But if we can find a way to give them names, it’ll make almost everything easier.

That said, names like '1 seem like they are just going to be sort of inherently confusing, which is why I’ve been toying with using the variable name etc.

But still, without some anonymous names, I don’t know how to handle cases like identifying the second argument of TyCtxt<'_, '_, 'tcx>.

[nikomatsakis]

Some experiments around wording. Specifically, if we did want to give a name. I’m consider this example but I’m thinking actually of stuff like TyCtxt:

• fully elaborated type is TyCtxt<'1, 'gcx, 'tcx>

The problem is that I feel like what is missing is some kind of “intro” explaining how these errors work, but I can’t think of a short way to do that. I keep thinking of stuff like this:

16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |         -----
   |         |
   |         the error concerns a lifetime that appears in `items` which we'll call `'1`
   |         the type of `items` is then `&'1 [T]`

Maybe:

16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |         -----
   |         |
   |         consider the lifetime denoted `'1` in the type `&'1 [T]`

Ah, one interesting observation. It’ll be hard to take advantage of it, but:

• In Rust 2018, most anonymous lifetimes have some kind of syntactic marker: it is either '_ or an &.

That means we could do something like this:

16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |                - let's call the lifetime of this reference `'1`

The resulting error:

error: `'static` lifetime required
  --> underscore-lifetime/dyn-trait-underscore.rs:18:5
   |
16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |                -        -------------------------- defaults to: `Box<dyn Iterator<Item = &T> + 'static>`
   |                |
   |                let's call the lifetime of this reference `'1`
17 |     Box::new(items.iter())
   |              ^^^^^ `'1` is cast into to a `dyn Iterator` type with a `'static` bound

Similarly you might have:

error[E0621]: `'tcx` lifetime required
  --> librustc_traits/implied_outlives_bounds.rs:43:5
   |
37 |       tcx: TyCtxt<'_, 'tcx, 'tcx>,
   |                   -- let's call this lifetime `'1`
...
43 | /     tcx.infer_ctxt()
44 | |        .enter_canonical_trait_query(&goal, |infcx, fulfill_cx, key| {
45 | |            let (param_env, ty) = key.into_parts();
46 | |            compute_implied_outlives_bounds(&infcx, param_env, ty)
47 | |        })
   | |_________^ `'1` is required to be `'tcx` here
   |
   | = help:  consider changing the type of `tcx` to `rustc::ty::TyCtxt<'tcx, 'tcx, 'tcx>`

[earthengine]

“unsatisfied lifetime constraints”

My idea: “unsatisfied lifetime constraints” =>

• “lifetime requirements conflicts” : when lifetimes are named but have conflict requirements

• “unnamed lifetime cannot be derived”: when one or more unnamed lifetime cannot be derived from constraints

[nikomatsakis]

“unnamed lifetime cannot be derived”: when one or more unnamed lifetime cannot be derived from constraints

I think this leads one to an inaccurate mental model – that is, the values of unnamed lifetimes (e.g., items: &u32) are not being inferred by the compiler. Rather, we are making sure that items: &u32 will be valid for any lifetime that the caller chooses, because the signatures says that the caller can pick any lifetime. But we have found that the body only works for the lifetime 'static.

It is similar to this situation:

fn foo<T>(t: T) -> u32 {
    t
}

That is, here we said "you can give me any type T and I will you back a u32", but then we are back the variable t – now, sometimes that might be a u32, but not always.

That makes me wonder though if we should pursue this line of explanation. Something like:

error: `'static` lifetime required
  --> underscore-lifetime/dyn-trait-underscore.rs:18:5
   |
16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |         -----           
   |         |
   |         caller of `a` is allowed to pick any lifetime for `items` they want
17 |     Box::new(items.iter())
   |              ^^^^^ but here `items` must have `'static` lifetime to be cast to `dyn Iterator`
   |
   | help: the return type defaults to `Box<dyn Iterator<Item = &T> + 'static>`
   |
16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |                         --------------------------

(OK, I threw in one of the other experiment, which is pulling out some of the hints to make things a bit less busy.)

[nikomatsakis]

One other thing: I have historically felt (and continue to feel!) that lifetime was a poor choice to use here. So I would like to see if we can rework the error messages not to use that word. I think it leads to bad intuitions, since it suggests the “lifetime of the underlying value”. Have to go look at the messages again.

[mark-i-m]

Thanks @nikomatsakis!

Some things I am not keen on:

• “unsatisfied lifetime constraints” as the header
  • probably this should be drawn instead from the “primary” error
• the phrase “fully elaborated type” – I like the idea, but it feels jargon-y. I can’t think of a better way to present such types though.
  • I do wonder if the --explain can explain “how to read” these errors, and if that might help a bit?

I personally don’t think “unsatisfied lifetime constraints” or “fully elaborated type” are very problematic. People will learn the jargon. We could try to soften it, but IMHO that’s where the effort should be spent.

What would stump me is things like defaults to: `Box<dyn Iterator<Item = &T> + 'static>`. It would take me a while to figure out where the default of 'static comes from and why. In this case, it’s easy enough, but in general, I have always had trouble figuring out where some of the defaults/inferred values come from.

[nikomatsakis]

It would take me a while to figure out where the default of 'static comes from and why.

So, what could we do to improve that, you think? (Leaving aside the question fo changing the language that @ekuber raised – I forgot to reply to that, but I think I’d prefer to focus here on just error messages; in any case, that default was not arrived at lightly, and I am reluctant to change it)

In this case, the reason for that is … kind of just … “those are the rules”. It seems like it’d be nice if we could give a link to a more detailed explanation – this is where the --teach mode would be awesome. But we don’t have that.

This is one reason I was moving more towards a “separate note” vs an inline note. In particular, it would give us room to say a bit more, such as:

dyn Trait types have a default lifetime bound of 'static

(That’s not quite right, the real rule is that their lifetime bound is 'static unless they appear inside a reference, so I have to think how to word it more precisely.)

Perhaps:

---

= note: the `dyn Iterable` type has a default bound of `'static`
    in the type `Box<dyn Iterable<Item = &T>>`, 
    so the return type here is equivalent to:
 
 | Box<dyn Iterable<Item = &T> + 'static>

or something…?

[davidtwco]

One idea I’ve had, and I’m not sure if it’s something we can do (or should do), would be to insert '1 into the argument in the span and highlight it with a colour to emphasise that it is an addition for the purpose of the error message?

That is, instead of:

error: unsatisfied lifetime constraints
  --> underscore-lifetime/dyn-trait-underscore.rs:18:5
   |
16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |         -----           -------------------------- defaults to: `Box<dyn Iterator<Item = &T> + 'static>`
   |         |
   |         the fully elaborated type of `items` is `&'1 [T]`
17 |     Box::new(items.iter())
   |     ^^^^^^^^^^^^^^^^^^^^^^ casting this value to `Box<dyn Iterator<Item = &T>>` requires `'1` to be `'static`

We would show (but the '1 is in some colour to indicate that it is new):

error: unsatisfied lifetime constraints
  --> underscore-lifetime/dyn-trait-underscore.rs:18:5
   |
16 | fn a<T>(items: &'1 [T]) -> Box<dyn Iterator<Item=&T>> {
   |                            -------------------------- defaults to: `Box<dyn Iterator<Item = &T> + 'static>`
17 |     Box::new(items.iter())
   |     ^^^^^^^^^^^^^^^^^^^^^^ casting this value to `Box<dyn Iterator<Item = &T>>` requires `'1` to be `'static`

I’m pretty certain that this is an awful idea, but you never know, it might inspire something in someone reading.

---

The first one is more generic, the second more specialized:

I think the specialized example shown in this message is perhaps my favourite iteration on the error message seen thus far, perhaps the only downside I see is that mentioned by @mark-i-m whereby defaults to: `Box<dyn Iterator<Item = &T> + 'static> isn’t immediately obvious where the 'static comes from - I’m not sure what the best way to explain this in the error is though. I also agree that terminology such as “fully elaborated type” isn’t too confusing or unclear.

[nikomatsakis]

I’m pretty certain that this is an awful idea

Interesting idea. regardless, I think doing what I suggested is probably a good first step – that is, we have to write the code to identify the place where the lifetime would go.

It’s also worth pointing out that this won’t apply in all cases, particularly around closures. For example, there is no explicit type on x here:

|x| foo(x)

so if we want to show the elaborated type of x: &'1 u32, we can’t do it by highlighting the existing code etc. So we probably need the “fully elaborated type” sort of phrasing as a fallback anyway. I like the “let’s call” terminology though – you could imagine

3 |     something(|x| foo(x))
  |                ^ let's call the type here `TyCtxt<'1, '_, '_>`

Another such example would be upvars; this closure may need to annotate x:

let x = ...;
foo(|| use(x))

[mark-i-m]

Yes, that is a significant improvement. In particular, it makes it clear that the error is due to a bug in the program, rather than forgetting to specifying a lifetime or something. I would be perfectly fine with seeing “'cause that’s the rules in an error message” as opposed to “because type Foo specifies lifetime 'static”.

[Eh2406]

In the tradition of bad ideas…

So we probably need the “fully elaborated type” sort of phrasing as a fallback anyway.

If we are uneliding things from the code anyway then we can just do:

3 |     something(|x: TyCtxt<'1, '_, '_>| foo(x))

and similarly for upvars

let x: TyCtxt<'1, '_, '_> = ...;
foo(|| use(x))

[nikomatsakis]

OK, so let me try to break this down to some immediate next steps. These are basically things to do to improve my existing PR #52021:

• code to find and highlight the precise part of the type where a region appears
  • i.e., to say something like "let’s call the lifetime of this reference '1"
  • seems like no matter what we’re going to want this
• code to identify when we are casting to a trait object with a default bound

hmm, ok, that was not as good a list as I hoped. have to come back to this. posting this message anyway to get it out of my “queue”

[lambda]

error: unsatisfied lifetime constraints
  --> underscore-lifetime/dyn-trait-underscore.rs:18:5
   |
16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |         -----           -------------------------- defaults to: `Box<dyn Iterator<Item = &T> + 'static>`
   |         |
   |         the fully elaborated type of `items` is `&'1 [T]`
17 |     Box::new(items.iter())
   |     ^^^^^^^^^^^^^^^^^^^^^^ casting this value to `Box<dyn Iterator<Item = &T>>` requires `'1` to be `'static`

Thoughts on the initial suggestion:

1. A good start, less extra verbiage for eyes to glaze over
2. Too wide. Have to scroll horizontally to see the messages. Wider than my usual terminals and editor windows, so would wrap badly
3. “fully elaborated type” is too jargony, and also just too verbose
4. While this is a lot less for the eyes to glaze over, it’s still quite a “garden path” in which I need to look over several places and combine information to figure out what’s going on

After looking at some of the later suggestions, I think that the jargony nature may not actually be the issue, but just the length. Any amount of prose moves the elaborated type fairly far away from what it’s labeling making it harder to read.

3 |     something(|x| foo(x))
  |                ^ let's call the type here `TyCtxt<'1, '_, '_>`

I think you might be able to get away with something much shorter, like:

3 |     something(|x| foo(x))
  |                ^ called `TyCtxt<'1, '_, '_>`

Or applied to the original example:

error: unsatisfied lifetime constraints
  --> underscore-lifetime/dyn-trait-underscore.rs:18:5
   |
16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |                ----     -------------------------- 
   |                |        defaults to `Box<dyn Iterator<Item = &T> + 'static>`
   |                called `&'1 [T]`
   |
17 |     Box::new(items.iter())
   |     ^^^^^^^^^^^^^^^^^^^^^^
   |     return type `Box<dyn Iterator<Item = &T> + 'static>` requires `'1` to be `'static`

I’m not entirely satisfied with that result, but I do find I can see a lot more of the relevant information quickly, and closer to the relevant code, than I could with all of the extra prose.

[Tom-Phinney]

Rather than

^ called TyCtxt<'1, '_, '_>

how about

^ here called TyCtxt<'1, '_, '_>

To me that makes clear both the referent and the explanatory substitution.

[illustrious-you]

“fully elaborated type” is too jargony, and also just too verbose

A few ideas: exact, definite, or complete type.

error: unsatisfied lifetime constraints
  --> underscore-lifetime/dyn-trait-underscore.rs:18:5
   |
16 | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
   |         -----           -------------------------- defaults to: `Box<dyn Iterator<Item = &T> + 'static>`
   |         |
   |         the exact type of `items` is `&'1 [T]`
17 |     Box::new(items.iter())
   |     ^^^^^^^^^^^^^^^^^^^^^^ casting this value to `Box<dyn Iterator<Item = &T>>` requires `'1` to be `'static`