Skip to content

Commit

Permalink
Don't consider delayed bugs for -Ztreat-err-as-bug.
Browse files Browse the repository at this point in the history
`-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.
- tests/ui/mir/lint/*.rs: These tests involved delayed bugs that get
  shown. The query stack disappears because these delayed bugs are now
  printed at the end, rather than when they are created.
- storage-return.rs: This is the only test with a big change. It
  triggers a delayed bug and then a normal error. It used to use
  `-Ztreat-err-as-bug=1` to abort on the delayed bug. That is no longer
  possible, and the delayed bug doesn't show up because of the normal
  error. So I changed the test to use the normal error. The normal error
  does seem related to the delayed bug, so hopefully it's good enough.
  • Loading branch information
nnethercote committed Jan 12, 2024
1 parent 3330940 commit 0bfd2f9
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 69 deletions.
54 changes: 17 additions & 37 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -861,10 +861,6 @@ impl DiagCtxt {
/// directly).
#[track_caller]
pub fn delayed_bug(&self, msg: impl Into<DiagnosticMessage>) -> ErrorGuaranteed {
let treat_next_err_as_bug = self.inner.borrow().treat_next_err_as_bug();
if treat_next_err_as_bug {
self.bug(msg);
}
DiagnosticBuilder::<ErrorGuaranteed>::new(self, DelayedBug, msg).emit()
}

Expand All @@ -878,10 +874,6 @@ impl DiagCtxt {
sp: impl Into<MultiSpan>,
msg: impl Into<DiagnosticMessage>,
) -> ErrorGuaranteed {
let treat_next_err_as_bug = self.inner.borrow().treat_next_err_as_bug();
if treat_next_err_as_bug {
self.span_bug(sp, msg);
}
DiagnosticBuilder::<ErrorGuaranteed>::new(self, DelayedBug, msg).with_span(sp).emit()
}

Expand Down Expand Up @@ -1370,20 +1362,14 @@ impl DiagCtxtInner {
}

fn treat_err_as_bug(&self) -> bool {
self.flags.treat_err_as_bug.is_some_and(|c| {
self.err_count + self.lint_err_count + self.delayed_bug_count() >= c.get()
})
self.flags.treat_err_as_bug.is_some_and(|c| self.err_count + self.lint_err_count >= c.get())
}

// Use this one before incrementing `err_count`.
fn treat_next_err_as_bug(&self) -> bool {
self.flags.treat_err_as_bug.is_some_and(|c| {
self.err_count + self.lint_err_count + self.delayed_bug_count() + 1 >= c.get()
})
}

fn delayed_bug_count(&self) -> usize {
self.span_delayed_bugs.len() + self.good_path_delayed_bugs.len()
self.flags
.treat_err_as_bug
.is_some_and(|c| self.err_count + self.lint_err_count + 1 >= c.get())
}

fn has_errors(&self) -> bool {
Expand All @@ -1395,7 +1381,7 @@ impl DiagCtxtInner {
}

fn flush_delayed(&mut self, kind: DelayedBugKind) {
let (bugs, explanation) = match kind {
let (bugs, note1) = match kind {
DelayedBugKind::Normal => (
std::mem::take(&mut self.span_delayed_bugs),
"no errors encountered even though `span_delayed_bug` issued",
Expand All @@ -1405,6 +1391,7 @@ impl DiagCtxtInner {
"no warnings or errors encountered even though `good_path_delayed_bugs` issued",
),
};
let note2 = "those delayed bugs will now be shown as internal compiler errors";

if bugs.is_empty() {
return;
Expand All @@ -1430,8 +1417,11 @@ impl DiagCtxtInner {

if i == 0 {
// Put the overall explanation before the `DelayedBug`s, to
// frame them better (e.g. separate warnings from them).
self.emit_diagnostic(Diagnostic::new(Bug, explanation));
// frame them better (e.g. separate warnings from them). Also,
// make it a note so it doesn't count as an error, because that
// could trigger `-Ztreat-err-as-bug`, which we don't want.
self.emit_diagnostic(Diagnostic::new(Note, note1));
self.emit_diagnostic(Diagnostic::new(Note, note2));
}

let mut bug =
Expand All @@ -1457,22 +1447,12 @@ impl DiagCtxtInner {

fn panic_if_treat_err_as_bug(&self) {
if self.treat_err_as_bug() {
match (
self.err_count + self.lint_err_count,
self.delayed_bug_count(),
self.flags.treat_err_as_bug.map(|c| c.get()).unwrap(),
) {
(1, 0, 1) => panic!("aborting due to `-Z treat-err-as-bug=1`"),
(0, 1, 1) => panic!("aborting due delayed bug with `-Z treat-err-as-bug=1`"),
(count, delayed_count, val) => {
if delayed_count > 0 {
panic!(
"aborting after {count} errors and {delayed_count} delayed bugs due to `-Z treat-err-as-bug={val}`",
)
} else {
panic!("aborting after {count} errors due to `-Z treat-err-as-bug={val}`")
}
}
let n = self.flags.treat_err_as_bug.map(|c| c.get()).unwrap();
assert_eq!(n, self.err_count + self.lint_err_count);
if n == 1 {
panic!("aborting due to `-Z treat-err-as-bug=1`");
} else {
panic!("aborting after {n} errors due to `-Z treat-err-as-bug={n}`");
}
}
}
Expand Down
16 changes: 6 additions & 10 deletions tests/ui/impl-trait/equality-in-canonical-query.clone.stderr
Original file line number Diff line number Diff line change
@@ -1,29 +1,25 @@
error: internal compiler error: no errors encountered even though `span_delayed_bug` issued
note: no errors encountered even though `span_delayed_bug` issued

note: those delayed bugs will now be shown as internal compiler errors

error: internal compiler error: {OpaqueTypeKey { def_id: DefId(rpit::{opaque#0}), args: [] }: OpaqueTypeDecl { hidden_type: OpaqueHiddenType { span: no-location (#0), ty: Alias(Opaque, AliasTy { args: [], def_id: DefId(foo::{opaque#0}) }) } }}
|
=


error: internal compiler error: error performing ParamEnvAnd { param_env: ParamEnv { caller_bounds: [], reveal: UserFacing }, value: ProvePredicate { predicate: Binder { value: ProjectionPredicate(AliasTy { args: [FnDef(DefId(rpit), []), ()], def_id: DefId(ops::function::FnOnce::Output) }, Term::Ty(Alias(Opaque, AliasTy { args: [], def_id: DefId(foo::{opaque#0}) }))), bound_vars: [] } } }
--> $DIR/equality-in-canonical-query.rs:19:5
--> $DIR/equality-in-canonical-query.rs:21:5
|
LL | same_output(foo, rpit);
| ^^^^^^^^^^^^^^^^^^^^^^
|

--> $DIR/equality-in-canonical-query.rs:19:5
--> $DIR/equality-in-canonical-query.rs:21:5
|
LL | same_output(foo, rpit);
| ^^^^^^^^^^^^^^^^^^^^^^







query stack during panic:
end of query stack
error: aborting due to 3 previous errors
error: aborting due to 2 previous errors

6 changes: 4 additions & 2 deletions tests/ui/impl-trait/equality-in-canonical-query.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// issue: #116877
// revisions: sized clone
//[sized] check-pass

//[clone] known-bug: #108498
//[clone] failure-status: 101
//[clone] normalize-stderr-test: "DefId\(.*?\]::" -> "DefId("
//[clone] normalize-stderr-test: "(?m)note: .*$" -> ""
//[clone] normalize-stderr-test: "(?m)note: we would appreciate a bug report.*\n\n" -> ""
//[clone] normalize-stderr-test: "(?m)note: rustc.*running on.*\n\n" -> ""
//[clone] normalize-stderr-test: "(?m)note: compiler flags.*\n\n" -> ""
//[clone] normalize-stderr-test: "(?m)note: delayed at.*$" -> ""
//[clone] normalize-stderr-test: "(?m)^ *\d+: .*\n" -> ""
//[clone] normalize-stderr-test: "(?m)^ *at .*\n" -> ""

Expand Down
8 changes: 6 additions & 2 deletions tests/ui/mir/lint/storage-live.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ error: internal compiler error: broken MIR in Item(DefId(0:8 ~ storage_live[HASH
StorageLive(_1) which already has storage here
--> $DIR/storage-live.rs:22:13
|
LL | StorageLive(a);
| ^^^^^^^^^^^^^^
|
note: delayed at compiler/rustc_mir_transform/src/lint.rs:97:26 - disabled backtrace
--> $DIR/storage-live.rs:22:13
|
LL | StorageLive(a);
| ^^^^^^^^^^^^^^

aborting due to `-Z treat-err-as-bug=1`
error: the compiler unexpectedly panicked. this is a bug.

query stack during panic:
#0 [mir_const] preparing `multiple_storage` for borrow checking
#1 [mir_promoted] promoting constants in MIR for `multiple_storage`
end of query stack
6 changes: 1 addition & 5 deletions tests/ui/mir/lint/storage-return.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
// compile-flags: -Zlint-mir -Ztreat-err-as-bug
// failure-status: 101
// dont-check-compiler-stderr
// error-pattern: has storage when returning
#![feature(custom_mir, core_intrinsics)]
extern crate core;
use core::intrinsics::mir::*;
Expand All @@ -12,7 +8,7 @@ fn main() {
let a: ();
{
StorageLive(a);
RET = a;
RET = a; //~ ERROR used binding isn't initialized
Return()
}
)
Expand Down
22 changes: 22 additions & 0 deletions tests/ui/mir/lint/storage-return.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0381]: used binding isn't initialized
--> $DIR/storage-return.rs:11:13
|
LL | / mir!(
LL | | let a: ();
LL | | {
LL | | StorageLive(a);
LL | | RET = a;
| | ^^^^^^^ value used here but it isn't initialized
LL | | Return()
LL | | }
LL | | )
| |_____- binding declared here but left uninitialized
|
help: consider assigning a value
|
LL | let a: () = todo!();
| +++++++++

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0381`.
2 changes: 1 addition & 1 deletion tests/ui/treat-err-as-bug/span_delayed_bug.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// compile-flags: -Ztreat-err-as-bug
// failure-status: 101
// error-pattern: aborting due to `-Z treat-err-as-bug=1`
// error-pattern: [trigger_span_delayed_bug] triggering a span delayed bug for testing incremental
// error-pattern: delayed span bug triggered by #[rustc_error(span_delayed_bug_from_inside_query)]
// normalize-stderr-test "note: .*\n\n" -> ""
// normalize-stderr-test "thread 'rustc' panicked.*:\n.*\n" -> ""
// rustc-env:RUST_BACKTRACE=0
Expand Down
7 changes: 6 additions & 1 deletion tests/ui/treat-err-as-bug/span_delayed_bug.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
error: internal compiler error: delayed span bug triggered by #[rustc_error(span_delayed_bug_from_inside_query)]
--> $DIR/span_delayed_bug.rs:12:1
|
LL | fn main() {}
| ^^^^^^^^^
|
note: delayed at compiler/rustc_middle/src/util/bug.rs:45:15 - disabled backtrace
--> $DIR/span_delayed_bug.rs:12:1
|
LL | fn main() {}
| ^^^^^^^^^

error: the compiler unexpectedly panicked. this is a bug.

query stack during panic:
#0 [trigger_span_delayed_bug] triggering a span delayed bug for testing incremental
end of query stack
Original file line number Diff line number Diff line change
@@ -1,29 +1,25 @@
error: internal compiler error: no errors encountered even though `span_delayed_bug` issued
note: no errors encountered even though `span_delayed_bug` issued

note: those delayed bugs will now be shown as internal compiler errors

error: internal compiler error: {OpaqueTypeKey { def_id: DefId(get_rpit::{opaque#0}), args: [] }: OpaqueTypeDecl { hidden_type: OpaqueHiddenType { span: no-location (#0), ty: Alias(Opaque, AliasTy { args: [], def_id: DefId(Opaque::{opaque#0}) }) } }}
|
=


error: internal compiler error: error performing ParamEnvAnd { param_env: ParamEnv { caller_bounds: [], reveal: UserFacing }, value: ProvePredicate { predicate: Binder { value: ProjectionPredicate(AliasTy { args: [FnDef(DefId(get_rpit), []), ()], def_id: DefId(ops::function::FnOnce::Output) }, Term::Ty(Alias(Opaque, AliasTy { args: [], def_id: DefId(Opaque::{opaque#0}) }))), bound_vars: [] } } }
--> $DIR/rpit_tait_equality_in_canonical_query.rs:28:5
--> $DIR/rpit_tait_equality_in_canonical_query.rs:31:5
|
LL | query(get_rpit);
| ^^^^^^^^^^^^^^^
|

--> $DIR/rpit_tait_equality_in_canonical_query.rs:28:5
--> $DIR/rpit_tait_equality_in_canonical_query.rs:31:5
|
LL | query(get_rpit);
| ^^^^^^^^^^^^^^^







query stack during panic:
end of query stack
error: aborting due to 3 previous errors
error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
//[current] known-bug: #108498
//[current] failure-status: 101
//[current] normalize-stderr-test: "DefId\(.*?\]::" -> "DefId("
//[current] normalize-stderr-test: "(?m)note: .*$" -> ""
//[current] normalize-stderr-test: "(?m)note: we would appreciate a bug report.*\n\n" -> ""
//[current] normalize-stderr-test: "(?m)note: rustc.*running on.*\n\n" -> ""
//[current] normalize-stderr-test: "(?m)note: compiler flags.*\n\n" -> ""
//[current] normalize-stderr-test: "(?m)note: delayed at.*$" -> ""
//[current] normalize-stderr-test: "(?m)^ *\d+: .*\n" -> ""
//[current] normalize-stderr-test: "(?m)^ *at .*\n" -> ""

Expand Down

0 comments on commit 0bfd2f9

Please sign in to comment.