From 43a0f5550626a4639f34bbd8d6f39673b1ae4b91 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 14 Dec 2023 15:47:03 +1100 Subject: [PATCH 1/8] Remove unused `Handler::treat_err_as_bug`. --- compiler/rustc_errors/src/lib.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index daa8a7706eb47..db573cd931de0 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -585,11 +585,6 @@ impl Handler { self } - pub fn treat_err_as_bug(mut self, treat_err_as_bug: NonZeroUsize) -> Self { - self.inner.get_mut().flags.treat_err_as_bug = Some(treat_err_as_bug); - self - } - pub fn with_flags(mut self, flags: HandlerFlags) -> Self { self.inner.get_mut().flags = flags; self From dc05a30996987b9a1a78184a5dc173b19404f1ab Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 13 Dec 2023 14:36:07 +1100 Subject: [PATCH 2/8] Inline and remove `HandlerInner::emit_diag_at_span`. It has a single call site. --- compiler/rustc_errors/src/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index db573cd931de0..3718fd535f516 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1575,14 +1575,10 @@ impl HandlerInner { #[track_caller] fn span_bug(&mut self, sp: impl Into, msg: impl Into) -> ! { - self.emit_diag_at_span(Diagnostic::new(Bug, msg.into()), sp); + self.emit_diagnostic(Diagnostic::new(Bug, msg.into()).set_span(sp)); panic::panic_any(ExplicitBug); } - fn emit_diag_at_span(&mut self, mut diag: Diagnostic, sp: impl Into) { - self.emit_diagnostic(diag.set_span(sp)); - } - fn failure_note(&mut self, msg: impl Into) { self.emit_diagnostic(&mut Diagnostic::new(FailureNote, msg)); } From 7bdb227567398bed342697ea1f76f2cb34c9a7c2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 13 Dec 2023 15:19:34 +1100 Subject: [PATCH 3/8] Avoid `struct_diagnostic` where possible. It's necessary for `derive(Diagnostic)`, but is best avoided elsewhere because there are clearer alternatives. This required adding `Handler::struct_almost_fatal`. --- compiler/rustc_builtin_macros/src/errors.rs | 4 ++-- compiler/rustc_codegen_llvm/src/errors.rs | 3 ++- compiler/rustc_errors/src/lib.rs | 19 +++++++++++++++++-- compiler/rustc_mir_transform/src/errors.rs | 2 +- compiler/rustc_parse/src/errors.rs | 4 ++-- compiler/rustc_session/src/errors.rs | 2 +- compiler/rustc_session/src/parse.rs | 11 +---------- 7 files changed, 26 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs index 0a3af2c2e1330..3d02cd72e5479 100644 --- a/compiler/rustc_builtin_macros/src/errors.rs +++ b/compiler/rustc_builtin_macros/src/errors.rs @@ -453,7 +453,7 @@ impl<'a> IntoDiagnostic<'a> for EnvNotDefinedWithUserMessage { rustc::untranslatable_diagnostic, reason = "cannot translate user-provided messages" )] - let mut diag = handler.struct_diagnostic(self.msg_from_user.to_string()); + let mut diag = handler.struct_err(self.msg_from_user.to_string()); diag.set_span(self.span); diag } @@ -804,7 +804,7 @@ pub(crate) struct AsmClobberNoReg { impl<'a> IntoDiagnostic<'a> for AsmClobberNoReg { fn into_diagnostic(self, handler: &'a Handler) -> DiagnosticBuilder<'a, ErrorGuaranteed> { let mut diag = - handler.struct_diagnostic(crate::fluent_generated::builtin_macros_asm_clobber_no_reg); + handler.struct_err(crate::fluent_generated::builtin_macros_asm_clobber_no_reg); diag.set_span(self.spans.clone()); // eager translation as `span_labels` takes `AsRef` let lbl1 = handler.eagerly_translate_to_string( diff --git a/compiler/rustc_codegen_llvm/src/errors.rs b/compiler/rustc_codegen_llvm/src/errors.rs index 57ea13ddcd6a8..e6e37a0233547 100644 --- a/compiler/rustc_codegen_llvm/src/errors.rs +++ b/compiler/rustc_codegen_llvm/src/errors.rs @@ -107,7 +107,8 @@ impl IntoDiagnostic<'_, FatalError> for ParseTargetMachineConfig<'_> { let (message, _) = diag.styled_message().first().expect("`LlvmError` with no message"); let message = handler.eagerly_translate_to_string(message.clone(), diag.args()); - let mut diag = handler.struct_diagnostic(fluent::codegen_llvm_parse_target_machine_config); + let mut diag = + handler.struct_almost_fatal(fluent::codegen_llvm_parse_target_machine_config); diag.set_arg("error", message); diag } diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 3718fd535f516..4093daba4b4fe 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -722,7 +722,12 @@ impl Handler { self.inner.borrow_mut().emit_stashed_diagnostics() } - /// Construct a builder with the `msg` at the level appropriate for the specific `EmissionGuarantee`. + /// Construct a builder with the `msg` at the level appropriate for the + /// specific `EmissionGuarantee`. + /// + /// Note: this is necessary for `derive(Diagnostic)`, but shouldn't be used + /// outside of that. Instead use `struct_err`, `struct_warn`, etc., which + /// make the diagnostic kind clearer. #[rustc_lint_diagnostics] #[track_caller] pub fn struct_diagnostic( @@ -937,13 +942,23 @@ impl Handler { result } - /// Construct a builder at the `Error` level with the `msg`. + /// Construct a builder at the `Fatal` level with the `msg`. #[rustc_lint_diagnostics] #[track_caller] pub fn struct_fatal(&self, msg: impl Into) -> DiagnosticBuilder<'_, !> { DiagnosticBuilder::new(self, Level::Fatal, msg) } + /// Construct a builder at the `Fatal` level with the `msg`, that doesn't abort. + #[rustc_lint_diagnostics] + #[track_caller] + pub fn struct_almost_fatal( + &self, + msg: impl Into, + ) -> DiagnosticBuilder<'_, FatalError> { + DiagnosticBuilder::new(self, Level::Fatal, msg) + } + /// Construct a builder at the `Help` level with the `msg`. #[rustc_lint_diagnostics] pub fn struct_help(&self, msg: impl Into) -> DiagnosticBuilder<'_, ()> { diff --git a/compiler/rustc_mir_transform/src/errors.rs b/compiler/rustc_mir_transform/src/errors.rs index 2358661738a55..dbf27ea60f836 100644 --- a/compiler/rustc_mir_transform/src/errors.rs +++ b/compiler/rustc_mir_transform/src/errors.rs @@ -65,7 +65,7 @@ pub(crate) struct RequiresUnsafe { impl<'sess> IntoDiagnostic<'sess> for RequiresUnsafe { #[track_caller] fn into_diagnostic(self, handler: &'sess Handler) -> DiagnosticBuilder<'sess, ErrorGuaranteed> { - let mut diag = handler.struct_diagnostic(fluent::mir_transform_requires_unsafe); + let mut diag = handler.struct_err(fluent::mir_transform_requires_unsafe); diag.code(rustc_errors::DiagnosticId::Error("E0133".to_string())); diag.set_span(self.span); diag.span_label(self.span, self.details.label()); diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index bc53ab83439d1..c51a5c095ee2f 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -1046,7 +1046,7 @@ impl<'a> IntoDiagnostic<'a> for ExpectedIdentifier { ) -> rustc_errors::DiagnosticBuilder<'a, ErrorGuaranteed> { let token_descr = TokenDescription::from_token(&self.token); - let mut diag = handler.struct_diagnostic(match token_descr { + let mut diag = handler.struct_err(match token_descr { Some(TokenDescription::ReservedIdentifier) => { fluent::parse_expected_identifier_found_reserved_identifier_str } @@ -1103,7 +1103,7 @@ impl<'a> IntoDiagnostic<'a> for ExpectedSemi { ) -> rustc_errors::DiagnosticBuilder<'a, ErrorGuaranteed> { let token_descr = TokenDescription::from_token(&self.token); - let mut diag = handler.struct_diagnostic(match token_descr { + let mut diag = handler.struct_err(match token_descr { Some(TokenDescription::ReservedIdentifier) => { fluent::parse_expected_semi_found_reserved_identifier_str } diff --git a/compiler/rustc_session/src/errors.rs b/compiler/rustc_session/src/errors.rs index 7eed59709c8de..aab7595ef6ef4 100644 --- a/compiler/rustc_session/src/errors.rs +++ b/compiler/rustc_session/src/errors.rs @@ -19,7 +19,7 @@ impl<'a> IntoDiagnostic<'a> for FeatureGateError { self, handler: &'a rustc_errors::Handler, ) -> rustc_errors::DiagnosticBuilder<'a, ErrorGuaranteed> { - let mut diag = handler.struct_diagnostic(self.explain); + let mut diag = handler.struct_err(self.explain); diag.set_span(self.span); diag.code(error_code!(E0658)); diag diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index 881e1de6755c5..525f00f5cd044 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -14,7 +14,7 @@ use rustc_data_structures::sync::{AppendOnlyVec, Lock, Lrc}; use rustc_errors::{emitter::SilentEmitter, Handler}; use rustc_errors::{ fallback_fluent_bundle, Diagnostic, DiagnosticBuilder, DiagnosticId, DiagnosticMessage, - EmissionGuarantee, ErrorGuaranteed, IntoDiagnostic, MultiSpan, Noted, StashKey, + ErrorGuaranteed, IntoDiagnostic, MultiSpan, Noted, StashKey, }; use rustc_feature::{find_feature_issue, GateIssue, UnstableFeatures}; use rustc_span::edition::Edition; @@ -390,13 +390,4 @@ impl ParseSess { pub fn struct_fatal(&self, msg: impl Into) -> DiagnosticBuilder<'_, !> { self.span_diagnostic.struct_fatal(msg) } - - #[rustc_lint_diagnostics] - #[track_caller] - pub fn struct_diagnostic( - &self, - msg: impl Into, - ) -> DiagnosticBuilder<'_, G> { - self.span_diagnostic.struct_diagnostic(msg) - } } From e3b7ecc1efc7170fe418e99e071598e7e14aebe1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 13 Dec 2023 16:03:54 +1100 Subject: [PATCH 4/8] Remove one use of `span_bug_no_panic`. It's unclear why this is used here. All entries in the third column of `UNICODE_ARRAY` are covered by `ASCII_ARRAY`, so if the lookup fails it's a genuine compiler bug. It was added way back in #29837, for no clear reason. This commit changes it to `span_bug`, which is more typical. --- compiler/rustc_parse/src/lexer/unicode_chars.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_parse/src/lexer/unicode_chars.rs b/compiler/rustc_parse/src/lexer/unicode_chars.rs index bbfb160ebf7cb..0dc6068895500 100644 --- a/compiler/rustc_parse/src/lexer/unicode_chars.rs +++ b/compiler/rustc_parse/src/lexer/unicode_chars.rs @@ -350,8 +350,7 @@ pub(super) fn check_for_substitution( let Some((_, ascii_name, token)) = ASCII_ARRAY.iter().find(|&&(s, _, _)| s == ascii_str) else { let msg = format!("substitution character not found for '{ch}'"); - reader.sess.span_diagnostic.span_bug_no_panic(span, msg); - return (None, None); + reader.sess.span_diagnostic.span_bug(span, msg); }; // special help suggestion for "directed" double quotes From 19d28a4f28ad8125775e73e97e8bab2b2229f4e1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 14 Dec 2023 12:26:15 +1100 Subject: [PATCH 5/8] Change `msg: impl Into` for bug diagnostics. To `msg: impl Into`, like all the other diagnostics. For consistency. --- compiler/rustc_errors/src/lib.rs | 14 +++++++------- compiler/rustc_expand/src/base.rs | 2 +- compiler/rustc_middle/src/ty/sty.rs | 6 ++++-- compiler/rustc_parse/src/parser/diagnostics.rs | 4 ++-- compiler/rustc_session/src/session.rs | 2 +- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 4093daba4b4fe..aae2910537a35 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1033,7 +1033,7 @@ impl Handler { self.emit_diag_at_span(Diagnostic::new_with_code(Warning(None), Some(code), msg), span); } - pub fn span_bug(&self, span: impl Into, msg: impl Into) -> ! { + pub fn span_bug(&self, span: impl Into, msg: impl Into) -> ! { self.inner.borrow_mut().span_bug(span, msg) } @@ -1045,7 +1045,7 @@ impl Handler { pub fn span_delayed_bug( &self, sp: impl Into, - msg: impl Into, + msg: impl Into, ) -> ErrorGuaranteed { let mut inner = self.inner.borrow_mut(); @@ -1056,10 +1056,10 @@ impl Handler { inner.err_count + inner.lint_err_count + inner.delayed_bug_count() + 1 >= c.get() }) { // FIXME: don't abort here if report_delayed_bugs is off - inner.span_bug(sp, msg.into()); + inner.span_bug(sp, msg); } - let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg.into()); - diagnostic.set_span(sp.into()); + let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg); + diagnostic.set_span(sp); inner.emit_diagnostic(&mut diagnostic).unwrap() } @@ -1589,8 +1589,8 @@ impl HandlerInner { } #[track_caller] - fn span_bug(&mut self, sp: impl Into, msg: impl Into) -> ! { - self.emit_diagnostic(Diagnostic::new(Bug, msg.into()).set_span(sp)); + fn span_bug(&mut self, sp: impl Into, msg: impl Into) -> ! { + self.emit_diagnostic(Diagnostic::new(Bug, msg).set_span(sp)); panic::panic_any(ExplicitBug); } diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 11db4bb401762..d75556ac7c4d5 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -1145,7 +1145,7 @@ impl<'a> ExtCtxt<'a> { pub fn span_err>(&self, sp: S, msg: impl Into) { self.sess.diagnostic().span_err(sp, msg); } - pub fn span_bug>(&self, sp: S, msg: impl Into) -> ! { + pub fn span_bug>(&self, sp: S, msg: impl Into) -> ! { self.sess.diagnostic().span_bug(sp, msg); } pub fn trace_macros_diag(&mut self) { diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 50a1b85b16918..297a0be6a27cb 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -15,7 +15,9 @@ use hir::def::DefKind; use polonius_engine::Atom; use rustc_data_structures::captures::Captures; use rustc_data_structures::intern::Interned; -use rustc_errors::{DiagnosticArgValue, ErrorGuaranteed, IntoDiagnosticArg, MultiSpan}; +use rustc_errors::{ + DiagnosticArgValue, DiagnosticMessage, ErrorGuaranteed, IntoDiagnosticArg, MultiSpan, +}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::LangItem; @@ -2005,7 +2007,7 @@ impl<'tcx> Ty<'tcx> { pub fn new_error_with_message>( tcx: TyCtxt<'tcx>, span: S, - msg: impl Into, + msg: impl Into, ) -> Ty<'tcx> { let reported = tcx.sess.span_delayed_bug(span, msg); Ty::new(tcx, Error(reported)) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 5295172b25e77..221fc70d9ff70 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -249,8 +249,8 @@ impl<'a> Parser<'a> { self.diagnostic().struct_span_err(sp, m) } - pub fn span_bug>(&self, sp: S, m: impl Into) -> ! { - self.diagnostic().span_bug(sp, m) + pub fn span_bug>(&self, sp: S, msg: impl Into) -> ! { + self.diagnostic().span_bug(sp, msg) } pub(super) fn diagnostic(&self) -> &'a Handler { diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 24c7459392abe..233e56e0c449d 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -632,7 +632,7 @@ impl Session { pub fn span_delayed_bug>( &self, sp: S, - msg: impl Into, + msg: impl Into, ) -> ErrorGuaranteed { self.diagnostic().span_delayed_bug(sp, msg) } From b0d5b442e952eb7b51361d4a3a6d11aff10d1cc0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 13 Dec 2023 16:36:57 +1100 Subject: [PATCH 6/8] Avoid `DiagnosticBuilder::::new` calls. The `Handler` functions that directly emit diagnostics can be more easily implemented using `struct_foo(msg).emit()`. This mirrors `Handler::emit_err` which just does `create_err(err).emit()`. `Handler::bug` is not converted because of weirdness involving conflation bugs and fatal errors with `EmissionGuarantee`. I'll fix that later. --- compiler/rustc_errors/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index aae2910537a35..068d2aaba1546 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1101,22 +1101,22 @@ impl Handler { #[rustc_lint_diagnostics] pub fn fatal(&self, msg: impl Into) -> ! { - DiagnosticBuilder::::new(self, Fatal, msg).emit().raise() + self.struct_fatal(msg).emit() } #[rustc_lint_diagnostics] pub fn err(&self, msg: impl Into) -> ErrorGuaranteed { - DiagnosticBuilder::::new(self, Error { lint: false }, msg).emit() + self.struct_err(msg).emit() } #[rustc_lint_diagnostics] pub fn warn(&self, msg: impl Into) { - DiagnosticBuilder::<()>::new(self, Warning(None), msg).emit(); + self.struct_warn(msg).emit() } #[rustc_lint_diagnostics] pub fn note(&self, msg: impl Into) { - DiagnosticBuilder::<()>::new(self, Note, msg).emit(); + self.struct_note(msg).emit() } pub fn bug(&self, msg: impl Into) -> ! { From 2c2c7f13a65f22969adf52a8040fb1bed842fb94 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 13 Dec 2023 17:15:58 +1100 Subject: [PATCH 7/8] Remove `Handler::emit_diag_at_span`. Compare `Handler::warn` and `Handler::span_warn`. Conceptually they are almost identical. But their implementations are weirdly different. `warn`: - calls `DiagnosticBuilder::<()>::new(self, Warning(None), msg)`, then `emit()` - which calls `G::diagnostic_builder_emit_producing_guarantee(self)` - which calls `handler.emit_diagnostic(&mut db.inner.diagnostic)` `span_warn`: - calls `self.emit_diag_at_span(Diagnostic::new(Warning(None), msg), span)` - which calls `self.emit_diagnostic(diag.set_span(sp))` I.e. they both end up at `emit_diagnostic`, but take very different routes to get there. This commit changes `span_*` and similar ones to not use `emit_diag_at_span`. Instead they just call `struct_span_*` + `emit`. Some nice side-effects of this: - `span_fatal` and `span_fatal_with_code` don't need `FatalError.raise()`, because `emit` does that. - `span_err` and `span_err_with_code` doesn't need `unwrap`. - `struct_span_note`'s `span` arg type is changed from `Span` to `impl Into` like all the other functions. --- compiler/rustc_errors/src/lib.rs | 32 +++++++++----------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 068d2aaba1546..508fa7c2e8e3a 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -975,8 +975,7 @@ impl Handler { #[rustc_lint_diagnostics] #[track_caller] pub fn span_fatal(&self, span: impl Into, msg: impl Into) -> ! { - self.emit_diag_at_span(Diagnostic::new(Fatal, msg), span); - FatalError.raise() + self.struct_span_fatal(span, msg).emit() } #[rustc_lint_diagnostics] @@ -987,8 +986,7 @@ impl Handler { msg: impl Into, code: DiagnosticId, ) -> ! { - self.emit_diag_at_span(Diagnostic::new_with_code(Fatal, Some(code), msg), span); - FatalError.raise() + self.struct_span_fatal_with_code(span, msg, code).emit() } #[rustc_lint_diagnostics] @@ -998,7 +996,7 @@ impl Handler { span: impl Into, msg: impl Into, ) -> ErrorGuaranteed { - self.emit_diag_at_span(Diagnostic::new(Error { lint: false }, msg), span).unwrap() + self.struct_span_err(span, msg).emit() } #[rustc_lint_diagnostics] @@ -1009,17 +1007,13 @@ impl Handler { msg: impl Into, code: DiagnosticId, ) -> ErrorGuaranteed { - self.emit_diag_at_span( - Diagnostic::new_with_code(Error { lint: false }, Some(code), msg), - span, - ) - .unwrap() + self.struct_span_err_with_code(span, msg, code).emit() } #[rustc_lint_diagnostics] #[track_caller] pub fn span_warn(&self, span: impl Into, msg: impl Into) { - self.emit_diag_at_span(Diagnostic::new(Warning(None), msg), span); + self.struct_span_warn(span, msg).emit() } #[rustc_lint_diagnostics] @@ -1030,7 +1024,7 @@ impl Handler { msg: impl Into, code: DiagnosticId, ) { - self.emit_diag_at_span(Diagnostic::new_with_code(Warning(None), Some(code), msg), span); + self.struct_span_warn_with_code(span, msg, code).emit() } pub fn span_bug(&self, span: impl Into, msg: impl Into) -> ! { @@ -1078,20 +1072,20 @@ impl Handler { #[track_caller] pub fn span_bug_no_panic(&self, span: impl Into, msg: impl Into) { - self.emit_diag_at_span(Diagnostic::new(Bug, msg), span); + self.emit_diagnostic(Diagnostic::new(Bug, msg).set_span(span)); } #[track_caller] #[rustc_lint_diagnostics] pub fn span_note(&self, span: impl Into, msg: impl Into) { - self.emit_diag_at_span(Diagnostic::new(Note, msg), span); + self.struct_span_note(span, msg).emit() } #[track_caller] #[rustc_lint_diagnostics] pub fn struct_span_note( &self, - span: Span, + span: impl Into, msg: impl Into, ) -> DiagnosticBuilder<'_, ()> { let mut db = DiagnosticBuilder::new(self, Note, msg); @@ -1337,14 +1331,6 @@ impl Handler { note.into_diagnostic(self) } - fn emit_diag_at_span( - &self, - mut diag: Diagnostic, - sp: impl Into, - ) -> Option { - self.emit_diagnostic(diag.set_span(sp)) - } - pub fn emit_artifact_notification(&self, path: &Path, artifact_type: &str) { self.inner.borrow_mut().emitter.emit_artifact_notification(path, artifact_type); } From 9a7841251115c79fe7e1c12e2e5d637e19ad940a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 14 Dec 2023 14:13:35 +1100 Subject: [PATCH 8/8] Split `Handler::emit_diagnostic` in two. Currently, `emit_diagnostic` takes `&mut self`. This commit changes it so `emit_diagnostic` takes `self` and the new `emit_diagnostic_without_consuming` function takes `&mut self`. I find the distinction useful. The former case is much more common, and avoids a bunch of `mut` and `&mut` occurrences. We can also restrict the latter with `pub(crate)` which is nice. --- compiler/rustc_borrowck/src/lib.rs | 4 +- compiler/rustc_codegen_ssa/src/back/write.rs | 2 +- .../src/transform/check_consts/check.rs | 4 +- .../rustc_errors/src/diagnostic_builder.rs | 20 ++++---- compiler/rustc_errors/src/emitter.rs | 2 +- compiler/rustc_errors/src/lib.rs | 51 +++++++++++++------ .../rustc_expand/src/proc_macro_server.rs | 2 +- compiler/rustc_hir_typeck/src/writeback.rs | 4 +- compiler/rustc_parse/src/lib.rs | 8 +-- .../rustc_query_system/src/dep_graph/graph.rs | 4 +- src/tools/miri/src/diagnostics.rs | 2 +- src/tools/rustfmt/src/parse/session.rs | 6 +-- 12 files changed, 64 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 3d3fd412ae0d1..7e0e598cd9f7d 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -2502,8 +2502,8 @@ mod error { if !self.errors.buffered.is_empty() { self.errors.buffered.sort_by_key(|diag| diag.sort_span); - for mut diag in self.errors.buffered.drain(..) { - self.infcx.tcx.sess.diagnostic().emit_diagnostic(&mut diag); + for diag in self.errors.buffered.drain(..) { + self.infcx.tcx.sess.diagnostic().emit_diagnostic(diag); } } diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 618a72272e58f..40fd8c5c1d692 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -1848,7 +1848,7 @@ impl SharedEmitterMain { d.code(code); } d.replace_args(diag.args); - handler.emit_diagnostic(&mut d); + handler.emit_diagnostic(d); } Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => { let msg = msg.strip_prefix("error: ").unwrap_or(&msg).to_string(); diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index 5380d3071d6c6..bb17602d3ba01 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -277,8 +277,8 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> { // "secondary" errors if they occurred. let secondary_errors = mem::take(&mut self.secondary_errors); if self.error_emitted.is_none() { - for mut error in secondary_errors { - self.tcx.sess.diagnostic().emit_diagnostic(&mut error); + for error in secondary_errors { + self.tcx.sess.diagnostic().emit_diagnostic(error); } } else { assert!(self.tcx.sess.has_errors().is_some()); diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs index b8bd86a72e4df..315e47c097156 100644 --- a/compiler/rustc_errors/src/diagnostic_builder.rs +++ b/compiler/rustc_errors/src/diagnostic_builder.rs @@ -132,7 +132,7 @@ impl EmissionGuarantee for ErrorGuaranteed { DiagnosticBuilderState::Emittable(handler) => { db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation; - let guar = handler.emit_diagnostic(&mut db.inner.diagnostic); + let guar = handler.emit_diagnostic_without_consuming(&mut db.inner.diagnostic); // Only allow a guarantee if the `level` wasn't switched to a // non-error - the field isn't `pub`, but the whole `Diagnostic` @@ -181,7 +181,7 @@ impl EmissionGuarantee for () { DiagnosticBuilderState::Emittable(handler) => { db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation; - handler.emit_diagnostic(&mut db.inner.diagnostic); + handler.emit_diagnostic_without_consuming(&mut db.inner.diagnostic); } // `.emit()` was previously called, disallowed from repeating it. DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {} @@ -207,7 +207,7 @@ impl EmissionGuarantee for Noted { // First `.emit()` call, the `&Handler` is still available. DiagnosticBuilderState::Emittable(handler) => { db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation; - handler.emit_diagnostic(&mut db.inner.diagnostic); + handler.emit_diagnostic_without_consuming(&mut db.inner.diagnostic); } // `.emit()` was previously called, disallowed from repeating it. DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {} @@ -236,7 +236,7 @@ impl EmissionGuarantee for Bug { DiagnosticBuilderState::Emittable(handler) => { db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation; - handler.emit_diagnostic(&mut db.inner.diagnostic); + handler.emit_diagnostic_without_consuming(&mut db.inner.diagnostic); } // `.emit()` was previously called, disallowed from repeating it. DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {} @@ -260,7 +260,7 @@ impl EmissionGuarantee for ! { DiagnosticBuilderState::Emittable(handler) => { db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation; - handler.emit_diagnostic(&mut db.inner.diagnostic); + handler.emit_diagnostic_without_consuming(&mut db.inner.diagnostic); } // `.emit()` was previously called, disallowed from repeating it. DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {} @@ -284,7 +284,7 @@ impl EmissionGuarantee for rustc_span::fatal_error::FatalError { DiagnosticBuilderState::Emittable(handler) => { db.inner.state = DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation; - handler.emit_diagnostic(&mut db.inner.diagnostic); + handler.emit_diagnostic_without_consuming(&mut db.inner.diagnostic); } // `.emit()` was previously called, disallowed from repeating it. DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {} @@ -365,7 +365,9 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> { } } - /// Emit the diagnostic. + /// Emit the diagnostic. Does not consume `self`, which may be surprising, + /// but there are various places that rely on continuing to use `self` + /// after calling `emit`. #[track_caller] pub fn emit(&mut self) -> G { G::diagnostic_builder_emit_producing_guarantee(self) @@ -640,13 +642,13 @@ impl Drop for DiagnosticBuilderInner<'_> { // No `.emit()` or `.cancel()` calls. DiagnosticBuilderState::Emittable(handler) => { if !panicking() { - handler.emit_diagnostic(&mut Diagnostic::new( + handler.emit_diagnostic(Diagnostic::new( Level::Bug, DiagnosticMessage::from( "the following error was constructed but not emitted", ), )); - handler.emit_diagnostic(&mut self.diagnostic); + handler.emit_diagnostic_without_consuming(&mut self.diagnostic); panic!("error was constructed but not emitted"); } } diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 3f257fdd9cf27..379883a0c18b7 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -581,7 +581,7 @@ impl Emitter for SilentEmitter { if let Some(ref note) = self.fatal_note { d.note(note.clone()); } - self.fatal_handler.emit_diagnostic(&mut d); + self.fatal_handler.emit_diagnostic(d); } } } diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 508fa7c2e8e3a..cf73c638d85e1 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1054,7 +1054,7 @@ impl Handler { } let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg); diagnostic.set_span(sp); - inner.emit_diagnostic(&mut diagnostic).unwrap() + inner.emit_diagnostic(diagnostic).unwrap() } // FIXME(eddyb) note the comment inside `impl Drop for HandlerInner`, that's @@ -1064,7 +1064,7 @@ impl Handler { let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg); if inner.flags.report_delayed_bugs { - inner.emit_diagnostic(&mut diagnostic); + inner.emit_diagnostic_without_consuming(&mut diagnostic); } let backtrace = std::backtrace::Backtrace::capture(); inner.good_path_delayed_bugs.push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); @@ -1072,7 +1072,9 @@ impl Handler { #[track_caller] pub fn span_bug_no_panic(&self, span: impl Into, msg: impl Into) { - self.emit_diagnostic(Diagnostic::new(Bug, msg).set_span(span)); + let mut diag = Diagnostic::new(Bug, msg); + diag.set_span(span); + self.emit_diagnostic(diag); } #[track_caller] @@ -1185,10 +1187,10 @@ impl Handler { DiagnosticMessage::Str(warnings), )), (_, 0) => { - inner.emit_diagnostic(&mut Diagnostic::new(Fatal, errors)); + inner.emit_diagnostic(Diagnostic::new(Fatal, errors)); } (_, _) => { - inner.emit_diagnostic(&mut Diagnostic::new(Fatal, format!("{errors}; {warnings}"))); + inner.emit_diagnostic(Diagnostic::new(Fatal, format!("{errors}; {warnings}"))); } } @@ -1255,8 +1257,17 @@ impl Handler { self.inner.borrow_mut().emitter.emit_diagnostic(&db); } - pub fn emit_diagnostic(&self, diagnostic: &mut Diagnostic) -> Option { - self.inner.borrow_mut().emit_diagnostic(diagnostic) + pub fn emit_diagnostic(&self, mut diagnostic: Diagnostic) -> Option { + self.emit_diagnostic_without_consuming(&mut diagnostic) + } + + // It's unfortunate this exists. `emit_diagnostic` is preferred, because it + // consumes the diagnostic, thus ensuring it is emitted just once. + pub(crate) fn emit_diagnostic_without_consuming( + &self, + diagnostic: &mut Diagnostic, + ) -> Option { + self.inner.borrow_mut().emit_diagnostic_without_consuming(diagnostic) } pub fn emit_err<'a>(&'a self, err: impl IntoDiagnostic<'a>) -> ErrorGuaranteed { @@ -1370,7 +1381,7 @@ impl Handler { // Here the diagnostic is given back to `emit_diagnostic` where it was first // intercepted. Now it should be processed as usual, since the unstable expectation // id is now stable. - inner.emit_diagnostic(&mut diag); + inner.emit_diagnostic(diag); } } @@ -1412,7 +1423,7 @@ impl HandlerInner { let has_errors = self.has_errors(); let diags = self.stashed_diagnostics.drain(..).map(|x| x.1).collect::>(); let mut reported = None; - for mut diag in diags { + for diag in diags { // Decrement the count tracking the stash; emitting will increment it. if diag.is_error() { if matches!(diag.level, Level::Error { lint: true }) { @@ -1432,14 +1443,20 @@ impl HandlerInner { } } } - let reported_this = self.emit_diagnostic(&mut diag); + let reported_this = self.emit_diagnostic(diag); reported = reported.or(reported_this); } reported } - // FIXME(eddyb) this should ideally take `diagnostic` by value. - fn emit_diagnostic(&mut self, diagnostic: &mut Diagnostic) -> Option { + fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { + self.emit_diagnostic_without_consuming(&mut diagnostic) + } + + fn emit_diagnostic_without_consuming( + &mut self, + diagnostic: &mut Diagnostic, + ) -> Option { if matches!(diagnostic.level, Level::Error { .. } | Level::Fatal) && self.treat_err_as_bug() { diagnostic.level = Level::Bug; @@ -1576,12 +1593,14 @@ impl HandlerInner { #[track_caller] fn span_bug(&mut self, sp: impl Into, msg: impl Into) -> ! { - self.emit_diagnostic(Diagnostic::new(Bug, msg).set_span(sp)); + let mut diag = Diagnostic::new(Bug, msg); + diag.set_span(sp); + self.emit_diagnostic(diag); panic::panic_any(ExplicitBug); } fn failure_note(&mut self, msg: impl Into) { - self.emit_diagnostic(&mut Diagnostic::new(FailureNote, msg)); + self.emit_diagnostic(Diagnostic::new(FailureNote, msg)); } fn flush_delayed( @@ -1613,7 +1632,7 @@ impl HandlerInner { if no_bugs { // Put the overall explanation before the `DelayedBug`s, to // frame them better (e.g. separate warnings from them). - self.emit_diagnostic(&mut Diagnostic::new(Bug, explanation)); + self.emit_diagnostic(Diagnostic::new(Bug, explanation)); no_bugs = false; } @@ -1628,7 +1647,7 @@ impl HandlerInner { } bug.level = Level::Bug; - self.emit_diagnostic(&mut bug); + self.emit_diagnostic(bug); } // Panic with `DelayedBugPanic` to avoid "unexpected panic" messages. diff --git a/compiler/rustc_expand/src/proc_macro_server.rs b/compiler/rustc_expand/src/proc_macro_server.rs index 5308e338d7f8b..d2c26668ea83a 100644 --- a/compiler/rustc_expand/src/proc_macro_server.rs +++ b/compiler/rustc_expand/src/proc_macro_server.rs @@ -502,7 +502,7 @@ impl server::FreeFunctions for Rustc<'_, '_> { None, ); } - self.sess().span_diagnostic.emit_diagnostic(&mut diag); + self.sess().span_diagnostic.emit_diagnostic(diag); } } diff --git a/compiler/rustc_hir_typeck/src/writeback.rs b/compiler/rustc_hir_typeck/src/writeback.rs index 1609c036fbd95..5e562d9453f15 100644 --- a/compiler/rustc_hir_typeck/src/writeback.rs +++ b/compiler/rustc_hir_typeck/src/writeback.rs @@ -504,8 +504,8 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { if !errors_buffer.is_empty() { errors_buffer.sort_by_key(|diag| diag.span.primary_span()); - for mut diag in errors_buffer { - self.tcx().sess.diagnostic().emit_diagnostic(&mut diag); + for diag in errors_buffer { + self.tcx().sess.diagnostic().emit_diagnostic(diag); } } } diff --git a/compiler/rustc_parse/src/lib.rs b/compiler/rustc_parse/src/lib.rs index 95352dbdc134f..9887a85e6a40c 100644 --- a/compiler/rustc_parse/src/lib.rs +++ b/compiler/rustc_parse/src/lib.rs @@ -51,8 +51,8 @@ macro_rules! panictry_buffer { match $e { Ok(e) => e, Err(errs) => { - for mut e in errs { - $handler.emit_diagnostic(&mut e); + for e in errs { + $handler.emit_diagnostic(e); } FatalError.raise() } @@ -165,8 +165,8 @@ fn try_file_to_source_file( fn file_to_source_file(sess: &ParseSess, path: &Path, spanopt: Option) -> Lrc { match try_file_to_source_file(sess, path, spanopt) { Ok(source_file) => source_file, - Err(mut d) => { - sess.span_diagnostic.emit_diagnostic(&mut d); + Err(d) => { + sess.span_diagnostic.emit_diagnostic(d); FatalError.raise(); } } diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index bc09972185a62..3b8ccb67bbefd 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -926,8 +926,8 @@ impl DepGraphData { let handle = qcx.dep_context().sess().diagnostic(); - for mut diagnostic in side_effects.diagnostics { - handle.emit_diagnostic(&mut diagnostic); + for diagnostic in side_effects.diagnostics { + handle.emit_diagnostic(diagnostic); } } } diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index ef394b163103b..e0f5914497502 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -512,7 +512,7 @@ pub fn report_msg<'tcx>( } } - handler.emit_diagnostic(&mut err); + handler.emit_diagnostic(err); } impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { diff --git a/src/tools/rustfmt/src/parse/session.rs b/src/tools/rustfmt/src/parse/session.rs index 0573df9de2f6d..06f9c4c624336 100644 --- a/src/tools/rustfmt/src/parse/session.rs +++ b/src/tools/rustfmt/src/parse/session.rs @@ -284,10 +284,8 @@ impl ParseSess { // Methods that should be restricted within the parse module. impl ParseSess { pub(super) fn emit_diagnostics(&self, diagnostics: Vec) { - for mut diagnostic in diagnostics { - self.parse_sess - .span_diagnostic - .emit_diagnostic(&mut diagnostic); + for diagnostic in diagnostics { + self.parse_sess.span_diagnostic.emit_diagnostic(diagnostic); } }