Add `ErrCode` #119972

nnethercote · 2024-01-14T21:47:05Z

Error codes are integers, but String is used everywhere to represent them. Gross!

This commit introduces ErrCode, an integral newtype for error codes, replacing String. The commit also introduces constants like E0123 for all the used error codes.

r? @estebank

rustbot · 2024-01-14T21:47:15Z

Some changes might have occurred in exhaustiveness checking

cc @Nadrieril

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

Some changes occurred in need_type_info.rs

cc @lcnr

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

rustc_macros::diagnostics was changed

cc @davidtwco, @compiler-errors, @TaKO8Ki

nnethercote · 2024-01-14T21:48:04Z

The first two commits (before the dummy ---- commit) are from #119922, which is this PR builds on.

Best reviewed one commit at a time.

cc @cjgillot @compiler-errors @oli-obk

GuillaumeGomez

Nice catch! :)

compiler/rustc_hir_typeck/src/errors.rs

nnethercote · 2024-01-16T02:10:06Z

I have updated the code with the new approach: defining a constant such as E0123 for every error code. This gives consistency and visual niceness and greppability.

nnethercote · 2024-01-16T03:45:09Z

BTW, defining all the error code constants in rustc_error_codes and the re-exporting them from rustc_errors seems a bit complicated. We could change register_diagnostics into a higher-order macro that is defined in rustc_error_codes but only called in rustc_errors, which means the constants would be defined in rustc_errors.

nnethercote · 2024-01-16T05:28:08Z

We could change register_diagnostics into a higher-order macro that is defined in rustc_error_codes but only called in rustc_errors, which means the constants would be defined in rustc_errors.

I have done this, I think it makes things nicer.

oli-obk

r=me with empty commit removed

oli-obk · 2024-01-16T08:24:06Z

README.md

this commit can be removed now

The first two commits (before the dummy ---- commit) are from #119922, which is this PR builds on.

Did you review those two commits from #119922 as well?

yes I did, didn't realize they were from a separate PR. Let's land the other one first then

👍 thanks

bors · 2024-01-17T10:34:35Z

☔ The latest upstream changes (presumably #119922) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk · 2024-01-17T11:18:49Z

r=me after rebase

cjgillot · 2024-01-20T00:14:47Z

compiler/rustc_errors/Cargo.toml

@@ -10,9 +10,11 @@ derive_setters = "0.1.6"
 rustc_ast = { path = "../rustc_ast" }
 rustc_ast_pretty = { path = "../rustc_ast_pretty" }
 rustc_data_structures = { path = "../rustc_data_structures" }
+rustc_error_codes = { path = "../rustc_error_codes" }


The point of rustc_error_codes was to avoid any dependency on the error code comments from the explanations this early in the dependency tree.
OTOH, the current way to decouple messages from code is using fluent. Should we go in this direction in the future? Or is the modification rate for this crate low enough now?

Interesting... the top-level comment in the crate says "This library is used to gather all error codes into one place, the goal being to make their maintenance easier." But nothing about the dependency graph. (I'm just noting this because it's a pet peeve of mine when that kind of important but non-obvious consideration isn't put into a comment somewhere, because it's exactly the kind of thing that should be put into a comment.)

Back to the original point: the approach we landed on of defining Ennnn constants makes the additional dependency unavoidable. Because we need one constant per error code, and those error codes are used everywhere, and they depend on the error code list. With the E!(0123) macro approach this additional dependency could have been avoided, because ErrCode and E! could have been defined in rustc_errors.

I see 93 commits to rustc_error_codes in 2023, though about a quarter of those appear to be merge commits. That doesn't seem terribly high to me.

nnethercote · 2024-01-21T22:38:39Z

I rebased.

@bors r=oli-obk

bors · 2024-01-21T22:38:42Z

📌 Commit 9ddc424 has been approved by oli-obk

It is now in the queue for this repository.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#118578 (core: introduce split_at{,_mut}_checked) - rust-lang#119369 (exclude unexported macro bindings from extern crate) - rust-lang#119408 (xous: misc fixes + add network support) - rust-lang#119943 (std::net: bind update for using backlog as `-1` too.) - rust-lang#119948 (Make `unsafe_op_in_unsafe_fn` migrated in edition 2024) - rust-lang#119999 (remote-test: use u64 to represent file size) - rust-lang#120152 (add help message for `exclusive_range_pattern` error) - rust-lang#120213 (Don't actually make bound ty/const for RTN) - rust-lang#120225 (Fix -Zremap-path-scope typo) Failed merges: - rust-lang#119972 (Add `ErrCode`) r? `@ghost` `@rustbot` modify labels: rollup

cjgillot · 2024-01-27T01:43:13Z

Needs rebase.
@bors r-

As is already done in `rustc_span` and `rustc_data_structures`.

This makes no sense, and has no effect. I suspect it's been confused with a `code = "{code}"` attribute on a subdiagnostic suggestion, where it is valid (but the "code" there is suggested source code, rather than an error code.)

Error codes are integers, but `String` is used everywhere to represent them. Gross! This commit introduces `ErrCode`, an integral newtype for error codes, replacing `String`. It also introduces a constant for every error code, e.g. `E0123`, and removes the `error_code!` macro. The constants are imported wherever used with `use rustc_errors::codes::*`. With the old code, we have three different ways to specify an error code at a use point: ``` error_code!(E0123) // macro call struct_span_code_err!(dcx, span, E0123, "msg"); // bare ident arg to macro call \#[diag(name, code = "E0123")] // string struct Diag; ``` With the new code, they all use the `E0123` constant. ``` E0123 // constant struct_span_code_err!(dcx, span, E0123, "msg"); // constant \#[diag(name, code = E0123)] // constant struct Diag; ``` The commit also changes the structure of the error code definitions: - `rustc_error_codes` now just defines a higher-order macro listing the used error codes and nothing else. - Because that's now the only thing in the `rustc_error_codes` crate, I moved it into the `lib.rs` file and removed the `error_codes.rs` file. - `rustc_errors` uses that macro to define everything, e.g. the error code constants and the `DIAGNOSTIC_TABLES`. This is in its new `codes.rs` file.

nnethercote · 2024-01-28T21:16:59Z

I rebased. Raising priority because this is conflict prone.

@bors r=oli-obk p=1

bors · 2024-01-28T21:17:02Z

📌 Commit 5d9dfbd has been approved by oli-obk

It is now in the queue for this repository.

bors · 2024-01-29T00:03:56Z

⌛ Testing commit 5d9dfbd with merge 8a0b5ae...

bors · 2024-01-29T02:19:08Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 8a0b5ae to master...

rust-timer · 2024-01-29T03:35:22Z

Finished benchmarking commit (8a0b5ae): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.8%, 0.8%]	1
Regressions ❌ (secondary)	2.4%	[2.1%, 3.4%]	5
Improvements ✅ (primary)	-2.0%	[-2.6%, -1.3%]	2
Improvements ✅ (secondary)	-2.8%	[-4.5%, -1.4%]	5
All ❌✅ (primary)	-1.1%	[-2.6%, 0.8%]	3

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.8%, -2.2%]	9
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 662.476s -> 660.85s (-0.25%)
Artifact size: 308.14 MiB -> 308.02 MiB (-0.04%)

WaffleLapkin · 2024-01-29T15:46:38Z

tests/ui-fulldeps/session-diagnostic/enforce_slug_naming.rs

@@ -19,6 +19,6 @@ use rustc_errors::{Applicability, MultiSpan};
 extern crate rustc_session;

 #[derive(Diagnostic)]
-#[diag(compiletest_example, code = "E0123")]
+#[diag(compiletest_example, code = 0123)]


Shouldn't this be E0123?

Good question! I investigated, turns out it doesn't matter. This test (deliberately) fails to compile because the slug name has the incorrect form. Apparently that error occurs earlier than the type error that we would get for the code, presumably during expansion of the derive(Diagnostic) proc macro.

I can fix it in a follow-up, but it's not urgent.

I figured it did not matter much, but since I noticed thought it's better to comment :)

In rust-lang#119972 the code should have become `E0123` rather than `0123`. This fix doesn't affect the outcome because the proc macro errors out before the type of the code is checked, but the fix makes the test's code consistent with other similar code elsewhere.

Run `cargo update`. The regression comes from this PR. rust-lang/rust#119972 The error codes were refactored so the include changed.

rustbot assigned estebank Jan 14, 2024

GuillaumeGomez approved these changes Jan 14, 2024

View reviewed changes

nnethercote mentioned this pull request Jan 14, 2024

Rework how diagnostic lints are stored. #119922

Merged

Noratrieb reviewed Jan 15, 2024

View reviewed changes

compiler/rustc_hir_typeck/src/errors.rs Outdated Show resolved Hide resolved

nnethercote force-pushed the add-ErrCode branch from 689ea2c to a1da3b6 Compare January 16, 2024 02:05

This comment has been minimized.

Sign in to view

nnethercote force-pushed the add-ErrCode branch from a1da3b6 to 8842cb8 Compare January 16, 2024 03:42

nnethercote force-pushed the add-ErrCode branch from 8842cb8 to ed65026 Compare January 16, 2024 05:27

oli-obk approved these changes Jan 16, 2024

View reviewed changes

estebank approved these changes Jan 19, 2024

View reviewed changes

cjgillot reviewed Jan 20, 2024

View reviewed changes

nnethercote force-pushed the add-ErrCode branch from ed65026 to 9ddc424 Compare January 21, 2024 22:35

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 21, 2024

This was referenced Jan 22, 2024

Rollup of 10 pull requests #120235

Closed

Rollup of 9 pull requests #120239

Merged

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 27, 2024

nnethercote added 3 commits January 29, 2024 07:40

Sort attributes in compiler/rustc_errors/src/lib.rs.

d91d164

As is already done in `rustc_span` and `rustc_data_structures`.

Remove bogus {code} attributes on TraitImplMismatch.

0321de2

This makes no sense, and has no effect. I suspect it's been confused with a `code = "{code}"` attribute on a subdiagnostic suggestion, where it is valid (but the "code" there is suggested source code, rather than an error code.)

nnethercote force-pushed the add-ErrCode branch from 9ddc424 to 5d9dfbd Compare January 28, 2024 21:16

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 28, 2024

nnethercote mentioned this pull request Jan 28, 2024

Miri subtree update #120451

Merged

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 29, 2024

bors merged commit 8a0b5ae into rust-lang:master Jan 29, 2024
12 checks passed

rustbot added this to the 1.77.0 milestone Jan 29, 2024

nnethercote deleted the add-ErrCode branch January 29, 2024 02:20

WaffleLapkin reviewed Jan 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `ErrCode` #119972

Add `ErrCode` #119972

nnethercote commented Jan 14, 2024 •

edited

Loading

rustbot commented Jan 14, 2024

nnethercote commented Jan 14, 2024

GuillaumeGomez left a comment

nnethercote commented Jan 16, 2024 •

edited

Loading

This comment has been minimized.

nnethercote commented Jan 16, 2024

nnethercote commented Jan 16, 2024

oli-obk left a comment

oli-obk Jan 16, 2024

nnethercote Jan 16, 2024

oli-obk Jan 16, 2024

nnethercote Jan 16, 2024

bors commented Jan 17, 2024

oli-obk commented Jan 17, 2024

cjgillot Jan 20, 2024

nnethercote Jan 21, 2024

nnethercote commented Jan 21, 2024

bors commented Jan 21, 2024

cjgillot commented Jan 27, 2024

nnethercote commented Jan 28, 2024

bors commented Jan 28, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

rust-timer commented Jan 29, 2024

WaffleLapkin Jan 29, 2024

nnethercote Jan 29, 2024 •

edited

Loading

WaffleLapkin Jan 29, 2024

Add ErrCode #119972

Add ErrCode #119972

Conversation

nnethercote commented Jan 14, 2024 • edited Loading

rustbot commented Jan 14, 2024

nnethercote commented Jan 14, 2024

GuillaumeGomez left a comment

Choose a reason for hiding this comment

nnethercote commented Jan 16, 2024 • edited Loading

This comment has been minimized.

nnethercote commented Jan 16, 2024

nnethercote commented Jan 16, 2024

oli-obk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Jan 17, 2024

oli-obk commented Jan 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nnethercote commented Jan 21, 2024

bors commented Jan 21, 2024

cjgillot commented Jan 27, 2024

nnethercote commented Jan 28, 2024

bors commented Jan 28, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

rust-timer commented Jan 29, 2024

Overall result: no relevant changes - no action needed

Choose a reason for hiding this comment

nnethercote Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add `ErrCode` #119972

Add `ErrCode` #119972

nnethercote commented Jan 14, 2024 •

edited

Loading

nnethercote commented Jan 16, 2024 •

edited

Loading

nnethercote Jan 29, 2024 •

edited

Loading