Stream: t-compiler/wg-diagnostics

Topic: get rid of error codes


oli (Jan 30 2020 at 14:59, on Zulip):

Hi @WG-diagnostics @GuillaumeGomez . Let's talk about replacing numeric error codes with string names. This happens in #67086. Please pick a time at https://terminplaner4.dfn.de/KlpwSW4AuBoCofUS I'll leave a more detailed explanation of the things to discuss below

GuillaumeGomez (Jan 30 2020 at 15:05, on Zulip):

Done! Also: it's very likely I won't be around on zulip so don't hesitate to ping me on discord.

oli (Jan 30 2020 at 15:14, on Zulip):
  1. Do we want to expose names instead of numbers to users? That can also be done by just creating an id->name map that is used before printing the error.
  2. I had the feeling everyone is d'accord with nixing numeric codes and using nice string names just like we have lint names. This feeling is obviously wrong. So let's talk about the points here. @simulacrum mentioned it being hard to grep/tidy for. Maybe something like error_id!(some_name) would alleviate this concern?
  3. Should we check that an error name is only used once? Apparently we don't have this check anymore since quite some time and haven't had a problem with it. Does anyone think we should reintroduce such a check?
simulacrum (Jan 30 2020 at 15:15, on Zulip):

no, no, I'm totally on board with nice names

simulacrum (Jan 30 2020 at 15:16, on Zulip):

I am opposed to "E0303" vs. E0303 where the first is actually restricted to that tight format

oli (Jan 30 2020 at 15:16, on Zulip):

right, but we need some intermediate step before moving to full names

oli (Jan 30 2020 at 15:16, on Zulip):

otherwise we'll never get it merged

oli (Jan 30 2020 at 15:17, on Zulip):

if we did error_id!(E0303) this would still work as the macro could take an ident fragment

simulacrum (Jan 30 2020 at 15:17, on Zulip):

yes, I think error_id! macro is a good idea

simulacrum (Jan 30 2020 at 15:17, on Zulip):

(makes the lint checking easier too)

simulacrum (Jan 30 2020 at 15:18, on Zulip):

I am fine with moving to "E0303" as an intermediate step (to be "quickly" replaced)

oli (Jan 30 2020 at 15:18, on Zulip):

so basically all the use sites at macro call sites that were changed from E0303 could in fact stay as they are and internally call error_id! on the value, and all the non-macro call sites could just use error_id!.

simulacrum (Jan 30 2020 at 15:19, on Zulip):

yep, and (in theory) we can also make the macro accept either

simulacrum (Jan 30 2020 at 15:19, on Zulip):

i.e. internally we map this_is_a_nice_name to E0303 for now

oli (Jan 30 2020 at 15:19, on Zulip):

oh

oli (Jan 30 2020 at 15:19, on Zulip):

hmm, interesting

simulacrum (Jan 30 2020 at 15:19, on Zulip):

(or vice versa)

simulacrum (Jan 30 2020 at 15:20, on Zulip):

or, as another straw man, we make the macro accept symbols (i.e., sym::

simulacrum (Jan 30 2020 at 15:20, on Zulip):

and then intern the current ids

oli (Jan 30 2020 at 15:20, on Zulip):

hehe

oli (Jan 30 2020 at 15:20, on Zulip):

nice

simulacrum (Jan 30 2020 at 15:20, on Zulip):

that way it's all the same type and so forth and super easy to migrate to any string

oli (Jan 30 2020 at 15:21, on Zulip):

ah not sure if we have symbols available in librustc_error

oli (Jan 30 2020 at 15:22, on Zulip):

yea doesn't look like it

simulacrum (Jan 30 2020 at 15:22, on Zulip):

sure we do?

simulacrum (Jan 30 2020 at 15:23, on Zulip):

depends on librustc_span

simulacrum (Jan 30 2020 at 15:23, on Zulip):

which has sym (used to by syntax_pos)

oli (Jan 30 2020 at 15:23, on Zulip):

ah so it does

Esteban Küber (Jan 30 2020 at 16:47, on Zulip):

Do we have a proposal on the format for the new codes?

oli (Jan 30 2020 at 16:48, on Zulip):

nope, I would have assumed "like lints"

Esteban Küber (Jan 30 2020 at 16:50, on Zulip):

My main concerns are that by having a string code that might prove suboptimal we can't really change it afterwards, that the output will be less consistent (ie where do I look for the start of the message) that there's duplication between the message and the code. I feel that it'd be a bigger benefit to try and get --teach working everywhere than try and improve the "error code as non-semantic pointer to docs" problem

Esteban Küber (Jan 30 2020 at 16:50, on Zulip):

But I acknowledge that none of these are big issues

Esteban Küber (Jan 30 2020 at 16:50, on Zulip):

We might have problems dealing with I line docs for a while though

oli (Jan 30 2020 at 16:51, on Zulip):

I feel no general pressure to do these changes now rather than later, so if you think there's some benefit to delaying, let's do that

centril (Jan 30 2020 at 22:38, on Zulip):

I don't think sym::the_code is a good idea

centril (Jan 30 2020 at 22:38, on Zulip):

It forces recompilation of rustc_span if you want to add a new error code

centril (Jan 30 2020 at 22:39, on Zulip):

which is basically recompiling the whole compiler

centril (Jan 30 2020 at 22:39, on Zulip):

regarding error_id!.. we already have error_code! in rustc_errors... what's the difference beyond code -> id ?

simulacrum (Jan 30 2020 at 22:39, on Zulip):

er, yeah, error_code would be the macro used I guess

simulacrum (Jan 30 2020 at 22:40, on Zulip):

(as a side note, I just did goto definition on a proc macro and it worked with RLA and I am speechless)

simulacrum (Jan 30 2020 at 22:41, on Zulip):

regardless, I think the issue of recompiling everything when adding a new symbol are not too interesting, i.e, we can make it work (there's no actual reason all symbols need to be in the same place, for example)

simulacrum (Jan 30 2020 at 22:41, on Zulip):

just the macro today requires that

centril (Jan 30 2020 at 22:41, on Zulip):

@Zoxc ^---

centril (Jan 30 2020 at 22:42, on Zulip):

but I think that should be fixed first though

simulacrum (Jan 30 2020 at 22:42, on Zulip):

but it could be pretty easily written I think to not need that, we'd need some constants that we update (offsets, basically); but it would only need to be updated every so often - we could give ourselves some "margin" so to speak

centril (Jan 30 2020 at 22:42, on Zulip):

we shouldn't regress dev UX in the interim

simulacrum (Jan 30 2020 at 22:42, on Zulip):

my understanding is that we add error codes like very rarely

centril (Jan 30 2020 at 22:43, on Zulip):

I mostly don't add error codes cause I just avoid error codes

centril (Jan 30 2020 at 22:43, on Zulip):

but I fairly frequently do add new errors

simulacrum (Jan 30 2020 at 22:43, on Zulip):

in any case, if we truly want to prevent typos, the way to do it is a global list

simulacrum (Jan 30 2020 at 22:43, on Zulip):

(and compiler or tidy-checking afterwards)

centril (Jan 30 2020 at 22:45, on Zulip):

I think my idealized version would be something like this:

centril (Jan 30 2020 at 22:46, on Zulip):

That is, descriptions would not be part of the rustc program

simulacrum (Jan 30 2020 at 22:47, on Zulip):

I don't personally see cause for the latter (it can be in a crate that is a dependency of say, rustc_driver or even rustc the binary crate).

The first, though, seems to match what I'm thinking with symbols

centril (Jan 30 2020 at 22:47, on Zulip):

Although it is worth noting that an advantage of error codes is that they are short, which makes them stick out less in typical error messages, whereas syntax::missing_comma is not short.

simulacrum (Jan 30 2020 at 22:47, on Zulip):

I think that's in practice something we could fix separately

simulacrum (Jan 30 2020 at 22:48, on Zulip):

e.g. Cargo could learn to not show error codes

simulacrum (Jan 30 2020 at 22:48, on Zulip):

(well, Cargo could learn to tell rustc to not show error codes)

simulacrum (Jan 30 2020 at 22:48, on Zulip):

I still feel like for 99% of errors they're not useful

simulacrum (Jan 30 2020 at 22:48, on Zulip):

(e.g., I have no idea how we can elaborate on missing comma :)

centril (Jan 30 2020 at 22:49, on Zulip):

fwiw, missing comma is a fairly simple error

centril (Jan 30 2020 at 22:49, on Zulip):

there are much more complicated ones in terms of understanding

centril (Jan 30 2020 at 22:49, on Zulip):

(looking at you, async-await errors...)

simulacrum (Jan 30 2020 at 22:49, on Zulip):

sure, yes

centril (Jan 30 2020 at 22:50, on Zulip):

I don't personally see cause for the latter (it can be in a crate that is a dependency of say, rustc_driver or even rustc the binary crate).

That's the current state; rustc_error_codes is only a dep of rustc_driver

centril (Jan 30 2020 at 22:51, on Zulip):

But moving them to dynamically loaded "text files" would be even later "linking"

simulacrum (Jan 30 2020 at 22:51, on Zulip):

My point is that in most cases we don't need an error code (so adding it can be costly from compiletime perspective) and when we do, it can be long, because you're (more) likely to look at it.

centril (Jan 30 2020 at 22:51, on Zulip):

such that you can swap out descriptions at runtime

simulacrum (Jan 30 2020 at 22:52, on Zulip):

I am against text files that we load at runtime in rustc, though not the idea in general. If we do that, we might as well just not include them in rustc and have a error code index (HTML/Markdown) like we do today, nixing rustc --explain.

centril (Jan 30 2020 at 22:52, on Zulip):

Haven't we been hearing that error codes are commonly useful?

simulacrum (Jan 30 2020 at 22:52, on Zulip):

We have heard that error codes are useful, yes, but that's not the assertion I'm making

centril (Jan 30 2020 at 22:52, on Zulip):

I don't understand the difference in the distinction

simulacrum (Jan 30 2020 at 22:53, on Zulip):

I'm saying most errors probably don't benefit from one. Those that do, are harder to fix -- i.e., you need to look at the error for a bit -- so having eye travel across a longer error code isn't a problem.

centril (Jan 30 2020 at 22:53, on Zulip):

Hmm; I don't feel confident in making a statistical judgement like that

centril (Jan 30 2020 at 22:53, on Zulip):

Remember, we are biased "language experts"

simulacrum (Jan 30 2020 at 22:55, on Zulip):

hm, I don't really see that being related, in the sense that all I'm saying is that error[a very long code that is quite long]: foo isn't great, yes

simulacrum (Jan 30 2020 at 22:55, on Zulip):

but it's also true that such errors are ones you look at for a while anyway

simulacrum (Jan 30 2020 at 22:55, on Zulip):

but okay, let's presume I'm wrong

centril (Jan 30 2020 at 22:56, on Zulip):

I am against text files that we load at runtime in rustc, though not the idea in general. If we do that, we might as well just not include them in rustc and have a error code index (HTML/Markdown) like we do today, nixing rustc --explain.

Can you elaborate on why? It seems to me that error descriptions are only ever needed when there's an error, which is a slow path, so perf shouldn't be an issue. Also, what's the plan for internationalization otherwise (maybe there isn't one?)

centril (Jan 30 2020 at 22:57, on Zulip):

Seems we both agree that we need to keep error code length under wraps or they start losing their value :slight_smile:

simulacrum (Jan 30 2020 at 22:58, on Zulip):

There is no known plan for internationalization, to my knowledge. I think using it as an argument is odd, it's too far out, and there's no known plan.

It's not about slowness. It's about the fact that "many files on disk" fits poorly with (for example) distros

centril (Jan 30 2020 at 22:58, on Zulip):

Finding a naming scheme that achieves that is a tough nut though

centril (Jan 30 2020 at 22:59, on Zulip):

I would be interested in hearing from @Manish Goregaokar re. descriptions and translation

simulacrum (Jan 30 2020 at 22:59, on Zulip):

What if we move the error code to a note?

simulacrum (Jan 30 2020 at 22:59, on Zulip):

then it can be a link, even

simulacrum (Jan 30 2020 at 22:59, on Zulip):

or at least length is no longer an issue

centril (Jan 30 2020 at 23:00, on Zulip):

@simulacrum hmm, can you try a sample error from today and adjust it?

simulacrum (Jan 30 2020 at 23:00, on Zulip):

I'm confused

simulacrum (Jan 30 2020 at 23:00, on Zulip):

you mean like paste an example of how it looks?

centril (Jan 30 2020 at 23:00, on Zulip):

and how you would change it with your "move to a note"

simulacrum (Jan 30 2020 at 23:02, on Zulip):
error[E0502]: cannot borrow `x` as immutable because it is also borrowed as mutable
 --> src/main.rs:4:13
  |
3 |     let a = &mut x;
  |             ------ mutable borrow occurs here
4 |     let b = &x;
  |             ^^ immutable borrow occurs here
5 |     *a = 3;
  |     ------ mutable borrow later used here
error[borrow_check::double_mutable]: cannot borrow `x` as immutable because it is also borrowed as mutable
 --> src/main.rs:4:13
  |
3 |     let a = &mut x;
  |             ------ mutable borrow occurs here
4 |     let b = &x;
  |             ^^ immutable borrow occurs here
5 |     *a = 3;
  |     ------ mutable borrow later used here
error: cannot borrow `x` as immutable because it is also borrowed as mutable
 --> src/main.rs:4:13
  |
3 |     let a = &mut x;
  |             ------ mutable borrow occurs here
4 |     let b = &x;
  |             ^^ immutable borrow occurs here
5 |     *a = 3;
  |     ------ mutable borrow later used here

    help: longer explanation can be viewed via `rustc --explain borrow_check::double_mutable`
centril (Jan 30 2020 at 23:04, on Zulip):

error[borrow_check::double_mutable]: could probably be error[borrow::double_mut]:

centril (Jan 30 2020 at 23:04, on Zulip):

last one looks good though

centril (Jan 30 2020 at 23:05, on Zulip):
    help: use `rustc --explain borrow_check::double_mutable` for a longer explanation
    help: longer explanation can be viewed via `rustc --explain borrow_check::double_mutable`
simulacrum (Jan 30 2020 at 23:05, on Zulip):

yeah, the last one is my option for 'here is how we avoid all problems with length' basically

simulacrum (Jan 30 2020 at 23:06, on Zulip):

(and get the advantage that it's easier to drop/edit/etc, in theory)

centril (Jan 30 2020 at 23:06, on Zulip):

too bad we can't hyperlink terminal texts lol

centril (Jan 30 2020 at 23:06, on Zulip):

such an outdated medium :D

simulacrum (Jan 30 2020 at 23:07, on Zulip):

hm, well, we can, I think

simulacrum (Jan 30 2020 at 23:07, on Zulip):

or at least on some platforms

simulacrum (Jan 30 2020 at 23:07, on Zulip):

but rustc should probably not be doing that

centril (Jan 30 2020 at 23:09, on Zulip):

So I like your third snippet; I think that's what we should do

simulacrum (Jan 30 2020 at 23:12, on Zulip):

:thumbs_up: from me fwiw

centril (Jan 30 2020 at 23:13, on Zulip):

that seems like a move we can do fairly independently

centril (Jan 30 2020 at 23:13, on Zulip):

though it will need:

  1. adjustements to tests so that they don't mention error codes
  2. p=100 to avoid bitrot :D
centril (Jan 30 2020 at 23:13, on Zulip):

we can do 1. first and separately

Manish Goregaokar (Jan 30 2020 at 23:24, on Zulip):

@simulacrum @centril there is definitely a plan for internationalization :)

nagisa (Jan 31 2020 at 01:49, on Zulip):

When the code is constant-length its easy to skim past it when its not relevant to you

nagisa (Jan 31 2020 at 01:50, on Zulip):

long names will invariably be… variable

GuillaumeGomez (Feb 11 2020 at 10:59, on Zulip):

So if I understood correctly, we'll go for string representation as an intermediate step. Then I'll finish my PR.

Jake Vossen (Feb 20 2020 at 17:00, on Zulip):

So I just decided to try to take up this PR, should I hold off on working on it until this change is made (or decided to be not made)? https://github.com/rust-lang/rust/issues/44710#issuecomment-588453311

Jake Vossen (Feb 20 2020 at 17:00, on Zulip):

I would also be willing to help with the transition if you need it

simulacrum (Feb 20 2020 at 20:56, on Zulip):

I think we're not yet ready to move of numeric error codes

simulacrum (Feb 20 2020 at 20:56, on Zulip):

My comment is totally outdated now too...

Jake Vossen (Feb 20 2020 at 23:36, on Zulip):

Alright, I will go ahead with work on that issue then

Last update: Jul 02 2020 at 19:55UTC