Format self correctly

b52eda0
Opened by Gauri Kholkar at 2023-01-08 18:42:53

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

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

    Niko Matsakis at 2017-06-16 18:03:46

  2. 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 https://github.com/rust-lang/rust/issues/42703.

    Niko Matsakis at 2017-07-17 13:13:20

  3. 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
    

    Niko Matsakis at 2017-07-23 14:16:28

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

    Niko Matsakis at 2017-07-23 14:17:43

  5. Hello @nikomatsakis, hello @gaurikholkar

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

    Cengiz Can at 2017-09-13 21:04:28

  6. @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.

    Gauri Kholkar at 2017-09-14 03:52:02

  7. 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
    

    Esteban Kuber at 2018-01-05 06:49:05

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

    Niko Matsakis at 2018-01-05 22:06:33

  9. 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.

    Esteban Kuber at 2018-08-29 23:29:50

  10. Triage: no change.

    Esteban Kuber at 2023-01-08 18:42:53