Some cleanups around diagnostic levels. #120520

nnethercote · 2024-01-31T05:30:18Z

Plus some refactoring in and around diagnostic levels and emission. Details in the individual commit logs.

r? @oli-obk

rustbot · 2024-01-31T05:30:28Z

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @TaKO8Ki

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

RalfJung · 2024-01-31T06:41:25Z

I find "weak bug" to still be quite confusing as a name. This is after all called in places where there is no bug - just some expensive computation that we should only do when the user actually sees the result. I would suggest a name that involves "require diagnostic" or "assert diagnostic" or so. That more directly expresses what is actually being checked.

nnethercote · 2024-01-31T07:30:53Z

I used "weak" because it's a weaker form of DelayedBug, because it doesn't provide an ErrorGuarantee. And DelayedBug seems like well-established terminology, so I didn't want to go with anything that required changing the names of both levels.

Also, I really want to eliminate WeakDelayedBug, because it kinda sucks, and only has two uses.

RalfJung · 2024-01-31T08:10:12Z

Yeah I saw that argument in your commit message. I don't agree though, this does not at all feel like a "weak version of delay_bug". delay_bug is used in cases where one would like to use bug!, but one cannot since rustc continues running for a while on invalid programs, and later phases of the compiler sometimes need to deal with clearly invalid state and want to make sure this never happens in a session where we emit any artifacts.

"good_path_bug" is nothing at all like that, so making the name similar is misleading IMO.

nnethercote · 2024-01-31T08:49:20Z

I interpret the bug in both delayed_bug and weak_delayed_bug as meaning "causes an ICE (bug) if it triggers", and the delayed as "triggers only at the end of execution, if at all". So to me they are very similar, the difference being the condition that must be met to avoid the trigger, and the EmissionGuarantee you get as a result.

RalfJung · 2024-01-31T09:19:52Z

For delay_span_bug, it makes sense to think of this as "if this line is ever reached, there was a bug" (with an asterisk about rustc continuing compilation for a bit even on blatantly invalid programs).

For good-path-bug, I don't think it makes sense to think of this as "if this line is ever reached, there was a bug". So the naming in this PR creates a false analogy.

If we look at how these two are used, I don't think they are as similar as you claim. They are just similar if you look at their implementation, but the API should be designed to cater its users, not its implementers.

nnethercote · 2024-01-31T11:18:14Z

For delay_span_bug, it makes sense to think of this as "if this line is ever reached, there was a bug" (with an asterisk about rustc continuing compilation for a bit even on blatantly invalid programs).

Are you using "bug" the same way I am, as in "bug in rustc"?

If you are, then "if this line is ever reached, there was a bug" is wrong, because span_delayed_bug gets called all the time when compiling erroneous programs, without rustc being buggy.

If the statement is instead "if this line is ever reached, the program being compiled is erroneous" then that makes more sense.

oli-obk · 2024-01-31T20:07:37Z

r=me on everything but the last commit.

Also, I really want to eliminate WeakDelayedBug, because it kinda sucks, and only has two uses.

let's just work on that without renaming it first

nnethercote · 2024-01-31T21:08:19Z

I didn't do a good job of separating out distinct changes, and the last commit has a couple of different things happening. I will revert the parts of it that change "good path" to "weak".

RalfJung · 2024-01-31T21:38:42Z

Maybe we should have a t-compiler MCP for the name, to get input from more people.

nnethercote · 2024-01-31T22:07:03Z

Maybe we should have a t-compiler MCP for the name, to get input from more people.

No thanks. I won't invite 15 people to weigh in via a process with a 10 day final comment period, just to get permission to rename something that has only ~30 occurrences in the compiler and that I want to remove anyway.

@oli-obk: I have updated the final commit to remove the "good path"-to-"weak" renaming parts. Should be good to go now, I think.

oli-obk · 2024-01-31T22:23:29Z

@bors r+

bors · 2024-01-31T22:23:32Z

📌 Commit 23510b2 has been approved by oli-obk

It is now in the queue for this repository.

bors · 2024-01-31T22:23:32Z

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

…i-obk Some cleanups around diagnostic levels. Plus some refactoring in and around diagnostic levels and emission. Details in the individual commit logs. r? `@oli-obk`

…i-obk Some cleanups around diagnostic levels. Plus some refactoring in and around diagnostic levels and emission. Details in the individual commit logs. r? ``@oli-obk``

The two kinds of delayed bug have quite different semantics so a stronger conceptual separation is nice. (`is_error` is a good example, because the two kinds have different behaviour.) The commit also moves the `DelayedBug` variant after `Error` in `Level`, to reflect the fact that it's weaker than `Error` -- it might trigger an error but also might not. (The pre-existing `downgrade_to_delayed_bug` function also reflects the notion that delayed bugs are lower/after normal errors.) Plus it condenses some of the comments on `Level` into a table, for easier reading, and introduces `can_be_top_or_sub` to indicate which levels can be used in top-level diagnostics vs. subdiagnostics. Finally, it renames `DiagCtxtInner::span_delayed_bugs` as `DiagCtxtInner::delayed_bugs`. The `span_` prefix is unnecessary because some delayed bugs don't have a span.

nnethercote · 2024-02-04T23:12:51Z

I suspect this caused problems here: #120561 (comment)

The commit involving suppressed_expected_diag was causing an ICE when compiling https://github.com/iron/iron. I have removed that commit and I'll redo it in a follow-up. I also fixed the conflicts.

@bors r=oli-obk

bors · 2024-02-04T23:12:53Z

📌 Commit 59e0bc2 has been approved by oli-obk

It is now in the queue for this repository.

…i-obk Some cleanups around diagnostic levels. Plus some refactoring in and around diagnostic levels and emission. Details in the individual commit logs. r? `@oli-obk`

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#120396 (Account for unbounded type param receiver in suggestions) - rust-lang#120423 (update indirect structural match lints to match RFC and to show up for dependencies) - rust-lang#120435 (Suggest name value cfg when only value is used for check-cfg) - rust-lang#120507 (Account for non-overlapping unmet trait bounds in suggestion) - rust-lang#120520 (Some cleanups around diagnostic levels.) - rust-lang#120521 (Make `NonZero` constructors generic.) - rust-lang#120527 (Switch OwnedStore handle count to AtomicU32) - rust-lang#120550 (Continue to borrowck even if there were previous errors) - rust-lang#120575 (Simplify codegen diagnostic handling) r? `@ghost` `@rustbot` modify labels: rollup

matthiaskrgr · 2024-02-05T08:40:27Z

@bors r- may need rebase
#120662 (comment)

matthiaskrgr · 2024-02-05T09:20:27Z

looks like this was not the problem 🎉
@bors r=oli-obk

bors · 2024-02-05T09:20:30Z

📌 Commit 59e0bc2 has been approved by oli-obk

It is now in the queue for this repository.

…i-obk Some cleanups around diagnostic levels. Plus some refactoring in and around diagnostic levels and emission. Details in the individual commit logs. r? `@oli-obk`

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#120023 (tidy: reduce allocs) - rust-lang#120396 (Account for unbounded type param receiver in suggestions) - rust-lang#120435 (Suggest name value cfg when only value is used for check-cfg) - rust-lang#120507 (Account for non-overlapping unmet trait bounds in suggestion) - rust-lang#120520 (Some cleanups around diagnostic levels.) - rust-lang#120575 (Simplify codegen diagnostic handling) - rust-lang#120670 (cleanup effect var handling) Failed merges: - rust-lang#120423 (update indirect structural match lints to match RFC and to show up for dependencies) r? `@ghost` `@rustbot` modify labels: rollup

…i-obk Some cleanups around diagnostic levels. Plus some refactoring in and around diagnostic levels and emission. Details in the individual commit logs. r? ``@oli-obk``

…i-obk Some cleanups around diagnostic levels. Plus some refactoring in and around diagnostic levels and emission. Details in the individual commit logs. r? ```@oli-obk```

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#120302 (various const interning cleanups) - rust-lang#120520 (Some cleanups around diagnostic levels.) - rust-lang#120521 (Make `NonZero` constructors generic.) - rust-lang#120527 (Switch OwnedStore handle count to AtomicU32) - rust-lang#120564 (coverage: Split out counter increment sites from BCB node/edge counters) - rust-lang#120575 (Simplify codegen diagnostic handling) - rust-lang#120597 (Suggest `[tail @ ..]` on `[..tail]` and `[...tail]` where `tail` is unresolved) - rust-lang#120609 (hir: Stop keeping prefixes for most of `use` list stems) - rust-lang#120633 (pattern_analysis: gather up place-relevant info) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#120520 (Some cleanups around diagnostic levels.) - rust-lang#120575 (Simplify codegen diagnostic handling) - rust-lang#120597 (Suggest `[tail @ ..]` on `[..tail]` and `[...tail]` where `tail` is unresolved) - rust-lang#120602 (rustc_monomorphize: fix outdated comment in partition) - rust-lang#120609 (hir: Stop keeping prefixes for most of `use` list stems) - rust-lang#120631 (Emit a diagnostic for invalid target options) - rust-lang#120632 (For E0223, suggest associated functions that are similar to the path) - rust-lang#120670 (cleanup effect var handling) - rust-lang#120673 (rustc_metadata: fix typo) - rust-lang#120683 (miri: fix ICE with symbolic alignment check on extern static) - rust-lang#120690 (Remove b-naber from the compiler review rotation) - rust-lang#120713 (Make async closures test use async bound modifier) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#120520 - nnethercote:rename-good-path, r=oli-obk Some cleanups around diagnostic levels. Plus some refactoring in and around diagnostic levels and emission. Details in the individual commit logs. r? ````@oli-obk````

rustbot assigned petrochenkov Jan 31, 2024

rustbot assigned oli-obk and unassigned petrochenkov Jan 31, 2024

rust-lang deleted a comment from rustbot Jan 31, 2024

This comment has been minimized.

Sign in to view

nnethercote force-pushed the rename-good-path branch from 56cd3d2 to 32803e1 Compare January 31, 2024 05:54

nnethercote force-pushed the rename-good-path branch from 32803e1 to 23510b2 Compare January 31, 2024 22:02

nnethercote changed the title ~~Rename "good path" delayed bugs as "weak" delayed bugs.~~ Some cleanups around diagnostic levels. Jan 31, 2024

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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 31, 2024

matthiaskrgr mentioned this pull request Feb 1, 2024

Rollup of 9 pull requests #120560

Closed

nnethercote force-pushed the rename-good-path branch from 23510b2 to 59e0bc2 Compare February 4, 2024 23:10

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 Feb 4, 2024

matthiaskrgr mentioned this pull request Feb 5, 2024

Rollup of 9 pull requests #120662

Closed

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 Feb 5, 2024

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 Feb 5, 2024

matthiaskrgr mentioned this pull request Feb 5, 2024

Rollup of 7 pull requests #120679

Closed

matthiaskrgr mentioned this pull request Feb 6, 2024

Rollup of 10 pull requests #120710

Closed

matthiaskrgr mentioned this pull request Feb 6, 2024

Rollup of 9 pull requests #120711

Closed

matthiaskrgr mentioned this pull request Feb 6, 2024

Rollup of 12 pull requests #120715

Merged

bors merged commit 5587be8 into rust-lang:master Feb 6, 2024
11 checks passed

rustbot added this to the 1.78.0 milestone Feb 6, 2024

nnethercote deleted the rename-good-path branch February 6, 2024 22:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Some cleanups around diagnostic levels. #120520

Some cleanups around diagnostic levels. #120520

nnethercote commented Jan 31, 2024 •

edited

Loading

rustbot commented Jan 31, 2024

This comment has been minimized.

RalfJung commented Jan 31, 2024 via email

nnethercote commented Jan 31, 2024

RalfJung commented Jan 31, 2024

nnethercote commented Jan 31, 2024

RalfJung commented Jan 31, 2024 •

edited

Loading

nnethercote commented Jan 31, 2024

oli-obk commented Jan 31, 2024

nnethercote commented Jan 31, 2024

RalfJung commented Jan 31, 2024

nnethercote commented Jan 31, 2024

oli-obk commented Jan 31, 2024

bors commented Jan 31, 2024

bors commented Jan 31, 2024

nnethercote commented Feb 4, 2024

bors commented Feb 4, 2024

matthiaskrgr commented Feb 5, 2024

matthiaskrgr commented Feb 5, 2024

bors commented Feb 5, 2024

Some cleanups around diagnostic levels. #120520

Some cleanups around diagnostic levels. #120520

Conversation

nnethercote commented Jan 31, 2024 • edited Loading

rustbot commented Jan 31, 2024

This comment has been minimized.

RalfJung commented Jan 31, 2024 via email

nnethercote commented Jan 31, 2024

RalfJung commented Jan 31, 2024

nnethercote commented Jan 31, 2024

RalfJung commented Jan 31, 2024 • edited Loading

nnethercote commented Jan 31, 2024

oli-obk commented Jan 31, 2024

nnethercote commented Jan 31, 2024

RalfJung commented Jan 31, 2024

nnethercote commented Jan 31, 2024

oli-obk commented Jan 31, 2024

bors commented Jan 31, 2024

bors commented Jan 31, 2024

nnethercote commented Feb 4, 2024

bors commented Feb 4, 2024

matthiaskrgr commented Feb 5, 2024

matthiaskrgr commented Feb 5, 2024

bors commented Feb 5, 2024

nnethercote commented Jan 31, 2024 •

edited

Loading

RalfJung commented Jan 31, 2024 •

edited

Loading