Don't crash on error codes passed to --explain which exceed our internal limit of 9999

[Kivooeo]

removed panic in case where we do --explain > 9999 and added check for it

now error looks like this instead of ICE

$ rustc.exe --explain E10000
error: E10000 is not a valid error code

fixes #140647
r? @fmease

[rustbot]

fmease is on vacation.

Please choose another assignee.

[rustbot]

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[jieyouxu]

I feel like this is very arbitrary. The 9999 limit is just an internal implementation detail (an internal "this is obviously wrong" check and would be bumped if we ever get to that many error codes) and shouldn't be exposed to the user. I would've just expected

$ rustc --explain E12345
error: E12345 is not a valid error code

to behave the same as

$ rustc --explain E4444
error: E4444 is not a valid error code

and

$ rustc --explain E12345678901234567890 # exceeds u32::MAX
error: E12345678901234567890 is not a valid error code

to gracefully fail with the same error message.

[Kivooeo]

@jieyouxu any actual idea why tidy start fails here?

tidy check
tidy error: Error code `E9999` is used in the compiler but not defined and documented in `compiler/rustc_error_codes/src/lib.rs`.

like... should I just add E9999 there..?
and about your suggestion, you asking to merge both errors into one error_code is not a valid error code right?

[rustbot]

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

[Kivooeo]

oh no, looks like adding E9999 was a miskate here
but it actually makes tidy pass, so idk, correct me here pls

[fmease]

any actual idea why tidy start fails here?

I'm pretty sure it's simply due to tidy (src/tools/tidy/src/error_codes.rs:361) grepping through ~all Rust compiler source files looking for \bE\d{4}\b and finding the E9999 in your previous error message that you've since removed.

[ShE3py]

You may want to add a regression test based on tests/ui/explain.rs

[Kivooeo]

@ShE3py oh, yes i want to add tests for it, didnt knew that there is some for --explain, should i create it separate tests or can do it in one file?

yeah, i guess i cant do this in one file
error: Option 'explain' given more than once

[ShE3py]

Yup, that's unfortunate but AFAIK you can only run rustc once per file.

Btw, //~? may be interesting to use:
https://rustc-dev-guide.rust-lang.org/tests/ui.html#error-without-line-information

[Kivooeo]

yep, //~? is a nice one

[fmease]

Two suggestions then we should be good to go!

[Kivooeo]

@fmease seems done, but check pls if i got you and made all right
i also may should add some tests for other cases like valid error code for error that not exists or just something weird like word instead of number etc

[fmease]

i also may should add some tests for other cases like valid error code for error that not exists or just something weird like word instead of number etc

If you'd like to, go right ahead, sure sounds like a good idea.

[Kivooeo]

@fmease should be good to go, added few more test cases

[davidtwco]

@bors r=davidtwco,fmease rollup

[bors]

📌 Commit 3c1c072 has been approved by davidtwco,fmease

It is now in the queue for this repository.