Regression in helpful compiler error message when working with lifetimes

[petergarnaes]

So I have the following code:

struct Article {
    proof_reader: ProofReader,
}

struct ProofReader {
    name: String,
}

pub trait HaveRelationship<To> {
    fn get_relation(&self) -> To;
}

impl HaveRelationship<&ProofReader> for Article {
    fn get_relation(&self) -> &ProofReader {
        &self.proof_reader
    }
}

Which on the stable compiler gives this helpful error message:

error[E0495]: cannot infer an appropriate lifetime for lifetime parameter in generic type due to conflicting requirements
  --> src/main.rs:14:5
   |
14 |     fn get_relation(&self) -> &ProofReader {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the method body at 14:5...
  --> src/main.rs:14:5
   |
14 | /     fn get_relation(&self) -> &ProofReader {
15 | |         &self.proof_reader
16 | |     }
   | |_____^
note: ...but the lifetime must also be valid for the lifetime '_ as defined on the impl at 13:23...
  --> src/main.rs:13:23
   |
13 | impl HaveRelationship<&ProofReader> for Article {
   |                       ^
   = note: ...so that the method type is compatible with trait:
           expected fn(&Article) -> &ProofReader
              found fn(&Article) -> &ProofReader

Yet on the nightly compiler the compiler error does not inform me about lifetimes, but tells me the signature needs to be what it already is:

error: `impl` item signature doesn't match `trait` item signature
  --> src/main.rs:14:5
   |
10 |     fn get_relation(&self) -> To;
   |     ----------------------------- expected fn(&Article) -> &ProofReader
...
14 |     fn get_relation(&self) -> &ProofReader {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found fn(&Article) -> &ProofReader
   |
   = note: expected `fn(&Article) -> &ProofReader`
              found `fn(&Article) -> &ProofReader`

playground stable version
playground nightly version

[jonas-schievink]

Happens on beta too

[estebank]

For what is worth, the correct code would be closer to:

pub trait HaveRelationship<'a, To: 'a> {
    fn get_relation(&'a self) -> To;
}

impl<'a> HaveRelationship<'a, &'a ProofReader> for Article {
    fn get_relation(&'a self) -> &'a ProofReader {
        &self.proof_reader
    }
}

The error is not incorrect, as there is a discrepancy between the trait and the impl, but it should provide more information in this case, particularly to show that the lifetimes in expected and found are not evaluated the same.

[estebank]

This is caused by #65068. I believe this is now a duplicate of #41343.

CC @nikomatsakis do you have any ideas of what the appropriate output should be for these cases?

[pnkfelix]

triage: P-medium, removing nomination

[nikomatsakis]

I don't think it's necessarily possible to tell what the correct code is here, but I would guess that

pub trait HaveRelationship<To> {
    fn get_relation(&self) -> &To;
}

impl HaveRelationship<ProofReader> for Article {
    fn get_relation(&self) -> &ProofReader {
        &self.proof_reader
    }
}

is closer to the mark. i.e., move the references into the trait signature.

[nikomatsakis]

Another good version:

struct Article {
    proof_reader: ProofReader,
}

struct ProofReader {
    name: String,
}

pub trait HaveRelationship<To> {
    fn get_relation(self) -> To;
}

impl<'a> HaveRelationship<&'a ProofReader> for &'a Article {
    fn get_relation(self) -> &'a ProofReader {
        &self.proof_reader
    }
}

fn main() {
    println!("Hello, world!");
}

[nikomatsakis]

In both cases, the principle is the same: move the & to the same place, whether that be the impl header, or the fn definition (in the trait).

[nikomatsakis]

This version of the code also works, but it seems more complex than both of the above versions and it doesn't allow anything in particular that they don't, as far as I know. It still doesn't require the To: 'a annotation, though, @estebank:

struct Article {
    proof_reader: ProofReader,
}

struct ProofReader {
    name: String,
}

pub trait HaveRelationship<'a, To> {
    fn get_relation(&'a self) -> To;
}

impl<'a> HaveRelationship<'a, &'a ProofReader> for Article {
    fn get_relation(&'a self) -> &'a ProofReader {
        &self.proof_reader
    }
}

fn main() {
    println!("Hello, world!");
}

[estebank]

I feel that the first of the offered suggestions makes the most sense for the general case, changing the signature consume Self feels more drastic.

[estebank]

I would rather not provide a structured suggestion for this case. What prose would you recommend here @nikomatsakis that would be suitable for a label on To's definition or as a note?

[estebank]

What do you think about this output?

error: `impl` item signature doesn't match `trait` item signature
  --> file5.rs:37:5
   |
31 | pub trait Bar<To, Fr> {
   |               --  -- this type parameter might not have a lifetime compatible with the `impl`
   |               |
   |               this type parameter might not have a lifetime compatible with the `impl`
32 |     fn get_relation(&self) -> (To, Fr);
   |     -----------------------------------
   |     |                          |   |
   |     |                          |   you might want to borrow this type parameter in the trait to make it match the `impl`
   |     |                          you might want to borrow this type parameter in the trait to make it match the `impl`
   |     expected `fn(&Article) -> (&ProofReader, &usize)`
...
37 |     fn get_relation(&self) -> (&ProofReader, &usize) {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&Article) -> (&ProofReader, &usize)`
   |
   = note: expected `fn(&Article) -> (&ProofReader, &usize)`
              found `fn(&Article) -> (&ProofReader, &usize)`
   = note: the lifetime requirements from the `trait` could not be satisfied by the `impl`
   = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output

I spent some time trying to actually verify if a type param has been replaced with a borrow, but it seems to be more involved than I think is worth for this case.

[nikomatsakis]

@estebank sorry for lack of replies, i've enqueued the PR back into my task queue...my first thought when reading the comments above are that it seems pretty complex, but I don't have any constructive ideas how to improve yet :)

[estebank]

@nikomatsakis fair. Keep in mind that the case above is exercising a more complex case than the more likely one:

error: `impl` item signature doesn't match `trait` item signature
  --> file5.rs:37:5
   |
31 | pub trait Bar<To> {
   |               -- this type parameter might not have a lifetime compatible with the `impl`
32 |     fn get_relation(&self) -> To;
   |     -----------------------------
   |     |                         |
   |     |                         you might want to borrow this type parameter in the trait to make it match the `impl`
   |     expected `fn(&Article) -> &ProofReader`
...
37 |     fn get_relation(&self) -> &ProofReader {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&Article) -> &ProofReader`
   |
   = note: expected `fn(&Article) -> &ProofReader`
              found `fn(&Article) -> &ProofReader`
   = note: the lifetime requirements from the `trait` could not be satisfied by the `impl`
   = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output

It is also difficult because anything I remove I feel like I'm hiding useful information.

[nikomatsakis]

@estebank I think this comment in particular

you might want to borrow this type parameter in the trait to make it match the impl

might be confusing to folks. Can we maybe just say "you might want to change this return type to make it match the impl"?