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

Default and expanded errors for rustc #1644

Merged
merged 14 commits into from Jul 14, 2016

Conversation

Projects
None yet
@jonathandturner
Copy link
Contributor

commented Jun 7, 2016

Proposed update to the Rust compiler error output format.

rendered (last updated: 6/27/2016 8:30am)

@jonathandturner jonathandturner changed the title Create 0000-rust-new-error-format.md Default and expanded errors for rustc Jun 7, 2016


There are a few unresolved questions:
* Editors that rely on pattern-matching the compiler output will need to be updated. It's an open question how best to transition to using the new errors. There is on-going discussion of standardizing the JSON output, which could also be used.
* Can additional error notes be shown without the "rainbow problem" where too many colors and too much boldness cause errors to beocome less readable?

This comment has been minimized.

Copy link
@alilleybrinker

alilleybrinker Jun 7, 2016

"beocome" -> "become"

This comment has been minimized.

Copy link
@ticki

ticki Jun 8, 2016

Contributor

I am sure this is a subjective problem. Some people prefer the colorful approach, others not so much, so I think configurability is the key here.

@petrochenkov

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2016

I'm probably not the main target auditory, but here's one data point.
The green areas is where my sight is usually focused when I look at error messages. The time frame is about few seconds. The rest of the area, especially long explanations, is pretty much noise and is never read.
error_message

EDIT: the red error_message - copy is important though as a visual start marker.

@jonathandturner

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2016

@petrochenkov - thanks! That's actually a really cool analysis. It's totally natural to skip around and only read the parts you want to read.

I don't talk about it much in this RFC, but I did a quick survey of how small number of expert Rust developers used the current error system. Many of them just looked at the line number (and possibly the start of the error message) and ignored the rest. They trusted their ability to debug with just the location in mind.

Being able to be visually parsed as you described is definitely an improvement and to have that much coverage in a few seconds is very promising.

Thanks again.

@bstrie

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

I feel like there are two orthogonal RFCs here. We should be able to discuss the new error formatting independently of whether explain should be redesigned.

@nrc nrc added the T-dev-tools label Jun 8, 2016

@phil-opp

This comment has been minimized.

Copy link

commented Jun 8, 2016

IMO the separator of the line number column is a bit noisy. How about just | instead of |>?:

rust-new-error-format-alternative

I think it's less visual clutter and much easier to read, especially the last lines. For example, there is no |> } anymore.

@ticki

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

I agree with @petrochenkov. A lot of it is not relevant at least 90% of the time, but it may be so for new beginners, so having different log-level-esque errors would be very cool.


# Alternatives

Rather than using the proposed error format format, we could only provide the verbose --explain style that is proposed in this RFC. Famous programmers like [John Carmack](https://twitter.com/ID_AA_Carmack/status/735197548034412546) have praised the Elm error format.

This comment has been minimized.

Copy link
@ticki

ticki Jun 8, 2016

Contributor

dybuk is an implementation of something like this.


Rather than using the proposed error format format, we could only provide the verbose --explain style that is proposed in this RFC. Famous programmers like [John Carmack](https://twitter.com/ID_AA_Carmack/status/735197548034412546) have praised the Elm error format.

![Image of Elm error](http://www.jonathanturner.org/images/elm_error.jpg)

This comment has been minimized.

Copy link
@ticki

ticki Jun 8, 2016

Contributor

This image is rather visual trickery. You should make sure that the same colorscheme is used throughout the RFC, to avoid unintended bias.

@jonathandturner

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2016

@bstrie - possibly, though this lays out what should be default as well (vs say just making the Elm-style the only error output)

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jun 8, 2016

I'm pretty excited to see this move forward, I've been using these errors messages on nightly and they're fantastic compared to the old system, and I can't wait to basically just get more of them.

I also really like the idea of --explain going inline with your text and being more descriptive of the code you literally wrote, that seems to be invaluable for learning code quickly as it reduces the path to working code (you just copy/paste what the errors tell you to do) and may help understanding of the rules in abstract over time. Maybe even one day we can have a mode of the compiler that auto-applies all these fancy suggestions and writes your code for you!

One part about removing the error code of --explain though is that we're losing a way to uniquely identify errors. I don't think this is too useful on the command line (kinda annoying to copy/paste), but I've heard it's very useful for googling or search around for solutions others have applied (e.g. when we don't have a suggestion to apply). I wonder if --explain could just expand the error text with a code somewhere in it though? Or maybe we could just unconditionally put the error codes in there all the time somewhere inconspicous (they're pretty small).

My only other thought is that on the removal of "error: aborting due to 2 previous errors" I actually really like seeing the total number of errors that were emitted sometimes. If you do some invasive refactoring and fix everywhere you remembered, this gives a great "progress bar" of how far you have left to go. Maybe that cold be folded into some message elsewhere though? I do agree that the dedicated "error: " line is a bit overkill so I don't really have a preference as to where it shows up so long as it's at the end.

@alilleybrinker

This comment has been minimized.

Copy link

commented Jun 8, 2016

Piggybacking on @alexcrichton's point, I know I've definitely appreciated the searchable error codes before. It makes it a lot easier to find explanations or people with similar situations, and a lot easier to help other people (they can say "I got this error code" and you have a solid sense of what went wrong, very quickly and succinctly).

@jonathandturner

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2016

@alexcrichton @AndrewBrinker - Definitely agree that keeping it google-able is a good design feature. Rather than using the --explain EXXX pattern, we could just have the error code be part of the error message. Maybe something like:

screen shot 2016-06-08 at 10 07 13 am

(and we could use a similar header for the --explain)

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

I think just keeping the codes, without having them be particularly important to explain, makes a lot of sense.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

(Oh, and I see no reason we can't also tell you the total number of errors while we let you know about --explain)

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

@bstrie Do you object to the idea of having --explain incorporate parts of the error? It feels like it fits thematically into this RFC -- which is to generally update the "look and feel" over error handling -- and I don't see why we would split it off absent some significant controversy.

For example, @jonathandturner and I spent a while debating whether --explain ought to be the default -- or even if it could be made into the only option! -- which suggests that the two questions (default output, explain output) are quite linked. (We ultimately decided against that, obviously.)

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jun 8, 2016

@jonathandturner that looks really good to me! We could maybe toy around with other concepts like:

error[E0455]: cannot borrow `v` as mutable more than once at a time

but that's all in the weeds of bike shedding :). My basic point was that we probably shouldn't jettison error codes even if --explain doesn't take them, and it sounds like we're all on board with that!

@nikomatsakis yeah I also agree it should be trivial to keep the number of errors while we also explain --explain (see what I did there?).

@camlorn

This comment has been minimized.

Copy link

commented Jun 8, 2016

Can we please get these as text? I don't know what's being proposed because the RFC only uses images, and images don't read with screen readers.

Will this make errors shorter? I have to scroll the console somewhat often even with 4 or 5 errors. I'd love a nice one or two line output format, but that's probably not what this is.

@ticki

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

@camlorn I will transcribe them for you, two seconds.

@ticki

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

@camlorn The first image is the old error format, where the path is written in the start of every line.

The second image is the new format:

error: `self` is not available in a static method. Maybe a `self` argument is missing? [--explain E0424]
  --> vec.rs:94:23
   |>
94 |>         debug_assert!(self.len <= block.size() / mem::size_of::<T>(), "Block not large enough to \
   |>                       ^^^^

The third example describes the long form:

error: cannot move out of borrowed content:
  --> /Users/jturner/Source/errors/borrowck-move-out-of-vec-tail.rs:30:37

I'm trying to track the ownership of the contents of `tail`, which is
borrowed, through this match statement:

29 |>            match tail {

In this match, you use a pattern which will bind to a member of `tail` as a
value. This will move the contents out of `tail`. Because `tail` is
borrowed, you can't safely move the contents:

30 |>              [Foo { string: aa },
   |>                             ^^ cannot move out of borrowed content

You can avoid moving the contents out by using a reference in the pattern, rather
than a move. A naive fix might look like:

30 |>              [Foo { string: ref aa },

The fourth image is the same as the first one. The 6th image is the same as the first one, but with the header (i.e., the error) highlighted. The 7th is the same image again with the line numbers highlighted. The 8th is the same image again with the code span highlighted.

The 9th image is:

error: `Shrinkable` is not a trait [--explain E0404]
 --> rust_error2.rs:5:6
  |>
3 |> type Shrinkable = Growable;
  |> --------------------------- type aliases cannot be used for traits
5 |> impl Shrinkable for i32 { }
  |>      ^^^^^^^^^^ `Shrinkable` is not a trait

The 10th image is the same as the third one. The 11th example is another example of the new format.

The last image is Elm's format:

Detected errors in 1 module.

-- ALIAS PROBLEM ---------------------------------------------- ././Maze.elm

This type alias is recursive, forming an infinite type!

21|>type alias Node =
22|>    { x : Int
23|>    , y : Int
24|>    , children : List Node
25|>    }

When I expand a recursive type alias, it just keeps getting bigger and bigger.
So dealiasing results in an infinitely large type! Try this instead:

    type Node
        = Node { x : Int, y : Int, children : List Node }

This is kind of a subtle distinction. I suggest the naive fix, but you can
often do something a bit nicer. So I would recommend reading more at:
<https://github.com/elm-lang/elm-compiler/blob/0.17.0/hints/recursive-alias.md>

That's all.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

Interesting. There has also been a discussion around a so-called "traditional" format which would (I think) be something highly condensed of the form

file:line: error: message

and maybe even no additional detail? This would be useful for older
tools that don't really know anything about Rust; it might also help
for screen reader support, I suppose, though you would probably be
interested in having the full details I imagine (i.e., the notes where
a borrow begins and so forth)

On Wed, Jun 08, 2016 at 12:14:45PM -0700, Austin Hicks wrote:

Can we please get these as text? I don't know what's being proposed because the RFC only uses images, and images don't read with screen readers.

Will this make errors shorter? I have to scroll the console somewhat often even with 4 or 5 errors. I'd love a nice one or two line output format, but that's probably not what this is.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#1644 (comment)

@ticki

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

I wonder what we can do for people like @camlorn with respect to the error format. @camlorn, what would be your ideal error format?

@camlorn

This comment has been minimized.

Copy link

commented Jun 8, 2016

thanks for the transcription.

As for what we can do generally? Convince Microsoft to let me set the font in the console even smaller. I don't need readable...

More seriously...

This is an opinion. I'm kind of unique in a lot of ways, not least that I'm one of the few blind people who considers C++ easy. My considerations may not be the considerations of a new blind programmer.

Firstly, we don't need --explain for advanced rust people. This takes terminal space. I know it exists. Reminding me of it is just an annoying thing to read. Obviously it needs to be on by default, but turning it off somehow would be nice. We could move the error code to be in brackets after the line numbers, or just in brackets after the message.

Prefixing with |> is pretty much the same as prefixing with ///. All my comments on the "Only allow /// for docs" rfc apply here: it's annoying but readable if I have to.

The underlines to show where the error is are useless completely. I don't think there's a particularly good solution unless we have a character which will be always reserved. If we did, we could surround the error with it in the same manner that you use parens. As it stands, I'd like to murder the underlines in the most painful way possible and not only for Rust. Surrounding the error's source with something might actually be helpful.

I've never quite used the information on borrow scopes. I don't know why. I think that part of it is that reading code in the terminal is inconvenient. I also find that my C++ experience puts me in a good place to reason about lifetimes.

Showing the surrounding code is not helpful. If I want to see the surrounding code, I'll use go to line in my editor. I think this one is a fundamental difference between blind and sighted people. There is no way for me to glance at anything, so the cost of reading the code in the terminal and then reading the code in the editor is about twice that of just using the editor. If I have to scroll to see it, it's more. If we can find a way to emphasize the error, showing the code for the error might be useful.

If I had to make a suggestion, provide the line numbers of the borrow regions, possibly with the text of the line. This might be { and } in practice, so that might not be so useful.

If we assume that this is a hypothetical terse error mode that you explicitly enable with an environment variable, you could have a switch to explicitly disable it. In that case, if I need the information in the expanded version, I can still get it.

Sorry I can't speak more definitely. I started a large project in Rust, but the larger project in C++ took priority for now. This means that I don't have enough experience to have a workflow that's a procedure as such. With the C++ compiler, I could document exactly what I press because it's always the same. With Rust, it's not quite to the level of mechanical procedure yet.

@brson

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

I like the direction of this RFC generally.

--explain is a stable compiler interface. How can we change it? I agree that --explain is a sweet name for this functionality, but I think we need to use a different one.

It's not clear to me what the fate of error codes is here. My impression is that you are proposing to remove them, but your images contain the current --explain E0002. Can you make this more clear, and if you are removing error codes can you remove them from the graphics?

"Famous programmers like John Carmack have praised the Elm error format."

John Carmack's fame doesn't really have any bearing on the validity of the proposal.

Also, I don't think RFCs should be giving props to other contributors in the summary. When creating the RFC process we made an explicit decision not to include author information, to avoid making RFC authorship itself an incentive (I think). If authorship is something that should be included in RFCs then the process itself should be modified to include that metadata, including crediting the actual author, not just their collaborators. If giving credit in the RFC text is something we want to encourage, then I don't know that the summary is the place for it. The summary is for summarizing the content of the RFC.

@nagisa

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

At the very least we should have a fully working json error output, which one could pass through an arbitrary filter of the form rustc >/dev/null 2>&1 | awesome_error_printer and thus making it possible to make rust output accessible, even if not by default. Sure that doesn’t change the fact about default experience being junk for people with special needs, but at least there would be an option, however minuscule.

@ticki

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2016

@camlorn @nagisa We should have a ACESSIBLITY_MODE variable.

@nielsle

This comment has been minimized.

Copy link

commented Jun 9, 2016

I think that it would be helpful for newbies to spend an entire sentence to introduce --explain E040

error: `Shrinkable` is not a trait [E040]
--> rust_error2.rs:5:6
  |>
3 |> type Shrinkable = Growable;
  |> --------------------------- type aliases cannot be used for traits
5 |> impl Shrinkable for i32 { }
  |>      ^^^^^^^^^^ `Shrinkable` is not a trait
For a detailed description of the error  type: rustc --explain E040

It would be great to have an ACESSIBILITY_MODE. Perhaps it could also control warnings about unoptimized builds #967.

The expanded error message effectively becomes a template. The text of the template is the educational text that is explaining the message more more detail. The template is then populated using the source lines, labels, and spans from the same compiler message that's printed in the default mode. This lets the message writer call out each label or span as appropriate in the expanded text.

It's possible to also add additional labels that aren't necessarily shown in the default error mode but would be available in the expanded error format. This gives the explain text writer maximal flexibility without impacting the readability of the default message. I'm currently prototyping an implementation of how this templating could work in practice.

This comment has been minimized.

Copy link
@nrc

nrc Jul 4, 2016

Member

It seems to me that there are two orthogonal features here that users might want either or both of: template explanations and inline explanations. There seems to be use cases for both (for both beginning and advanced users) so it might be nice to have access to both separately. It might be that we want to only expose one option to rustc users, but that IDEs can configure things in more detail (this might just come down to ignoring or rendering parts of the JSON errors, in practice).

This comment has been minimized.

Copy link
@nrc

nrc Jul 4, 2016

Member

As I mentioned earlier, I think the --explain errors should be a separate RFC, but in particular it seems premature to have an RFC without the templating language or other details specified.

This comment has been minimized.

Copy link
@jonathandturner

jonathandturner Jul 4, 2016

Author Contributor

I'm not sure I follow this point. The templating language piece seems more engineer-related rather than design related. We could iterate the templating piece without impacting the user's experience.

This comment has been minimized.

Copy link
@nrc

nrc Jul 5, 2016

Member

I guess I think of the interface we present to Rustdoc authors as a user-facing API, not just the output of Rustdoc. Therefore, there is a design question there that should be answered in an RFC.

This comment has been minimized.

Copy link
@jonathandturner

jonathandturner Jul 5, 2016

Author Contributor

The way I'm picturing this, only compiler authors who are writing error messages will see the templates. It's not intended to be a user-facing API for users to write their own. Similar to the existing error reporting mechanism the compiler currently uses, it would just have an additional "backend" of outputting to a template.

It sounds like you might have bigger ambitions for it? If so, we should chat.

Be changed to notify users of this ability:

```
note: compile failed due to 2 errors. You can compile again with `--explain errors` for more information

This comment has been minimized.

Copy link
@nrc

nrc Jul 4, 2016

Member

to clarify, this should only happen if in fact there is more information to show?

This comment has been minimized.

Copy link
@jonathandturner

jonathandturner Jul 4, 2016

Author Contributor

Right, with the assumption that in the fullness of time, we have --explain errors cover most, if not all, errors.


# Stabilization

Currently, these new rust error format is available on nightly using the ```export RUST_NEW_ERROR_FORMAT=true``` environment variable. Ultimately, this should become the default. In order to get there, we need to ensure that the new error format is indeed an improvement over the existing format in practice.

This comment has been minimized.

Copy link
@nrc

nrc Jul 4, 2016

Member

nit grammar: "these ... is"

@jonathandturner

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2016

@nrc

re:lack of experience with the templated format, are you specifically referring to the style of the text? In this RFC, we introduce only using the user's code rather than writing our own examples. We already have a fair bit of experience writing --explain text. It's true, though, that we haven't implemented this version, so I think it'd be natural to also keep it behind a flag for a cycle, like @alexcrichton suggested. I can see it maturing at a different rate than the new default errors, so we may decide one or the other needs longer bake time and opting to stabilize one first then the other.

You're right, though, it will need some iteration on the --explain errors side style-wise to make sure we're striking the right tone.

@nrc

This comment has been minimized.

Copy link
Member

commented Jul 5, 2016

I was referring to the whole thing of templated explains. AIUI, we haven't implemented any version, or have I missed something? I thought the one cycle behind a flag was with the latest version of the extended errors, rather than extended explain, which presumably would need to be behind a flag or something until it stabilised (for some definition).

The tone seems like something that can be evolved separately from the technical components, and it feels like the next RFC will deal with that?

@jonathandturner

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2016

@nrc - right, I'm just pointing out that we went a little out of order with the revised default errors in that we had it behind a flag before the RFC rather than the other way around. :) Yeah, the expanded ones can be behind a flag.

And yes, there will be a follow-up RFC on the tone. I mention it in this RFC in the last line of the summary.

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jul 5, 2016

🔔 This RFC is now entering its final comment period 🔔


My own personal opinion is that I'm super excited to see this in the compiler and the new error format is great. Having any functionality behind a flag for at least a cycle makes me comfortable in terms of stability, and this is also an area where the specifics I at least believe are relatively loose as tools would primarily desire a JSON-like interface rather than parse the current format (which is in the works).

@jonathandturner

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2016

Just to follow-up here, since nrc and I were chatting a bit offline.

Basically, because of the different levels of maturity of the default and the extended errors, each will be behind a separate flag. Default will align with this rfc, stay behind flag for a cycle, then move to being on by default. Extended errors will finish experimentation, get a follow-up rfc with implementation details, and this will kick off its final cycle in moving out from under flag.

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jul 6, 2016

@jonathandturner sounds good to me, could you ensure that the text of the RFC reflects that as well?

@jonathandturner

This comment has been minimized.

Copy link
Contributor Author

commented Jul 6, 2016

@alexcrichton - updated!

@brson

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2016

I'm happy with this, though I think the --explain errors flag is awkward. If not for legacy issues I would expect this to be a single flag (--explain :p). I might prefer this to be --explain-errors. Frustrating design constraints.

I agree with @nrc that in hindsight it would have been preferable for this RFC to be just about the new error format, and not deal with --explain at all. The two features seem orthogonal.

@brson

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2016

This RFC only specifies 1 feature name, but there are two stabilization cycles here, one for the new errors and one for the new explanations. Can a second feature be added?

@brson

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2016

Can this RFC address the matter of editors parsing the new text? What's the story for rust-mode specifically? Is this parseable with a regex? The Drawbacks section seems to imply this, but the examples show key error information on multiple lines so it's not obvious that a single regex can parse out errors.

@jonathandturner

This comment has been minimized.

Copy link
Contributor Author

commented Jul 12, 2016

@brson - I'd be happy to explore deprecating --explain. Since we're already going to have another RFC cycle to move the expanded errors along, perhaps we can explore replacing --explain there?

Not sure what you mean by "This RFC only specifies 1 feature name". Default errors and expanded errors are both explained here. Maybe I missed what you meant?

I've reached out to IDE authors, and they're aware of the change. Some are talking about doing JSON support, some only have minimal support for errors and don't parse, and we're working with those who are parsing the text and need to update their parser (vim specifically). I think this is okay to be offline with those authors since it'll be different for different editors. The regex details here are more specific to the regex the editors use. There are updated techniques, eg @nikomatsakis has updated his emacs install to support the newer errors, so the knowledge there is growing.

Before we turn it on by default, it'd be good to at least have editors moving to support it so that the support can be there when it's turned on in a stable release.

@brson

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2016

@brson - I'd be happy to explore deprecating --explain. Since we're already going to have another RFC cycle to move the expanded errors along, perhaps we can explore replacing --explain there?

Yes. Please make it an unresolved question here.

Not sure what you mean by "This RFC only specifies 1 feature name". Default errors and expanded errors are both explained here. Maybe I missed what you meant?

This line names a single feature, "default_and_expanded_errors_for_rustc". Presumably that one will apply to the new error format, and be attached to its tracking issue. What name will be attached to the tracking issue for enhanced --explain?

I've reached out to IDE authors, and they're aware of the change. Some are talking about doing JSON support, some only have minimal support for errors and don't parse, and we're working with those who are parsing the text and need to update their parser (vim specifically). I think this is okay to be offline with those authors since it'll be different for different editors. The regex details here are more specific to the regex the editors use. There are updated techniques, eg @nikomatsakis has updated his emacs install to support the newer errors, so the knowledge there is growing.

I don't think this addresses the concern that a single regex can pull out the error information from the text. In this example:

+error[E0499]: cannot borrow `foo.bar1` as mutable more than once at a time
+  --> src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:29:22

The word 'error' is one one line, and the file name and number are on another. It's not obvious to me that this can be reliably parsed as a single regex, which I suspect (bot don't know), is a requirement for less capable editors. rust-mode is probably not the right barometer since emacs can execute arbitrary lisp to parse buffers. IDEs are not the main concern since they are powerful and can use JSON output - it's lesser editors like e.g. Kate. Unfortunately I don't have a concrete example of an editor that gets just a single regex to parse the error buffer (maybe @nrc or @vadimcn know).

@jonathandturner

This comment has been minimized.

Copy link
Contributor Author

commented Jul 13, 2016

"The word 'error' is one one line, and the file name and number are on another. It's not obvious to me that this can be reliably parsed as a single regex, which I suspect (bot don't know), is a requirement for less capable editors." <- this is why we are reaching out to the authors of those editors. We're also looking at things we can do on our end, like better buffering of the error.

@jonathandturner

This comment has been minimized.

Copy link
Contributor Author

commented Jul 13, 2016

"What name will be attached to the tracking issue for enhanced --explain?"

Does this need to be named in this RFC? I'm confused.

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jul 14, 2016

The tools team got a chance to discuss this RFC the other day, and the decision was to merge!

There were some key points brought up while discussing this that the tools team felt needed to be addressed before stabilization, but implementation on an unstable basis seemed fine. Specifically, we were thinking:

  • Editors - before stabilizing, the new errors need to be proven to be parseable in a number of editors. For example we should have a command-line editor like vim/emacs working but we should also have a higher level IDE like IntelliJ working. Note that we're not requiring that every editor under the sun is up to date, just that the basics are proven out and no showstoppers arise.
  • --explain - a roughly separate portion of this RFC, we're going to consider it separately for stability from the main error format. That is, we think the new error format will get turned on by default before the --explain changes are available in a stable compiler. This will likely also require a further RFC depending on the extent of the changes.

In the meantime, we'll continue to have an opt-in to using the new error format (currently an environment variable) which will allow users to test out the errors, editors to start to catch up, etc.

Thanks again for all the discussion on this RFC everyone, and thanks @jonathandturner for pushing on this! Getting lots of people happy with a stylistic decision is no small feat!

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jul 14, 2016

Tracking issue for default errors: rust-lang/rust#34826
Tracking issue for --explain: rust-lang/rust#34827

Oh and one last point --

Does this need to be named in this RFC? I'm confused.

Ah as part of --explain we don't really have a feature name like we do for libraries, so I think it's fine to just create this on the fly like we do with many others

@alexcrichton alexcrichton merged commit fc6499c into rust-lang:master Jul 14, 2016

bors added a commit to rust-lang/rust that referenced this pull request Aug 9, 2016

Auto merge of #35401 - jonathandturner:enable_json_and_new_errors, r=…
…nikomatsakis

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

bors added a commit to rust-lang/rust that referenced this pull request Aug 9, 2016

Auto merge of #35401 - jonathandturner:enable_json_and_new_errors, r=…
…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

@chriskrycho chriskrycho referenced this pull request Dec 31, 2016

Closed

Document all features in the reference #38643

0 of 17 tasks complete

@chriskrycho chriskrycho referenced this pull request Mar 11, 2017

Closed

Document all features #9

18 of 48 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.