Format self correctly

[gaurikholkar-zz]

Fix the formatting of self in the error message.

struct Foo {
  field: i32
}

impl Foo {

fn foo<'a>(&self, x: &'a Foo) -> &'a Foo {

    if true { self } else { x }
    
    // now gives:
// error[E0611]: explicit lifetime required in the type of `self`
//    |
// 20 |     fn foo<'a>(&self, x: &'a Foo) -> &'a Foo {
//    |                  ^^^^^ consider changing the type of `self` to `&'a Foo`
// ...
// 25 |         if true { self } else { x }
//    |                   ---- lifetime `'a` required

    // should actually:
// error[E0611]: explicit lifetime required in the type of `self`
//    |
// 20 |     fn foo<'a>(&self, x: &'a Foo) -> &'a Foo {
//    |                  ^^^^^ consider changing to `&'a self`
// ...
// 25 |         if true { self } else { x }
//    |                   ---- lifetime `'a` required
  }
  
}

^^^^^ consider changing the type of `self` to `&'a Foo`

should actually be

^^^^^ consider changing to `&'a self`

cc @nikomatsakis

[nikomatsakis]

Once @gaurikholkar's PR lands, I'll try to write up some mentoring instructions for this.

[nikomatsakis]

So actually, this test:

struct Foo {
  field: i32
}

impl Foo {
  fn foo<'a>(&self, x: &'a Foo) -> &'a Foo {
    if true { self } else { x }
  }
}

fn main() { }

presently generates an "old-style" inference error:

error[E0312]: lifetime of reference outlives lifetime of borrowed content...

This is because we ruled out self arguments here. I think we would want to accept the case where the parameter whose type needs to be changed is self, first, but we have to be careful about cases where the anonymous region appears in both the self type and the return type -- we probably need to exclude that case, for the reasons discussed in #42703.

[nikomatsakis]

Here is an example where there is no potential confusion around the return type, but we still currently use the old-school errors:

struct Foo {
  field: i32
}

impl Foo {
  fn foo<'a>(&self, mut x: Vec<&'a Foo>) {
    x.push(self);
  }
}

fn main() { }

I think we should present this error as:

// error[E0611]: explicit lifetime required in the type of `self`
//    |
// 20 |     fn foo<'a>(&self, mut x: Vec<&'a Foo>) {
//    |                ^^^^^ consider changing to `&'a self`
// ...
// 25 |         x.push(self);
//    |                 ---- lifetime `'a` required

[nikomatsakis]

Main difference is the wording, we don't want to say "change the type of self to &'a Foo".

[cengiz-io]

Hello @nikomatsakis, hello @gaurikholkar

I've picked this up. Will ping you if needed 😛

[gaurikholkar-zz]

@nikomatsakis a small chunk of these messages are also triggering E0623 now as per the return type and self PR right? I'm guessing the adding of EBR and LBR has also reduced this set of messages.

[estebank]

Current output:

error[E0623]: lifetime mismatch
 --> src/main.rs:9:15
  |
7 | fn foo<'a>(&self, x: &'a Foo) -> &'a Foo {
  |            -----                 -------
  |            |
  |            this parameter and the return type are declared with different lifetimes...
8 | 
9 |     if true { self } else { x }
  |               ^^^^ ...but data from `self` is returned here

[nikomatsakis]

It may be that the current output is good enough. The bug was filed to give a suggestion like "try changing to &'a self.

[estebank]

Current output with NLL:

error: unsatisfied lifetime constraints
  --> src/lib.rs:10:15
   |
8  | fn foo<'a>(&self, x: &'a Foo) -> &'a Foo {
   |        --  - let's call the lifetime of this reference `'1`
   |        |
   |        lifetime `'a` defined here
9  | 
10 |     if true { self } else { x }
   |               ^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'1`

I believe we should add the suggestion back, proposing constraining the &self borrow to 'a, both with NLL enabled and disabled.

[estebank]

Triage: no change.