Add long error explanation for E0271

[GuillaumeGomez]

Part of #24407.

[rust-highfive]

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

[GuillaumeGomez]

r? @pnkfelix

[pnkfelix]

don't you also need to remove the other entry for E0271 from the register_diagnostics! call below?

(I imagine this is why the Travis build failed.)

You can check this sort of thing quickly + locally by running make tidy on your end.

[GuillaumeGomez]

Erf, it's because I wasn't on the good branch and copy/paste my code. I just forgot to remove the entry in the register_diagnostics. I fix it.

[GuillaumeGomez]

@pnkfelix: I removed the entry. If you want me to squash, I'll do it this evening. Beside, have you see something that I'd need to fix ?

[pnkfelix]

@GuillaumeGomez so, looking at this again, it seems like you have a very specific help message attached to E0271, which is signaled from report_projection_error

So, how did you decide on this help text?

In particular, I would have guessed that while this message does arise when one writes code like the example you gave (transcribed here):

fn main() {
    let vs = vec![1, 2, 3, 4];

    for v in &vs {
        match v {
            1 => {}
            _ => {}
        }
    }
}

it can also arise in scenarios that have nothing to do with match. That may make this message actively misleading rather than helpful.

Here is another example where this same error code (E0271) can arise, and your help would be very confusing:

fn main() {
    trait Trait { type AssociatedType; }
    impl Trait for i8 { type AssociatedType = char; }
    fn foo<T>(t: T) where T: Trait<AssociatedType=u32> {
        println!("in foo");
    }

    foo(3i8);
}

playpen

[pnkfelix]

In case it is not clear: In your example, this error message bubbles up from constraints that arise from the mix of the way that a constraint on the type for v bubbles up out of its use in match, versus how the type of v is passed downward from the fact that it is the type of the associated type Item of the Iterator trait in our for-loop + iterator protocol.

But in general, this error message has nothing to do with match, for-loops, nor iterators. It is solely about a type mismatch, that happens to involve (somewhere along the line) a constraint that forces the associated type of some particular trait to be equivalent to some other type.

In your example, the constraint is that slice::Iter::Item must be equal to (some integral type), but the Item for any slice::Iter is always a reference, never an integral type. (Clearly a separate bug here is that we should not be printing "(some integral type)" as _ in our error messages, as the latter is far too general and mysterious.)

In my follow-up example, the constraint is that i8::AssociatedType must be equal to u32, and i8::AssociatedType is defined to be char (and thus the error reports "expected char, found u32").

[GuillaumeGomez]

Oh okay ! My example was the only case I succeed to have the E0271 error. Thanks to your explanations, I'll be able to add a lot more precise error description. So thank you !

[GuillaumeGomez]

@pnkfelix: I changed it. I used you example and some of your explanations to do so. What do you think of it ?

[pnkfelix]

I don't know, something about this error and this message just does not sit quite right with me. Sorry I do not have more constructive feedback at the moment

[GuillaumeGomez]

I have this feeling too but it's hard to have a good sight of our own work. Then let's wait for others' opinion.

[michaelsproul]

This looks like a tricky error to explain, I'll help you dig into it more tomorrow 😄

[GuillaumeGomez]

Yes, kind of. Thanks a lot !

[pnkfelix]

Okay, after reflecting a long time on this, I think I know what has been bothering me and how I would fix it.

The problem with your description

In your description of the error, you say "a type mismatch that happens to involve a constraint that forces the associated type of some particular trait to be equivalent to some other type" -- I find this somewhat vague. That is, it is technically correct, but as a novice reader (and perhaps even as an expert), I would not be confident in my understanding of what features of the language are involved that are causing this error to arise.

That sort of vagueness would be okay, if the first example I saw was as simple as possible and very clearly indicated how the terms above map to the example. But in your text, the very first example is of how the problem can arise with a combination of a for-loop and a match -- it is incredibly hard to map that onto this text. Especially since you do not remind the reader what the definition for the Iterator trait is (or that the Iterator trait is even involved with for-loops), so it is not immediately clear that associated types are one of the key components in the text you have provided.

How I would fix it

1. First, I would elaborate slightly on what sort of constraint you are referring to in the first paragraph.
2. Then, I would reorder the examples, so that the self-contained one that I gave you
   comes first.
3. Finally, I would add comments to the for-loop example with a reminder that Iterator has an Item associated type, and that associated type is what is being constrained by the v in the match (since it is being inferred to have some integral type).

Suggested revised text

This is because of a type mismatch between the associated type of some
trait (e.g. T::Bar, where T implements trait Quux { type Bar; })
and another type U that is required to be equal to T::Bar, but is not.
Examples follow.

Here is a basic example:

trait Trait { type AssociatedType; }
fn foo<T>(t: T) where T: Trait<AssociatedType=u32> {
    println!("in foo");
} 
impl Trait for i8 { type AssociatedType = &'static str; }
foo(3_i8);

Here is that same example again, with some explanatory comments:

trait Trait { type AssociatedType; }

fn foo<T>(t: T) where T: Trait<AssociatedType=u32> {
//                    ~~~~~~~~ ~~~~~~~~~~~~~~~~~~
//                        |            |
//         This says `foo` can         |
//           only be used with         |
//              some type that         |
//         implements `Trait`.         |
//                                     |
//                             This says not only must
//                             `T` be an impl of `Trait`
//                             but also that the impl
//                             must assign the type `u32`
//                             to the associated type.
    println!("in foo");
}

impl Trait for i8 { type AssociatedType = &'static str; }
~~~~~~~~~~~~~~~~~   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
//      |                             |
// `i8` does have                     |
// implementation                     |
// of `Trait`...                      |
//                  ... but it is an implementation
//                  that assigns `&'static str` to
//                  the associated type.

foo(3_i8);
// Here, we invoke `foo` with an `i8`, which does not satisfy
// the constraint `<i8 as Trait>::AssociatedType=32`, and
// therefore the type-checker complains with this error code.

Here is a more subtle instance of the same problem, that can
arise with for-loops in Rust:

let vs: Vec<i32> = vec![1, 2, 3, 4];
for v in &vs {
    match v {
        1 => {}
        _ => {}
    }
}

The above fails because of an analogous type mismatch,
though may be harder to see. Again, here are some
explanatory comments for the same example:

{
    let vs = vec![1, 2, 3, 4];

    // `for`-loops use a protocol based on the `Iterator`
    // trait. Each item yielded in a `for` loop has the
    // type `Iterator::Item` -- that is, `Item` is the
    // associated type of the concrete iterator impl.
    for v in &vs { 
//      ~    ~~~
//      |     |
//      |    We borrow `vs`, iterating over a sequence of
//      |    *references* of type `&Elem` (where `Elem` is
//      |    vector's element type). Thus, the associated
//      |    type `Item` must be a reference `&`-type ...
//      |
//  ... and `v` has the type `Iter::Item`, as dictated by
//  the `for`-loop protocol ...

        match v {
            1 => {}
//          ~
//          |
// ... but *here*, `v` is forced to have some integral type;
// only types like `u8`,`i8`,`u16`,`i16`, et cetera can
// match the pattern `1` ...

            _ => {}
        }

// ... therefore, the compiler complains, because it sees
// an attempt to solve the equations
// `some integral-type` = type-of-`v`
//                      = `Item::Item`
//                      = `&Elem` (i.e. `some reference type`)
//
// which cannot possibly all be true.

    }
}

To avoid those issues, you have to make the types match correctly.
So we can fix the previous examples like this:

// Basic Example:
trait Trait { type AssociatedType; }
fn foo<T>(t: T) where T: Trait<AssociatedType = &'static str> {
    println!("in foo");
}
impl Trait for i8 { type AssociatedType = &'static str; }
foo(3_i8);

// For-Loop Example:
let vs = vec![1, 2, 3, 4];
for v in &vs {
    match v {
        &1 => {}
        _ => {}
    }
}

[pnkfelix]

(okay its taken me nearly an hour but I think I'm done updating the above example in place. Maybe I should have just taken over this PR... oh well, anyway, feel free to incorporate as much of the above as you'd like...)

[GuillaumeGomez]

Wo... That's very impressive. Your revised version is perfect. I'd feel bad to just take your work and commit it. So as you prefer : do you want to open a new PR and I close this one or do you want me to commit your code ?

[pnkfelix]

@GuillaumeGomez do whatever you like; I don't need authorship credit. I was more looking at this as an exercise, in terms of identifying why I didn't like the original message (which was hard) and then how to fix it (which was easier, once the first task was done)

[pnkfelix]

If you do incorporate it yourself, please do try to fix the typos. (There are at least two I can see offhand, like Item::Item)

[GuillaumeGomez]

Then I'll do as you say, but it'll be said in the commit that is it your work. I'm really not comfortable to not mention you and your work.

[GuillaumeGomez]

@pnkfelix: I updated your code, I only found two typos (just like you said actually) and it made me discover a dead link in documentations. :-)

[pnkfelix]

@GuillaumeGomez great; please do fix make tidy too

[pnkfelix]

(And squash as well...)

[GuillaumeGomez]

Squash done. @pnkfelix: Thanks a lot for your "help" (since you did all the job, I don't think help is the good word ^^).

[pnkfelix]

@bors r+ 30f88c8 rollup

[bors]

⌛ Testing commit 30f88c8 with merge dc630d0...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt