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

Old school errors not showing labels as notes #35330

Closed
sophiajt opened this issue Aug 4, 2016 · 5 comments
Closed

Old school errors not showing labels as notes #35330

sophiajt opened this issue Aug 4, 2016 · 5 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@sophiajt
Copy link
Contributor

sophiajt commented Aug 4, 2016

Originally, we had intended that old school errors would treat most labels as notes just so we would have similar information between the two formats. With the latest refactor, this seems to have been removed.

This may or may not be related: https://gist.github.com/trixnz/ad11e68687529e164427df8f8eb63116

error[E0507]: cannot move out of borrowed content
  --> src/test/compile-fail/borrowck/borrowck-move-out-of-vec-tail.rs:30:17
   |
30 |                 &[Foo { string: a },
   |                 ^               - hint: to prevent move, use `ref a` or `ref mut a`
   |                 |
   |                 cannot move out of borrowed content
...
34 |                   Foo { string: b }] => {
   |                                 - ...and here (use `ref b` or `ref mut b`)

error: aborting due to previous error

Same error after unset RUST_NEW_ERROR_FORMAT

jturner-23759:rust jturner$ rustc src/test/compile-fail/borrowck/borrowck-move-out-of-vec-tail.rs 
src/test/compile-fail/borrowck/borrowck-move-out-of-vec-tail.rs:30:17: 34:37 error: cannot move out of borrowed content [E0507]
src/test/compile-fail/borrowck/borrowck-move-out-of-vec-tail.rs:30                 &[Foo { string: a },
                                                                                   ^               ^
src/test/compile-fail/borrowck/borrowck-move-out-of-vec-tail.rs:30:17: 34:37 help: run `rustc --explain E0507` to see a detailed explanation
error: aborting due to previous error 
@nagisa
Copy link
Member

nagisa commented Aug 6, 2016

Maybe it would make sense to have a separate LABEL marker for the labels?

@sophiajt
Copy link
Contributor Author

sophiajt commented Aug 6, 2016

@nagisa - yeah, though we should chat with @nikomatsakis. He's been leaning towards more tests being UI tests that check for these things, so you can check layout at the same time.

@nikomatsakis
Copy link
Contributor

@jonathandturner I am reminded that I had planned to start a thread on internals to talk it over. I am not sure what I think would be best, but I feel pretty sure that I do not like the fact that //~ foo works (i.e., I think I would prefer limiting things to //~ [ERROR|WARNING|NOTE|HELP], which would then at least get the rule that notes/helps must be exhaustively checked if they are checked at all.

bors added a commit that referenced this issue Aug 9, 2016
…jonathandturner

Turn on new errors and json mode

This PR is a big-switch, but on a well-worn path:

* Turns on new errors by default (and removes old skool)
* Moves json output from behind a flag

The RFC for new errors [landed](rust-lang/rfcs#1644) and as part of that we wanted some bake time.  It's now had a few weeks + all the time leading up to the RFC of people banging on it.  We've also had [editors updating to the new format](https://github.com/saviorisdead/RustyCode/pull/159) and expect more to follow.

We also have an [issue on old skool](#35330) that needs to be fixed as more errors are switched to the new style, but it seems silly to fix old skool errors when we fully intend to throw the switch in the near future.

This makes it lean towards "why not just throw the switch now, rather than waiting a couple more weeks?"  I only know of vim that wanted to try to parse the new format but were not sure how, and I think we can reach out to them and work out something in the 8 weeks before this would appear in a stable release.

We've [hashed out](#35330) stabilizing JSON output, and it seems like people are relatively happy making what we have v1 and then likely adding to it in the future.  The idea is that we'd maintain backward compatibility and just add new fields as needed.  We'll also work on a separate output format that'd be better suited for interactive tools like IDES (since JSON message can get a little long depending on the error).

This PR stabilizes JSON mode, allowing its use without `-Z unstable-options`

Combined, this gives editors two ways to support errors going forward: parsing the new error format or using the JSON mode.  By moving JSON to stable, we can also add support to Cargo, which plugin authors tell us does help simplify their support story.

r? @nikomatsakis
cc @rust-lang/tools

Closes #34826
@bstrie bstrie added the A-diagnostics Area: Messages for errors, warnings, and lints label Aug 24, 2016
@lambda-fairy
Copy link
Contributor

Should this be closed now, since old school errors no longer exist (#35401)?

@sophiajt
Copy link
Contributor Author

sophiajt commented Dec 5, 2016

@lfairy - good catch

@sophiajt sophiajt closed this as completed Dec 5, 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
Projects
None yet
Development

No branches or pull requests

5 participants