Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

E0094 needs better underline #35966

Closed
sophiajt opened this issue Aug 24, 2016 · 7 comments
Closed

E0094 needs better underline #35966

sophiajt opened this issue Aug 24, 2016 · 7 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@sophiajt
Copy link
Contributor

Currently, E0094 looks like:

error[E0094]: intrinsic has wrong number of type parameters: found 2, expected 1
  --> src/test/compile-fail/E0094.rs:13:5
   |
13 |     fn size_of<T, U>() -> usize; //~ ERROR E0094
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 1 type parameter

A better span would underline the type parameters themselves.

error[E0094]: intrinsic has wrong number of type parameters: found 2, expected 1
  --> src/test/compile-fail/E0094.rs:13:5
   |
13 |     fn size_of<T, U>() -> usize; //~ ERROR E0094
   |                ^^^^ expected 1 type parameter
@bstrie bstrie added the A-diagnostics Area: Messages for errors, warnings, and lints label Aug 24, 2016
@KiChjang
Copy link
Member

So we're looking at an Item of the ItemFn variant. More specifically, it's the Generics struct that resides in an ItemFn. This should be pretty straight-forward since Generics contains a span method which we can call to get us the span.

@kyrias
Copy link
Contributor

kyrias commented Aug 24, 2016

Tried that, but getting

error[E0094]: intrinsic has wrong number of type parameters: found 2, expected 1
  --> src/test/compile-fail/E0094.rs:13:5
   |
13 |     fn size_of<T, U>() -> usize; //~ ERROR E0094
   |     ^^^^^^^^^^------^^^^^^^^^^^^
   |               |
   |               expected 1 type parameter

instead, which is closer, but still not quite. Going to look more at it tomorrow though.

@sophiajt
Copy link
Contributor Author

@kyrias - almost there, just need to also update the span at the start of the error message as well. When both are updated, it will will turn the new underline into the primary span (the ^^^ underline)

@kyrias
Copy link
Contributor

kyrias commented Aug 24, 2016

Ah, didn't think about that span! (And I just realized it's almost 01:00.)

To get there I have to match on rustc::hir::ForeignItem_ which can be either ForeignItemFn or ForeignItemStatic though, and if I add the original error to the second match branch I of course get a warning that E0094 has already been defined, so I'm not sure how to solve that correctly. Can it.node be ForeignItemStatic at that point? Haven't had time to look at it right now, have to sleep so I don't end up too late tomorrow, but should have time during the day tomorrow.

Edit: I could of course set a variable with either the span from the generics or the it.span, but I don't think that would be appropriate, since they end up printing different error messages for the same error code.

@KiChjang
Copy link
Member

KiChjang commented Aug 24, 2016

You don't need to overthink this:

let span = match it.node {
    ForeignItem_::ForeignItemFn(_, ref generics) => generics.span().unwrap_or(it.span),
    _ => it.span
};

And then use span as the first argument to span_label.

@KiChjang
Copy link
Member

KiChjang commented Aug 24, 2016

Essentially, it.span here is being treated as a fallback, and I think that's fine. They do not "print different error messages" as you put it, only that the spans would be different, and it's no big deal.

@kyrias
Copy link
Contributor

kyrias commented Aug 25, 2016

Yeah, I'm not sure what I was thinking. That serves me right for doing things while going to bed is 3 hours overdue. Currently compiling, will submit a PR in a while when all tests are done.

@brson brson added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Aug 26, 2016
@bors bors closed this as completed in 6f93d3c Aug 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

5 participants