Confusing "type parameter" in error message.

@dkadashev

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.

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.

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.

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.

Anyone working on this?

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!

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

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

@gaurikholkar

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

@kevincox

I've strated working on it @kevincox

Made the following changes to my locally and WIP.

@estebank

I guess we go with the DefId then @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?

@estebank

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 #47319 (comment) still stand or is there a better way now, given the amount of time passed?

@dkadashev

@dkadashev I believe that is indeed the case.

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

@rustbot

@rustbot claim

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)`

Rollup merge of rust-lang#66014 - dkadashev:47319-type-param-def-loca…

Navigation Menu

Repository navigation

Confusing "type parameter" in error message. #47319

1 remaining item

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Confusing "type parameter" in error message. #47319

Description

Activity

Mark-Simulacrum commented on Jan 10, 2018

estebank commented on Jan 10, 2018

kevincox commented on Jan 11, 2018

gaurikholkar-zz commented on Jan 11, 2018

estebank commented on Jan 11, 2018

jdahlstrom commented on Jan 17, 2018

kevincox commented on Jan 17, 2018

estebank commented on Jan 17, 2018

gaurikholkar-zz commented on Jan 18, 2018

gaurikholkar-zz commented on Jan 21, 2018

gaurikholkar-zz commented on Jan 28, 2018

estebank commented on Jan 28, 2018

1 remaining item

dkadashev commented on Oct 20, 2019

estebank commented on Oct 24, 2019

dkadashev commented on Oct 31, 2019

dkadashev commented on Oct 31, 2019

dkadashev commented on Oct 31, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions