Skip to content

Deprecate and soft-remove error codes #67068

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

Closed
Mark-Simulacrum opened this issue Dec 5, 2019 · 22 comments
Closed

Deprecate and soft-remove error codes #67068

Mark-Simulacrum opened this issue Dec 5, 2019 · 22 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Mark-Simulacrum
Copy link
Member

Today's compiler team meeting seemed to expose rough consensus that error codes as they stand today are not helping end-users fix code, or helping us develop the compiler.

This is because error codes do not meaningfully identify an error to an experienced user, so one cannot derive meaning from them without grepping the compiler codebase (and, sometimes, --explain can help). However, in many cases, a single error code is emitted for multiple similar errors (from a user perspective) but internally the causes of those errors may be quite different. It is also true that many of the complicated Rust errors that users ask for help with are not ones that can be readily explained in the abstract -- looking and helping with specific code is necessary.

It is then our impression that removing the error codes, as detailed below, is a reasonable step to take.

This issue proposes that we deprecate the --explain flag to the compiler and silence emission of error codes in compiler messages. That is, we would no longer emit error[EXXX]: ... instead just emitting error: ..., and no longer print For more information about this error, try "rustc --explain E0282".. We would also stop publishing new versions of the error index.

We would not delete the infrastructure in the compiler that supports this functionality, though, i.e., error code definitions, explanations, error emission would still take E3303 or a similar error as it does today. Furthermore, if -Zui-testing is passed, the error codes would be essentially emitted just as before. This means that reverting the deprecation/removal would be fairly easy. (Notably, we currently rely on these error codes in tidy to check that they're still being emitted by the compiler. Arguably, that's not immediately helpful, but we don't want to break that functionality for no reason if we decide to later revert).

If there are no significant complaints from end users, we may consider removing the functionality completely (i.e., no longer keeping the error code registry and explanations around at all) at a later point in time, but this explicitly does not propose making such a step. It has been noted during discussion of this on Discord that the future of error emission may involve something like #61132 which would somewhat supersede numeric error codes. @eddyb also points out that if we don't go that far, we can at least try to get wins by replacing numeric error codes with alphabetic identifiers, such as error[BorrowCk.UseAfterMove]: ... to allow grouping errors by type and/or potentially emitting no more than one of a particular category of error.

I am filing this issue here, rather than as (for example) a compiler team meeting proposal, because I would like to get some feedback and discussion in an async context, and perhaps even come to a consensus on this plan, without using up a full Friday slot.

cc @rust-lang/wg-diagnostics @GuillaumeGomez

@Mark-Simulacrum Mark-Simulacrum added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 5, 2019
@eddyb
Copy link
Member

eddyb commented Dec 5, 2019

To be fair, I did mostly refer to #61132 when talking about (alphabetic) "identifiers", but I agree that we can do something along those lines even without #61132.

@nikomatsakis
Copy link
Contributor

I would like feedback from newer users on this point -- it might be that a internals or users thread would help get useful data.

@Mark-Simulacrum
Copy link
Member Author

I will endeavor to make a post to users (I think internals, at least presumably, is not a good venue for finding representative beginners). Maybe a twitter poll or something along those lines makes sense, too, phrased along the lines of "Have you used --explain?" (Since error codes otherwise do not serve a purpose).

@frondeus
Copy link

frondeus commented Dec 6, 2019

I know few people in my company which were recommending using --explain for beginners. My boss said it is a blessing that --explain gives you more context why error happened and what are common solutions.

But in last year I used it maybe twice.

@fan-tom
Copy link

fan-tom commented Dec 6, 2019

I used error codes a lot when started learning rust, but mostly by error-index than by --explain, epsecially because IntellijIDEA automatiaclly turns them to link and opening error description is as easy as clicking that link.

@estebank
Copy link
Contributor

estebank commented Dec 6, 2019

Just published https://twitter.com/ekuber/status/1203005186228146176 to unofficially get some numbers.

The problem I see is that we do not have any way of tracking usage of --explain, so we don't know whether it is useful, and we can get numbers from the error code index, but it'll be hard to figure out how many of those visitors come due to the error code and how many come there from the error message.

@estebank
Copy link
Contributor

estebank commented Dec 6, 2019

From the description of the conversation (haven't read the meeting transcript yet and I couldn't attend it when it happened) it seems to me that it would be an argument to expand the index with many more specific cases. I do feel that a lot of the shorter descriptions can be subsumed by the diagnostic itself, but there are cases where we can't identify which of a handful of cases we've encountered so we need to describe 3 or 4 different causes for the current problem. For those later cases, the index will always be a useful communication tool.

@skade
Copy link
Contributor

skade commented Dec 6, 2019

We got active positive feedback at RustBridge Berlin on the errors and their explanations.

@eddyb
Copy link
Member

eddyb commented Dec 6, 2019

Also, in case this is unclear: we can get the benefits of --explain without error codes, by having a flag to add the explanations after every error and silence duplicate errors etc.

It would likely be more useful than codes, including for IDEs, for the same reason codes are inconvenient for me: they are like a pointer address I have to manually "dereference".

@estebank
Copy link
Contributor

estebank commented Dec 6, 2019

@eddyb the problem I see with that is if we update a diagnostic but somehow forget to update the index, then we loose searchability. Also, error codes rarely change their meaning over time, but the messages can be wildly different as we iterate and improve on things. This means the potential pool of discoverable documentation decreases over time.

The idea you're talking about sounds like --teach to me, but that will require even more boilerplate throughout and increases the threshold for new contributors from "write a new markdown file and add to a list" to "write code with understanding of the DiagnosticBuilder API".

@Ixrec
Copy link
Contributor

Ixrec commented Dec 7, 2019

Although I'm not a "new user", I very much like the idea of:

replacing numeric error codes with alphabetic identifiers, such as error[BorrowCk.UseAfterMove]: ... to allow grouping errors by type and/or potentially emitting no more than one of a particular category of error.

and would like to add to this that for many errors, there may be multiple things worth explaining, and human-readable identifiers give us a useful way of distinguishing them. For example, if a user runs into a borrow check error involving async/await and closures, I would expect some users need to --explain the basic borrow checking rules, other users need to --explain closure captures, while still others need to only --explain borrowing across yield points. This seems potentially more helpful to users and easier to maintain than writing a single very long error explanation for every specific combination of circumstances that we put a different short error message on, without all the uncertainty and complexity that --teach-like proposals bring.

@JohnTitor JohnTitor added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Dec 8, 2019
@jrop
Copy link

jrop commented Dec 9, 2019

I rather like the "explain" system (I have used it a few times), although I suppose that an alpha-numeric system may be more ergonomic.

@estebank
Copy link
Contributor

With a relatively small sample, I am surprised that more people have used --explain exclusively than the index exclusively. About 2/3rds use one and/or the other.

a third of 133 rust users do not use explain or the error code index

@Lokathor
Copy link
Contributor

I had no idea there was an index.

@estebank
Copy link
Contributor

estebank commented Dec 13, 2019

@Lokathor https://doc.rust-lang.org/error-index.html, it should be the first result for "<ERROR CODE> rust".

@Lokathor
Copy link
Contributor

Not on the first page of google results for me.

Anyway, my point was that I've used rust since 1.16, and I hit a lot of errors, and I never paid enough attention to find out about some website thing.

@thomcc
Copy link
Member

thomcc commented Dec 13, 2019

I also had no idea and have definitely searched for the error code before, and it sometimes helps find the issue.

I feel like, instead of/along with saying E0072 in the error message, linking to https://doc.rust-lang.org/error-index.html#E0072 directly (instead of hoping search would drag it up) would have been very helpful in surfacing this index and helping users understand the issue.

That said I don't really care, but I've also run headfirst into most of these errors enough times that they've left a permanent imprint on my brain. Between the two of these (codes vs index), the index actually seems to be the more useful thing if it were linked to (The code just serves as an anchor for the link).

@nikomatsakis
Copy link
Contributor

I think the survey results settle it for me -- I wouldn't want to deprecate the error codes without some convincing replacement (but I think there are lots of plausible things). I'm also remembering various times (e.g., future incompatibility etc) where it's been great to be able to give more context in the --explain.

(I still prefer the idea of 'longer messages that are given in place', myself, but building such feature would be hard and take a lot of iteration I suspect?)

@Mark-Simulacrum
Copy link
Member Author

I tend to agree here that we've reached the rough conclusion that we cannot just remove them. With that in mind, I'm going to close this issue, though if someone wants to reopen and use this as a place to try and come up with a reworked plan to move forward, please do so -- I don't have the bandwidth for that myself right now.

@panstromek
Copy link
Contributor

I think it's worth adding that intellij-rust uses error codes in helper messages and create clickable links to error-index from them.

image

They also use them to track progress of their analysis: intellij-rust/intellij-rust#886

I'd also add that error code is more reliable identifier than error message, because message is more likely to change. This helps a bit when looking for help online. I'd actually welcome more error codes for more specific cases, because some of them are too general atm (as mentioned). That's just a beginner perspective, though.

@GuillaumeGomez
Copy link
Member

What we had in mind was something similar to error codes like identifier such as "invalid-lifetime" or equivalent.

@panstromek
Copy link
Contributor

@GuillaumeGomez I see, that's indeed a better idea ;)

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 C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests