Skip to content

Don't emit delayed good-path bugs on panic #117397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 1, 2023
Merged

Don't emit delayed good-path bugs on panic #117397

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ impl Drop for HandlerInner {
// instead of "require some error happened". Sadly that isn't ideal, as
// lints can be `#[allow]`'d, potentially leading to this triggering.
// Also, "good path" should be replaced with a better naming.
if !self.has_any_message() && !self.suppressed_expected_diag {
if !self.has_any_message() && !self.suppressed_expected_diag && !std::thread::panicking() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might make sense to also add this at delayed_span_bugs? When there's a panic, I'm much more likely to be interested in the panic than the delayed bug. The delayed bug is actually not a bug if compilation stops with an error later -- they are used for cases where "if this happens we must never produce an output file, but we can't bug! here and now since compilation continues when an error was found, to report more than one error at a time". In those cases, when a panic occurred, all is good -- no output file is produced, after all.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm gonna leave that up to another PR.

I actually slighlty disagree, or at least think it needs a bit more nuance than the delayed_good_path_bug case -- I think that some delayed span bugs are noise (e.g. dropping opaque type storage), but some others are actually pretty important in understanding why the compiler got so messed up that it's now panicking (e.g. diagnostics that are .delay_as_bug()'d).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I wrote a bunch of delay_span_bug in the interpreter, and none of them I'd like to see on panic.

But I guess I'll wait until I actually hit that situation before trying to convince you of that. ;)

let bugs = std::mem::replace(&mut self.delayed_good_path_bugs, Vec::new());
self.flush_delayed(
bugs,
Expand Down