Overhaul `-Ztreat-err-as-bug` #119871

nnethercote · 2024-01-12T00:02:44Z

It's current behaviour is surprising, in a bad way. This also makes the implementation more complex than it needs to be.

r? @oli-obk

nnethercote · 2024-01-12T00:05:53Z

Let me explain in some detail. Consider this program with a single type error.

fn main() { let a: u32 = true; }

Compile it normally and you get this:

error[E0308]: mismatched types
 --> a.rs:1:26
  |
1 | fn main() { let a: u32 = true; }
  |                    ---   ^^^^ expected `u32`, found `bool`
  |                    |
  |                    expected due to this

Ok, that's fine.

Now compile it with -Ztreat-err-as-bug=1 and you get this:

error: internal compiler error: `TypeError` when attempting coercion but no error emitted
 --> a.rs:1:26
  |
1 | fn main() { let a: u32 = true; }
  |                          ^^^^

thread 'rustc' panicked at compiler/rustc_errors/src/lib.rs:1467:30:
aborting due to `-Z treat-err-as-bug=1`

There is a type error, and the compiler panics afterwards. But the error is labelled as an ICE, there is no error code, the error message is different, and it is missing the "expected" markers. Huh?

Now compile it with -Ztreat-err-as-bug=2 and you get this:

error[E0308]: mismatched types
 --> a.rs:1:26
  |
1 | fn main() { let a: u32 = true; }
  |                    ---   ^^^^ expected `u32`, found `bool`
  |                    |
  |                    expected due to this

thread 'rustc' panicked at compiler/rustc_errors/src/lib.rs:1471:25:
aborting after 1 errors and 1 delayed bugs due to `-Z treat-err-as-bug=2`

The type error is back to the original: it's a normal error instead of an ICE, the error message matches, the error code is there, and the "expected" markers are there. And again it panics afterwards. This is the behaviour I would expect with -Ztreat-err-as-bug. What is going on?

The final line of the last example explains it: "aborting after 1 errors and 1 delayed bugs". The compiler has "delayed bugs": a kind of second-class diagnostic that only gets printed at the end if no other errors are printed. They're widely used but mostly invisible; you never see them unless there's a bug in the compiler. And yet, -Ztreat-err-as-bug treats delayed bugs the same as normal errors.

So now we can now understand the -Ztreat-err-as-bug=1 output. When handling this type error, the compiler first emits a delayed bug, and then emits a normal error. It's showing the delayed bug, which has a different message, less span information, and happens to be treated as an ICE rather than a normal error followed by a panic.

I think this behaviour is very confusing and not what anybody is likely to want in practice. This PR changes -Ztreat-err-as-bug so it ignores delayed bugs, unless they get promoted to proper bugs and are printed.

cc @compiler-errors

compiler-errors · 2024-01-12T00:08:46Z

This is somewhat disappointing since it doesn't (afaict) give you the capability to intercept a delayed bug anymore. I may add a -Zemit-delayed-bug-as-err to pair with this, for compiler debugging purposes.

edit: see convo in #119872, as long as we can land that, I'm happy with this.

nnethercote · 2024-01-12T04:34:50Z

If I wait for #119872 to merge, then I can use -Zeagerly-emit-delayed-bugs to undo the storage-return.rs test changes, which is nice.

tests/ui/mir/lint/storage-return.rs

…ed-bugs, r=oli-obk,nnethercote Give me a way to emit all the delayed bugs as errors (add `-Zeagerly-emit-delayed-bugs`) This is probably a *better* way to inspect all the delayed bugs in a program that what exists currently (and therefore makes it very easy to choose the right number `N` with `-Zemit-err-as-bug=N`, though I guess the naming is a bit ironic when you pair both of the flags together, but that feels like naming bikeshed more than anything). This pacifies my only concern with rust-lang#119871 (comment), because (afaict?) that PR doesn't allow you to intercept a delayed bug's stack trace anymore, which as someone who debugs the compiler a lot, is something that I can *promise* that I do. r? `@nnethercote` or `@oli-obk`

Rollup merge of rust-lang#119872 - compiler-errors:eagerly-emit-delayed-bugs, r=oli-obk,nnethercote Give me a way to emit all the delayed bugs as errors (add `-Zeagerly-emit-delayed-bugs`) This is probably a *better* way to inspect all the delayed bugs in a program that what exists currently (and therefore makes it very easy to choose the right number `N` with `-Zemit-err-as-bug=N`, though I guess the naming is a bit ironic when you pair both of the flags together, but that feels like naming bikeshed more than anything). This pacifies my only concern with rust-lang#119871 (comment), because (afaict?) that PR doesn't allow you to intercept a delayed bug's stack trace anymore, which as someone who debugs the compiler a lot, is something that I can *promise* that I do. r? `@nnethercote` or `@oli-obk`

bors · 2024-01-12T20:51:06Z

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

`-Ztreat-err-as-bug` treats normal errors and delayed bugs equally, which can lead to some really surprising results. This commit changes `-Ztreat-err-as-bug` so it ignores delayed bugs, unless they get promoted to proper bugs and are printed. This feels to me much simpler and more logical. And it simplifies the implementation: - The `-Ztreat-err-as-bug` check is removed from in `DiagCtxt::{delayed_bug,span_delayed_bug}`. - `treat_err_as_bug` doesn't need to count delayed bugs. - The `-Ztreat-err-as-bug` panic message is simpler, because it doesn't have to mention delayed bugs. Output of delayed bugs is now more consistent. They're always printed the same way. Previously when they triggered `-Ztreat-err-as-bug` they would be printed slightly differently, via `span_bug` in `span_delayed_bug` or `delayed_bug`. A minor behaviour change: the "no errors encountered even though `span_delayed_bug` issued" printed before delayed bugs is now a note rather than a bug. This is done so it doesn't get counted as an error that might trigger `-Ztreat-err-as-bug`, which would be silly. This means that if you use `-Ztreat-err-as-bug=1` and there are no normal errors but there are delayed bugs, the first delayed bug will be shown (and the panic will happen after it's printed). Also, I have added a second note saying "those delayed bugs will now be shown as internal compiler errors". I think this makes it clearer what is happening, because the whole concept of delayed bugs is non-obvious. There are some test changes. - equality-in-canonical-query.rs: Minor output changes, and the error count reduces by one because the "no errors encountered even though `span_delayed_bug` issued" message is no longer counted as an error. - rpit_tait_equality_in_canonical_query.rs: Ditto. - storage-live.rs: The query stack disappears because these delayed bugs are now printed at the end, rather than when they are created. - storage-return.rs, span_delayed_bug.rs: now need `-Zeagerly-emit-delayed-bugs` because they need the delayed bugs emitted immediately to preserve behaviour.

nnethercote · 2024-01-12T23:21:08Z

I updated and adjusted some of the test changes back by using the new -Zeagerly-emit-delayed-bugs. Should be good to go now.

compiler-errors

r=me when green

nnethercote · 2024-01-13T02:21:56Z

It's green.

@bors r=compiler-errors

bors · 2024-01-13T02:21:58Z

📌 Commit f1ac541 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2024-01-13T08:15:55Z

⌛ Testing commit f1ac541 with merge 284cb71...

bors · 2024-01-13T10:13:16Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 284cb71 to master...

rust-timer · 2024-01-13T11:27:49Z

Finished benchmarking commit (284cb71): 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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.4%	[-2.4%, -2.4%]	1
Improvements ✅ (secondary)	-2.3%	[-2.3%, -2.3%]	1
All ❌✅ (primary)	-2.4%	[-2.4%, -2.4%]	1

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)	2.0%	[1.0%, 2.5%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 668.237s -> 669.206s (0.15%)
Artifact size: 308.17 MiB -> 308.17 MiB (0.00%)

rustbot assigned oli-obk Jan 12, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 12, 2024

This comment has been minimized.

Sign in to view

compiler-errors mentioned this pull request Jan 12, 2024

Give me a way to emit all the delayed bugs as errors (add -Zeagerly-emit-delayed-bugs) #119872

Merged

nnethercote force-pushed the overhaul-treat-err-as-bug branch from 40f206e to 0bfd2f9 Compare January 12, 2024 03:08

tmiasko reviewed Jan 12, 2024

View reviewed changes

tests/ui/mir/lint/storage-return.rs Outdated Show resolved Hide resolved

compiler-errors assigned compiler-errors and unassigned oli-obk Jan 12, 2024

nnethercote force-pushed the overhaul-treat-err-as-bug branch from 0bfd2f9 to f1ac541 Compare January 12, 2024 23:02

compiler-errors approved these changes Jan 12, 2024

View reviewed changes

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 13, 2024

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

bors merged commit 284cb71 into rust-lang:master Jan 13, 2024
12 checks passed

rustbot added this to the 1.77.0 milestone Jan 13, 2024

nnethercote deleted the overhaul-treat-err-as-bug branch January 13, 2024 22:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Overhaul `-Ztreat-err-as-bug` #119871

Overhaul `-Ztreat-err-as-bug` #119871

nnethercote commented Jan 12, 2024

nnethercote commented Jan 12, 2024

compiler-errors commented Jan 12, 2024 •

edited

Loading

This comment has been minimized.

nnethercote commented Jan 12, 2024

bors commented Jan 12, 2024

nnethercote commented Jan 12, 2024

compiler-errors left a comment

nnethercote commented Jan 13, 2024

bors commented Jan 13, 2024

bors commented Jan 13, 2024

bors commented Jan 13, 2024

rust-timer commented Jan 13, 2024

Overhaul -Ztreat-err-as-bug #119871

Overhaul -Ztreat-err-as-bug #119871

Conversation

nnethercote commented Jan 12, 2024

nnethercote commented Jan 12, 2024

compiler-errors commented Jan 12, 2024 • edited Loading

This comment has been minimized.

nnethercote commented Jan 12, 2024

bors commented Jan 12, 2024

nnethercote commented Jan 12, 2024

compiler-errors left a comment

Choose a reason for hiding this comment

nnethercote commented Jan 13, 2024

bors commented Jan 13, 2024

bors commented Jan 13, 2024

bors commented Jan 13, 2024

rust-timer commented Jan 13, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Overhaul `-Ztreat-err-as-bug` #119871

Overhaul `-Ztreat-err-as-bug` #119871

compiler-errors commented Jan 12, 2024 •

edited

Loading