Request for feedback: `impl dyn Trait {}` implicit `'static` diagnostic

[ekuber]

Hi! I have a PR up to change this

into this

What changes would you make to the new output? Is it clear to you what and why is happening and why the fix is appropriate?

[djc]

More context here: https://github.com/rust-lang/rust/issues/71341.

With the new error, I would say it's not clear to me what and why is happening -- at most I can vaguely intuit that the fix is appropriate. It's still an improvement because it's much more specific, though!

Is there more extensive documentation for E0759 that you can link here?

[CAD97]

The issue, boiled down:

• You have &'a dyn Trait1
• There's an impl Trait2 for dyn Trait1
• You're trying to call Trait2::method with the &'a dyn Trait1
• The impl isn't applicable because it's on 'static trait objects only without a lifetime annotation on that impl

I think it's potentially misleading to say the 'static bound is introduced at the method; from the definition side it's the impl line that has the 'static requirement. It is calling the method on the &'a dyn Trait1 that's causing the problem (as 'a does not satisfy 'static), but glancing at the new details here made me think that the method declaration was somehow at fault, before I read the labels better.

[kornel]

To me the new version is clearly better. Showing implicit requirement is great.

I generally dislike when error messages mention "must live for 'static lifetime", because in my experience users interpret this as "change all 'a to 'static", which makes things even worse. But here there's a suggestion to add + '_, which is nice.

[ekuber]

@djc

Is there more extensive documentation for E0759 that you can link here?

I'll introduce a new error code for this, I think it makes sense to do so as E0759 is currently only for implicit static bounds in return position trait objects and impl trait.

[ekuber]

@CAD97

but glancing at the new details here made me think that the method declaration was somehow at fault, before I read the labels better.

Great feedback! What did you glance at? I'm hesitant at changing the red span, as it is the source of the problem, but maybe I can change that labels wording...

[ekuber]

@kornel

I generally dislike when error messages mention "must live for 'static lifetime", because in my experience users interpret this as "change all 'a to 'static ", which makes things even worse. But here there's a suggestion to add + '_ , which is nice.

I agree 100% and am slowly changing all instances of 'static suggestions coming from implicit bounds. I'm aiming at only showing 'static as a suggestion when it is 99% likely it is the only thing you can do.

[ekuber]

What does the following look like (increasing redundancy in an effort to make it clearer)?

error[E0767]: `val` has lifetime `'a` but calling `use_self` introduces an implicit `'static` lifetime requirement
  --> src/test/ui/suggestions/impl-on-dyn-trait-with-implicit-static-bound.rs:37:13
   |
36 |     fn use_it<'a>(val: &'a dyn ObjectTrait) -> &'a () {
   |                        ------------------- this data with lifetime `'a`...
37 |         val.use_self()
   |             ^^^^^^^^ ...is captured and required to live as long as `'static` here because of an implicit lifetime bound on the trait object's `impl` of `bar::MyTrait`
   |
note: calling `bar::ObjectTrait`'s `impl` of trait `bar::MyTrait`'s method `use_self` introduces an implicit `'static` requirement
  --> src/test/ui/suggestions/impl-on-dyn-trait-with-implicit-static-bound.rs:31:26
   |
31 |     impl MyTrait for dyn ObjectTrait {
   |                          ^^^^^^^^^^^ this trait object has an implicit `'static` lifetime requirement
32 |         fn use_self(&self) -> &() {
   |            -------- `'static` requirement is introduced when calling this method
help: consider relaxing the implicit `'static` requirement
   |
31 |     impl MyTrait for dyn ObjectTrait + '_ {
   |                                      ^^^^

---

Another iteration:

[Tom-Phinney]

I find the above error messages/explanations to be a lot less confusing. Definite improvement !!

[CAD97]

@ekuber

What did you glance at?

It's the fourth span that initially suggested something about the method definition was part of the problem. Perhaps rewording the third and fourth span notes to also use a ... continuation (to make it clear its one idea), something like

this trait object has an implicit `'static` lifetime requirement...
...which is required in order to call this method

It's not the red span which feels wrong (it's very right!), it's that fourth span which felt wrong before digesting exactly what the message said.

[ekuber]

What about changing the wording? (I'm trying to avoid relying on source order for the text, although that might be reasonable for this case)

note: `bar::ObjectTrait`'s `impl` of `bar::MyTrait` has an implicit `'static` requirement
  --> $DIR/impl-on-dyn-trait-with-implicit-static-bound.rs:31:26
   |
LL |     impl MyTrait for dyn ObjectTrait {
   |                          ^^^^^^^^^^^ this has an implicit `'static` lifetime requirement
LL |         fn use_self(&self) -> &() { panic!() }
   |            -------- calling this method introduces the `impl` type's `'static` requirement

[CAD97]

That's definitely better, at least. I think what it was is that I read "^------ 'static introduced" and autocompleted "introduced here", which isn't quite correct. Putting "calling this method" first eliminates that misleading shortcut.

(Source order in this case is guaranteed, since it's impl header => method in the impl block, so relying on it would be "less bad" than between two logically unordered elements.)

[ekuber]

The only case where the order is not assured is for methods with default implementations:

trait Foo {
    fn foo(&self) {}
}
trait Bar {}
impl Foo for dyn Bar {}

error[E0767]: `val` has lifetime `'a` but calling `foo` introduces an implicit `'static` lifetime requirement
 --> file6.rs:9:9
  |
8 | fn baz<'a>(val: &'a dyn Bar) -> &'a () {
  |                 ----------- this data with lifetime `'a`...
9 |     val.foo()
  |         ^^^ ...is captured and required to live as long as `'static` here because of an implicit lifetime on the `impl` of `Foo`
  |
note: `Bar`'s `impl` of `Foo` has an implicit `'static` requirement
 --> file6.rs:7:18
  |
2 |     fn foo(&self) -> &() {
  |        --- calling this method introduces the `impl` `'static` requirement
...
7 | impl Foo for dyn Bar {}
  |                  ^^^ this has an implicit `'static` lifetime requirement
help: consider relaxing the implicit `'static` requirement
  |
7 | impl Foo for dyn Bar + '_ {}
  |                      ^^^^

but I could handle that case specifically....

[djc]

I still find the error confusing because per @CAD97's initial post, I interpret the use_self() method as only being valid for &'static dyn Trait values, rather than the use_self() call then introducing an &'a lifetime in the return value. I see some analogy here with methods from impl blocks guarded by unsatisfied where clauses, but maybe that analogy doesn't fully work with these lifetime issues?

[ekuber]

@djc

I interpret the use_self() method as only being valid for &'static dyn Trait values, rather than the use_self() call then introducing an &'a lifetime in the return value.

What you're saying makes sense, but note that this is about dyn Trait + 'static, not &'static Trait (this last one implies &'static Trait + 'static). That being said, I'm unsure how we can modify the diagnostic to carry the nuance that the method is available, but only if dyn Trait + 'static, in the same way we do for methods available if Self: Sized.

[ekuber]

Just to keep everybody posted, this is what my final iteration looks like: