Rust: Confusing "type parameter" in error message.

[kevincox]

I have thought this is odd for a while, however after teaching a Rust introduction class I found numerous attendees stumbling on this weird error for a simple and common case.

error[E0308]: mismatched types
   --> src/codelab.rs:100:5
    |
100 |   tail.val
    |   ^^^^^^^^ expected enum `std::option::Option`, found type parameter
    |
    = note: expected type `std::option::Option<T>`
    = note:    found type `T`

The "found type parameter" part is confusing, as I (and most students) interpret this as some sort of weird syntax-like error. For example it feels similar to the "expected {, found ;" you get from syntax errors.

I think that simply adding `T` after the message would make this much more understandable.

error[E0308]: mismatched types
   --> src/codelab.rs:100:5
    |
100 |   tail.val
    |   ^^^^^^^^ expected enum `std::option::Option`, found type parameter `T`
    |
    = note: expected type `std::option::Option<T>`
    = note:    found type `T`

Possibly more controversial is dropping "parameter" or "type parameter" I don't think they add much value to the error message.

This issue has been assigned to @dkadashev via this comment.

[Mark-Simulacrum]

Well, I think rather we can say something like "generic T" or similar -- we probably don't want just "T" since there could be a type with that name in scope as well.

Likely a rather simple fix, but I wasn't able to quickly find where this diagnostic gets emitted.

[estebank]

I would go further and say that the full output should be:

error[E0308]: mismatched types
   --> src/codelab.rs:100:5
    |
xx  |  fn foo<T>(...) => T {
    |         - this type parameter
100 |   tail.val
    |   ^^^^^^^^ expected enum `std::option::Option`, found type parameter `T`
    |
    = note: expected type `std::option::Option<T>`
    = note:    found type `T`

I believe you could modify note_and_explain_type_err in order to check if the error is of TypeError::Sorts(ty.sty: TypeVariants::Param(param), _) | TypeError::Sorts(_, ty.sty: TypeVariants::Param(param)) (but not TypeError::Sorts(ty.sty: TypeVariants::Param(param), ty.sty: TypeVariants::Param(param)). The hard part is to either look at the enclosing scope (meaning passing some DefId into the mentioned method) or expanding ParamTy to hold its Span in order to be able to show it in errors (I think there are other cases where it'd be needed to have it, so I'd be inclined to go down that route).

As for the main span's label, these are the lines where the "expected {}, found {}" is being produced. You could either enclose it with a check for an expected/found type parameter, or (better yet) modify sort_string to include the type argument's name.

[kevincox]

I would go further and say that the full output should be: ... ^ this type parameter

This seems a little unnecessary to me as the names are often obvious enough. But I'm not opposed to this.

[gaurikholkar]

Anyone working on this?

[estebank]

This seems a little unnecessary to me as the names are often obvious enough.

Usually (except in the case of nested impl/fn blocks), but adding that makes it so that if you're using rustc directly, you have all the necessary context available before you go back to your editor, and if you're using an editor that supports the RLS, the editor will highlight the proper span. Also consider that error messages have to be understandable to both people very acquainted with the language as well as to people who have just started programming. When in doubt, I lean towards the side of over explaining.

Furthermore, consider the current snippet: we don't know wether this was in an impl, a struct or an fn. Having that information available would make it easier to give more targeted explanations on stackoverflow, for example.

Anyone working on this?

Feel free to take it on!

[jdahlstrom]

Another pretty confusing instance of this in #47499, only with two type parameters.

[kevincox]

Has anyone taken ownership? I'd be willing to start a patch this weekend if no one is working on it.

[estebank]

I believe @gaurikholkar wanted to take it up, but I don't know if she's already started.

[gaurikholkar]

I've strated working on it @kevincox

[gaurikholkar]

Made the following changes to my locally and WIP.

[gaurikholkar]

I guess we go with the DefId then @estebank ?

[estebank]

As things stand, I believe that will be the best bet. Could you try and see how hard it would be to add a HashMap<DefId, Span> for param ids to, I'm guessing tcx?

[steveklabnik]

Triage: @gaurikholkar , did you ever finish this work?

[dkadashev]

Hi @estebank, if no one is working on this (which seems to be the case) then I'd like to pick this up (as a first contribution to the compiler) if that's OK.

I think it makes sense to make two separate PRs, the first one for the "modify sort_string to include the type argument's name" part and the second for the span part, since these changes are fairly unrelated to each other, and the former one is trivial and seems to be helpful on its own. Or it can be just separate commits in a single PR if that is preferable. Please let me know what's the best way here.

Also does the recommendation from https://github.com/rust-lang/rust/issues/47319#issuecomment-361092821 still stand or is there a better way now, given the amount of time passed?

[estebank]

@dkadashev I believe that is indeed the case.

[dkadashev]

Status update: after playing with this for a while and hitting pretty much the same problems described in https://github.com/rust-lang/rust/pull/47574 I think I finally have a fix (nice and small). Preparing a PR now (will take some time though - clean builds, etc.)

[dkadashev]

@rustbot claim

[dkadashev]

Some examples.

Two parameters case that was covered by #47499:

3 | fn foo<T, U>(t: T, u: U) {
  |        -  - found type parameter
  |        |
  |        expected type parameter
4 |     bar(t, u);
  |            ^ expected type parameter `T`, found type parameter `U`

One parameter case from the top of this ticket:

9  | fn foo1<T>(t: T) {
   |         - this type parameter
10 |     bar1(t);
   |          ^
   |          |
   |          expected enum `std::option::Option`, found type parameter `T`
   |          help: try using a variant of the expected enum: `Some(t)`

Same for impl - including type params on different levels:

17 | impl<T> S<T> {
   |      - expected type parameter
...
22 |     fn foo3<U>(self, u: U) {
   |             - found type parameter
23 |         bar(self.t, u);
   |                     ^ expected type parameter `T`, found type parameter `U`

17 | impl<T> S<T> {
   |      - this type parameter
18 |     fn foo2(self) {
19 |         bar1(self.t);
   |              ^^^^^^
   |              |
   |              expected enum `std::option::Option`, found type parameter `T`
   |              help: try using a variant of the expected enum: `Some(self.t)`