From 30774b00610bb3ab3339931f5eba21574d35a39a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 9 Feb 2024 16:12:18 +1100 Subject: [PATCH 1/4] Remove final unwanted `unchecked_error_guaranteed` calls. Now that error counts can't go up and down due to stashing/stealing, we have a nice property: (err_count > 0) iff (an ErrorGuaranteed has been produced) So we can now record `ErrorGuaranteed`s within `DiagCtxt` and use that in methods like `has_error`, instead of checking that the count is greater than 0 and calling `unchecked_error_guaranteed` to create the `ErrorGuaranteed`. In fact, we can record a `Vec` and use its length to count the number, instead of maintaining a separate count. --- compiler/rustc_errors/src/lib.rs | 118 +++++++++++++++++-------------- 1 file changed, 65 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index ab3ad0e9d68..fc5a8ef13f0 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -428,10 +428,14 @@ pub struct DiagCtxt { struct DiagCtxtInner { flags: DiagCtxtFlags, - /// The number of lint errors that have been emitted, including duplicates. - lint_err_count: usize, - /// The number of non-lint errors that have been emitted, including duplicates. - err_count: usize, + /// The error guarantees from all emitted errors. The length gives the error count. + err_guars: Vec, + /// The error guarantee from all emitted lint errors. The length gives the + /// lint error count. + lint_err_guars: Vec, + /// The delayed bugs and their error guarantees. + delayed_bugs: Vec<(DelayedDiagnostic, ErrorGuaranteed)>, + good_path_delayed_bugs: Vec, /// The number of stashed errors. Unlike the other counts, this can go up /// and down, so it doesn't guarantee anything. @@ -447,8 +451,6 @@ struct DiagCtxtInner { has_printed: bool, emitter: Box, - delayed_bugs: Vec, - good_path_delayed_bugs: Vec, /// This flag indicates that an expected diagnostic was emitted and suppressed. /// This is used for the `good_path_delayed_bugs` check. suppressed_expected_diag: bool, @@ -560,7 +562,7 @@ impl Drop for DiagCtxtInner { fn drop(&mut self) { self.emit_stashed_diagnostics(); - if !self.has_errors() { + if self.err_guars.is_empty() { self.flush_delayed(DelayedBugKind::Normal) } @@ -604,15 +606,15 @@ impl DiagCtxt { Self { inner: Lock::new(DiagCtxtInner { flags: DiagCtxtFlags { can_emit_warnings: true, ..Default::default() }, - lint_err_count: 0, - err_count: 0, + err_guars: Vec::new(), + lint_err_guars: Vec::new(), + delayed_bugs: Vec::new(), + good_path_delayed_bugs: Vec::new(), stashed_err_count: 0, deduplicated_err_count: 0, deduplicated_warn_count: 0, has_printed: false, emitter, - delayed_bugs: Vec::new(), - good_path_delayed_bugs: Vec::new(), suppressed_expected_diag: false, taught_diagnostics: Default::default(), emitted_diagnostic_codes: Default::default(), @@ -661,14 +663,14 @@ impl DiagCtxt { /// the overall count of emitted error diagnostics. pub fn reset_err_count(&self) { let mut inner = self.inner.borrow_mut(); - inner.lint_err_count = 0; - inner.err_count = 0; inner.stashed_err_count = 0; inner.deduplicated_err_count = 0; inner.deduplicated_warn_count = 0; inner.has_printed = false; // actually free the underlying memory (which `clear` would not do) + inner.err_guars = Default::default(); + inner.lint_err_guars = Default::default(); inner.delayed_bugs = Default::default(); inner.good_path_delayed_bugs = Default::default(); inner.taught_diagnostics = Default::default(); @@ -932,7 +934,7 @@ impl DiagCtxt { /// This excludes lint errors, delayed bugs, and stashed errors. #[inline] pub fn err_count(&self) -> usize { - self.inner.borrow().err_count + self.inner.borrow().err_guars.len() } /// This excludes normal errors, lint errors and delayed bugs. Unless @@ -946,36 +948,19 @@ impl DiagCtxt { /// This excludes lint errors, delayed bugs, and stashed errors. pub fn has_errors(&self) -> Option { - self.inner.borrow().has_errors().then(|| { - // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. - #[allow(deprecated)] - ErrorGuaranteed::unchecked_error_guaranteed() - }) + self.inner.borrow().has_errors() } /// This excludes delayed bugs and stashed errors. Unless absolutely /// necessary, prefer `has_errors` to this method. pub fn has_errors_or_lint_errors(&self) -> Option { - let inner = self.inner.borrow(); - let result = inner.has_errors() || inner.lint_err_count > 0; - result.then(|| { - // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. - #[allow(deprecated)] - ErrorGuaranteed::unchecked_error_guaranteed() - }) + self.inner.borrow().has_errors_or_lint_errors() } /// This excludes stashed errors. Unless absolutely necessary, prefer /// `has_errors` or `has_errors_or_lint_errors` to this method. pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option { - let inner = self.inner.borrow(); - let result = - inner.has_errors() || inner.lint_err_count > 0 || !inner.delayed_bugs.is_empty(); - result.then(|| { - // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. - #[allow(deprecated)] - ErrorGuaranteed::unchecked_error_guaranteed() - }) + self.inner.borrow().has_errors_or_lint_errors_or_delayed_bugs() } pub fn print_error_count(&self, registry: &Registry) { @@ -1055,7 +1040,7 @@ impl DiagCtxt { pub fn abort_if_errors(&self) { let mut inner = self.inner.borrow_mut(); inner.emit_stashed_diagnostics(); - if inner.has_errors() { + if !inner.err_guars.is_empty() { FatalError.raise(); } } @@ -1175,8 +1160,21 @@ impl DiagCtxt { ) { let mut inner = self.inner.borrow_mut(); + // This "error" is an odd duck. + // - It's only produce with JSON output. + // - It's not emitted the usual way, via `emit_diagnostic`. + // - The `$message_type` field is "unused_externs" rather than the usual + // "diagnosic". + // + // We count it as a lint error because it has a lint level. The value + // of `loud` (which comes from "unused-externs" or + // "unused-externs-silent"), also affects whether it's treated like a + // hard error or not. if loud && lint_level.is_error() { - inner.lint_err_count += 1; + // This `unchecked_error_guaranteed` is valid. It is where the + // `ErrorGuaranteed` for unused_extern errors originates. + #[allow(deprecated)] + inner.lint_err_guars.push(ErrorGuaranteed::unchecked_error_guaranteed()); inner.panic_if_treat_err_as_bug(); } @@ -1236,7 +1234,7 @@ impl DiagCtxt { impl DiagCtxtInner { /// Emit all stashed diagnostics. fn emit_stashed_diagnostics(&mut self) { - let has_errors = self.has_errors(); + let has_errors = !self.err_guars.is_empty(); for (_, diag) in std::mem::take(&mut self.stashed_diagnostics).into_iter() { // Decrement the count tracking the stash; emitting will increment it. if diag.is_error() { @@ -1298,9 +1296,13 @@ impl DiagCtxtInner { // when an error is first emitted, also), but maybe there's a case // in which that's not sound? otherwise this is really inefficient. let backtrace = std::backtrace::Backtrace::capture(); - self.delayed_bugs.push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); + // This `unchecked_error_guaranteed` is valid. It is where the + // `ErrorGuaranteed` for delayed bugs originates. #[allow(deprecated)] - return Some(ErrorGuaranteed::unchecked_error_guaranteed()); + let guar = ErrorGuaranteed::unchecked_error_guaranteed(); + self.delayed_bugs + .push((DelayedDiagnostic::with_backtrace(diagnostic, backtrace), guar)); + return Some(guar); } GoodPathDelayedBug => { let backtrace = std::backtrace::Backtrace::capture(); @@ -1334,7 +1336,6 @@ impl DiagCtxtInner { !self.emitted_diagnostics.insert(diagnostic_hash) }; - let level = diagnostic.level; let is_error = diagnostic.is_error(); let is_lint = diagnostic.is_lint.is_some(); @@ -1373,36 +1374,47 @@ impl DiagCtxtInner { } if is_error { + // This `unchecked_error_guaranteed` is valid. It is where the + // `ErrorGuaranteed` for errors and lint errors originates. + #[allow(deprecated)] + let guar = ErrorGuaranteed::unchecked_error_guaranteed(); + guaranteed = Some(guar); if is_lint { - self.lint_err_count += 1; + self.lint_err_guars.push(guar); } else { - self.err_count += 1; + self.err_guars.push(guar); } self.panic_if_treat_err_as_bug(); } - - #[allow(deprecated)] - if level == Level::Error { - guaranteed = Some(ErrorGuaranteed::unchecked_error_guaranteed()); - } }); guaranteed } fn treat_err_as_bug(&self) -> bool { - self.flags.treat_err_as_bug.is_some_and(|c| self.err_count + self.lint_err_count >= c.get()) + self.flags + .treat_err_as_bug + .is_some_and(|c| self.err_guars.len() + self.lint_err_guars.len() >= 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 + 1 >= c.get()) + .is_some_and(|c| self.err_guars.len() + self.lint_err_guars.len() + 1 >= c.get()) } - fn has_errors(&self) -> bool { - self.err_count > 0 + fn has_errors(&self) -> Option { + self.err_guars.get(0).copied() + } + + fn has_errors_or_lint_errors(&self) -> Option { + self.has_errors().or_else(|| self.lint_err_guars.get(0).copied()) + } + + fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option { + self.has_errors_or_lint_errors() + .or_else(|| self.delayed_bugs.get(0).map(|(_, guar)| guar).copied()) } fn failure_note(&mut self, msg: impl Into) { @@ -1412,7 +1424,7 @@ impl DiagCtxtInner { fn flush_delayed(&mut self, kind: DelayedBugKind) { let (bugs, note1) = match kind { DelayedBugKind::Normal => ( - std::mem::take(&mut self.delayed_bugs), + std::mem::take(&mut self.delayed_bugs).into_iter().map(|(b, _)| b).collect(), "no errors encountered even though delayed bugs were created", ), DelayedBugKind::GoodPath => ( @@ -1477,7 +1489,7 @@ impl DiagCtxtInner { fn panic_if_treat_err_as_bug(&self) { if self.treat_err_as_bug() { let n = self.flags.treat_err_as_bug.map(|c| c.get()).unwrap(); - assert_eq!(n, self.err_count + self.lint_err_count); + assert_eq!(n, self.err_guars.len() + self.lint_err_guars.len()); if n == 1 { panic!("aborting due to `-Z treat-err-as-bug=1`"); } else { From e0a0cc29710828066da211abf605e6c75c7410fc Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 12 Feb 2024 15:18:18 +1100 Subject: [PATCH 2/4] Remove `dcx` arg from `ReportErrorExt::add_args`. Because it also has a `DiagnosticBuilder` arg, which contains a `dcx` reference. Also rename some `builder` variables as `diag`, because that's the usual name. --- .../rustc_const_eval/src/const_eval/error.rs | 2 +- compiler/rustc_const_eval/src/errors.rs | 134 ++++++++---------- .../src/interpret/eval_context.rs | 2 +- 3 files changed, 62 insertions(+), 76 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/error.rs b/compiler/rustc_const_eval/src/const_eval/error.rs index 71085c2b2a5..80d02589900 100644 --- a/compiler/rustc_const_eval/src/const_eval/error.rs +++ b/compiler/rustc_const_eval/src/const_eval/error.rs @@ -151,7 +151,7 @@ where let mut err = tcx.dcx().create_err(err); let msg = error.diagnostic_message(); - error.add_args(tcx.dcx(), &mut err); + error.add_args(&mut err); // Use *our* span to label the interp error err.span_label(our_span, msg); diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index a649526c196..11679ab77e3 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/compiler/rustc_const_eval/src/errors.rs @@ -426,7 +426,7 @@ pub struct UndefinedBehavior { pub trait ReportErrorExt { /// Returns the diagnostic message for this error. fn diagnostic_message(&self) -> DiagnosticMessage; - fn add_args(self, dcx: &DiagCtxt, builder: &mut DiagnosticBuilder<'_, G>); + fn add_args(self, diag: &mut DiagnosticBuilder<'_, G>); fn debug(self) -> String where @@ -434,11 +434,11 @@ pub trait ReportErrorExt { { ty::tls::with(move |tcx| { let dcx = tcx.dcx(); - let mut builder = dcx.struct_allow(DiagnosticMessage::Str(String::new().into())); + let mut diag = dcx.struct_allow(DiagnosticMessage::Str(String::new().into())); let message = self.diagnostic_message(); - self.add_args(dcx, &mut builder); - let s = dcx.eagerly_translate_to_string(message, builder.args()); - builder.cancel(); + self.add_args(&mut diag); + let s = dcx.eagerly_translate_to_string(message, diag.args()); + diag.cancel(); s }) } @@ -505,20 +505,17 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> { } } - fn add_args( - self, - dcx: &DiagCtxt, - builder: &mut DiagnosticBuilder<'_, G>, - ) { + fn add_args(self, diag: &mut DiagnosticBuilder<'_, G>) { use UndefinedBehaviorInfo::*; + let dcx = diag.dcx; match self { Ub(_) => {} Custom(custom) => { (custom.add_args)(&mut |name, value| { - builder.arg(name, value); + diag.arg(name, value); }); } - ValidationError(e) => e.add_args(dcx, builder), + ValidationError(e) => e.add_args(diag), Unreachable | DivisionByZero @@ -533,20 +530,18 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> { | UninhabitedEnumVariantWritten(_) | UninhabitedEnumVariantRead(_) => {} BoundsCheckFailed { len, index } => { - builder.arg("len", len); - builder.arg("index", index); + diag.arg("len", len); + diag.arg("index", index); } UnterminatedCString(ptr) | InvalidFunctionPointer(ptr) | InvalidVTablePointer(ptr) => { - builder.arg("pointer", ptr); + diag.arg("pointer", ptr); } PointerUseAfterFree(alloc_id, msg) => { - builder - .arg("alloc_id", alloc_id) + diag.arg("alloc_id", alloc_id) .arg("bad_pointer_message", bad_pointer_message(msg, dcx)); } PointerOutOfBounds { alloc_id, alloc_size, ptr_offset, ptr_size, msg } => { - builder - .arg("alloc_id", alloc_id) + diag.arg("alloc_id", alloc_id) .arg("alloc_size", alloc_size.bytes()) .arg("ptr_offset", ptr_offset) .arg("ptr_size", ptr_size.bytes()) @@ -554,47 +549,47 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> { } DanglingIntPointer(ptr, msg) => { if ptr != 0 { - builder.arg("pointer", format!("{ptr:#x}[noalloc]")); + diag.arg("pointer", format!("{ptr:#x}[noalloc]")); } - builder.arg("bad_pointer_message", bad_pointer_message(msg, dcx)); + diag.arg("bad_pointer_message", bad_pointer_message(msg, dcx)); } AlignmentCheckFailed(Misalignment { required, has }, msg) => { - builder.arg("required", required.bytes()); - builder.arg("has", has.bytes()); - builder.arg("msg", format!("{msg:?}")); + diag.arg("required", required.bytes()); + diag.arg("has", has.bytes()); + diag.arg("msg", format!("{msg:?}")); } WriteToReadOnly(alloc) | DerefFunctionPointer(alloc) | DerefVTablePointer(alloc) => { - builder.arg("allocation", alloc); + diag.arg("allocation", alloc); } InvalidBool(b) => { - builder.arg("value", format!("{b:02x}")); + diag.arg("value", format!("{b:02x}")); } InvalidChar(c) => { - builder.arg("value", format!("{c:08x}")); + diag.arg("value", format!("{c:08x}")); } InvalidTag(tag) => { - builder.arg("tag", format!("{tag:x}")); + diag.arg("tag", format!("{tag:x}")); } InvalidStr(err) => { - builder.arg("err", format!("{err}")); + diag.arg("err", format!("{err}")); } InvalidUninitBytes(Some((alloc, info))) => { - builder.arg("alloc", alloc); - builder.arg("access", info.access); - builder.arg("uninit", info.bad); + diag.arg("alloc", alloc); + diag.arg("access", info.access); + diag.arg("uninit", info.bad); } ScalarSizeMismatch(info) => { - builder.arg("target_size", info.target_size); - builder.arg("data_size", info.data_size); + diag.arg("target_size", info.target_size); + diag.arg("data_size", info.data_size); } InvalidNichedEnumVariantWritten { enum_ty } => { - builder.arg("ty", enum_ty.to_string()); + diag.arg("ty", enum_ty.to_string()); } AbiMismatchArgument { caller_ty, callee_ty } | AbiMismatchReturn { caller_ty, callee_ty } => { - builder.arg("caller_ty", caller_ty.to_string()); - builder.arg("callee_ty", callee_ty.to_string()); + diag.arg("caller_ty", caller_ty.to_string()); + diag.arg("callee_ty", callee_ty.to_string()); } } } @@ -674,7 +669,7 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> { } } - fn add_args(self, dcx: &DiagCtxt, err: &mut DiagnosticBuilder<'_, G>) { + fn add_args(self, err: &mut DiagnosticBuilder<'_, G>) { use crate::fluent_generated as fluent; use rustc_middle::mir::interpret::ValidationErrorKind::*; @@ -684,12 +679,12 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> { } let message = if let Some(path) = self.path { - dcx.eagerly_translate_to_string( + err.dcx.eagerly_translate_to_string( fluent::const_eval_validation_front_matter_invalid_value_with_path, [("path".into(), DiagnosticArgValue::Str(path.into()))].iter().map(|(a, b)| (a, b)), ) } else { - dcx.eagerly_translate_to_string( + err.dcx.eagerly_translate_to_string( fluent::const_eval_validation_front_matter_invalid_value, [].into_iter(), ) @@ -700,7 +695,6 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> { fn add_range_arg( r: WrappingRange, max_hi: u128, - dcx: &DiagCtxt, err: &mut DiagnosticBuilder<'_, G>, ) { let WrappingRange { start: lo, end: hi } = r; @@ -724,7 +718,7 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> { ("hi".into(), DiagnosticArgValue::Str(hi.to_string().into())), ]; let args = args.iter().map(|(a, b)| (a, b)); - let message = dcx.eagerly_translate_to_string(msg, args); + let message = err.dcx.eagerly_translate_to_string(msg, args); err.arg("in_range", message); } @@ -746,7 +740,7 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> { ExpectedKind::EnumTag => fluent::const_eval_validation_expected_enum_tag, ExpectedKind::Str => fluent::const_eval_validation_expected_str, }; - let msg = dcx.eagerly_translate_to_string(msg, [].into_iter()); + let msg = err.dcx.eagerly_translate_to_string(msg, [].into_iter()); err.arg("expected", msg); } InvalidEnumTag { value } @@ -757,11 +751,11 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> { err.arg("value", value); } NullablePtrOutOfRange { range, max_value } | PtrOutOfRange { range, max_value } => { - add_range_arg(range, max_value, dcx, err) + add_range_arg(range, max_value, err) } OutOfRange { range, max_value, value } => { err.arg("value", value); - add_range_arg(range, max_value, dcx, err); + add_range_arg(range, max_value, err); } UnalignedPtr { required_bytes, found_bytes, .. } => { err.arg("required_bytes", required_bytes); @@ -802,13 +796,13 @@ impl ReportErrorExt for UnsupportedOpInfo { UnsupportedOpInfo::ExternStatic(_) => const_eval_extern_static, } } - fn add_args(self, _: &DiagCtxt, builder: &mut DiagnosticBuilder<'_, G>) { + fn add_args(self, diag: &mut DiagnosticBuilder<'_, G>) { use crate::fluent_generated::*; use UnsupportedOpInfo::*; if let ReadPointerAsInt(_) | OverwritePartialPointer(_) | ReadPartialPointer(_) = self { - builder.help(const_eval_ptr_as_bytes_1); - builder.help(const_eval_ptr_as_bytes_2); + diag.help(const_eval_ptr_as_bytes_1); + diag.help(const_eval_ptr_as_bytes_2); } match self { // `ReadPointerAsInt(Some(info))` is never printed anyway, it only serves as an error to @@ -816,10 +810,10 @@ impl ReportErrorExt for UnsupportedOpInfo { // print. So it's not worth the effort of having diagnostics that can print the `info`. UnsizedLocal | Unsupported(_) | ReadPointerAsInt(_) => {} OverwritePartialPointer(ptr) | ReadPartialPointer(ptr) => { - builder.arg("ptr", ptr); + diag.arg("ptr", ptr); } ThreadLocalStatic(did) | ExternStatic(did) => { - builder.arg("did", format!("{did:?}")); + diag.arg("did", format!("{did:?}")); } } } @@ -835,18 +829,14 @@ impl<'tcx> ReportErrorExt for InterpError<'tcx> { InterpError::MachineStop(e) => e.diagnostic_message(), } } - fn add_args( - self, - dcx: &DiagCtxt, - builder: &mut DiagnosticBuilder<'_, G>, - ) { + fn add_args(self, diag: &mut DiagnosticBuilder<'_, G>) { match self { - InterpError::UndefinedBehavior(ub) => ub.add_args(dcx, builder), - InterpError::Unsupported(e) => e.add_args(dcx, builder), - InterpError::InvalidProgram(e) => e.add_args(dcx, builder), - InterpError::ResourceExhaustion(e) => e.add_args(dcx, builder), + InterpError::UndefinedBehavior(ub) => ub.add_args(diag), + InterpError::Unsupported(e) => e.add_args(diag), + InterpError::InvalidProgram(e) => e.add_args(diag), + InterpError::ResourceExhaustion(e) => e.add_args(diag), InterpError::MachineStop(e) => e.add_args(&mut |name, value| { - builder.arg(name, value); + diag.arg(name, value); }), } } @@ -864,28 +854,24 @@ impl<'tcx> ReportErrorExt for InvalidProgramInfo<'tcx> { } } } - fn add_args( - self, - dcx: &DiagCtxt, - builder: &mut DiagnosticBuilder<'_, G>, - ) { + fn add_args(self, diag: &mut DiagnosticBuilder<'_, G>) { match self { InvalidProgramInfo::TooGeneric | InvalidProgramInfo::AlreadyReported(_) => {} InvalidProgramInfo::Layout(e) => { - // The level doesn't matter, `diag` is consumed without it being used. + // The level doesn't matter, `dummy_diag` is consumed without it being used. let dummy_level = Level::Bug; - let diag: DiagnosticBuilder<'_, ()> = - e.into_diagnostic().into_diagnostic(dcx, dummy_level); - for (name, val) in diag.args() { - builder.arg(name.clone(), val.clone()); + let dummy_diag: DiagnosticBuilder<'_, ()> = + e.into_diagnostic().into_diagnostic(diag.dcx, dummy_level); + for (name, val) in dummy_diag.args() { + diag.arg(name.clone(), val.clone()); } - diag.cancel(); + dummy_diag.cancel(); } InvalidProgramInfo::FnAbiAdjustForForeignAbi( AdjustForForeignAbiError::Unsupported { arch, abi }, ) => { - builder.arg("arch", arch); - builder.arg("abi", abi.name()); + diag.arg("arch", arch); + diag.arg("abi", abi.name()); } } } @@ -900,7 +886,7 @@ impl ReportErrorExt for ResourceExhaustionInfo { ResourceExhaustionInfo::AddressSpaceFull => const_eval_address_space_full, } } - fn add_args(self, _: &DiagCtxt, _: &mut DiagnosticBuilder<'_, G>) {} + fn add_args(self, _: &mut DiagnosticBuilder<'_, G>) {} } impl rustc_errors::IntoDiagnosticArg for InternKind { diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index dd9dfe3fe79..ff90059203a 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -445,7 +445,7 @@ pub fn format_interp_error<'tcx>(dcx: &DiagCtxt, e: InterpErrorInfo<'tcx>) -> St #[allow(rustc::untranslatable_diagnostic)] let mut diag = dcx.struct_allow(""); let msg = e.diagnostic_message(); - e.add_args(dcx, &mut diag); + e.add_args(&mut diag); let s = dcx.eagerly_translate_to_string(msg, diag.args()); diag.cancel(); s From d4b77f64e4085f06caf5bd4ef8ae3c64d681eeff Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 12 Feb 2024 15:26:59 +1100 Subject: [PATCH 3/4] Tweak delayed bug mentions. Now that we have both `delayed_bug` and `span_delayed_bug`, it makes sense to use the generic term "delayed bug" more. --- compiler/rustc_errors/src/lib.rs | 6 +++--- compiler/rustc_feature/src/builtin_attrs.rs | 2 +- compiler/rustc_hir_analysis/src/check/wfcheck.rs | 6 +++--- .../rustc_infer/src/infer/canonical/canonicalizer.rs | 2 +- compiler/rustc_interface/src/queries.rs | 6 +++--- compiler/rustc_middle/src/query/mod.rs | 6 +++--- compiler/rustc_middle/src/ty/region.rs | 8 ++++---- compiler/rustc_middle/src/ty/visit.rs | 2 +- compiler/rustc_middle/src/util/bug.rs | 12 ++++++------ compiler/rustc_span/src/symbol.rs | 2 +- src/tools/clippy/tests/integration.rs | 2 +- .../rust-analyzer/crates/hir-def/src/attr/builtin.rs | 2 +- tests/incremental/delayed_span_bug.rs | 4 ++-- tests/ui/treat-err-as-bug/span_delayed_bug.rs | 4 ++-- tests/ui/treat-err-as-bug/span_delayed_bug.stderr | 4 ++-- 15 files changed, 34 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index fc5a8ef13f0..2a68443ee38 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -411,8 +411,8 @@ impl CodeSuggestion { /// or `.span_bug` rather than a failed assertion, etc. pub struct ExplicitBug; -/// Signifies that the compiler died with an explicit call to `.delay_*_bug` -/// rather than a failed assertion, etc. +/// Signifies that the compiler died due to a delayed bug rather than a failed +/// assertion, etc. pub struct DelayedBugPanic; /// A `DiagCtxt` deals with errors and other compiler output. @@ -1446,7 +1446,7 @@ impl DiagCtxtInner { { let _ = write!( &mut out, - "delayed span bug: {}\n{}\n", + "delayed bug: {}\n{}\n", bug.inner .messages .iter() diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 019cc1c847e..6aedd2a5e33 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -806,7 +806,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_attr!(TEST, rustc_regions, Normal, template!(Word), WarnFollowing), rustc_attr!( TEST, rustc_error, Normal, - template!(Word, List: "span_delayed_bug_from_inside_query"), WarnFollowingWordOnly + template!(Word, List: "delayed_bug_from_inside_query"), WarnFollowingWordOnly ), rustc_attr!(TEST, rustc_dump_user_args, Normal, template!(Word), WarnFollowing), rustc_attr!(TEST, rustc_evaluate_where_clauses, Normal, template!(Word), WarnFollowing), diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index 646a84b043c..1bdfa452360 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -118,9 +118,9 @@ where return Err(err); } else { // HACK(oli-obk): tests/ui/specialization/min_specialization/specialize_on_type_error.rs - // causes an error (span_delayed_bug) during normalization, without reporting an error, - // so we need to act as if no error happened, in order to let our callers continue and - // report an error later in check_impl_items_against_trait. + // causes an delayed bug during normalization, without reporting an error, so we need + // to act as if no error happened, in order to let our callers continue and report an + // error later in check_impl_items_against_trait. return Ok(()); } } diff --git a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs index 156a4f71017..99882a42abc 100644 --- a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs +++ b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs @@ -191,7 +191,7 @@ impl CanonicalizeMode for CanonicalizeQueryResponse { // // rust-lang/rust#57464: `impl Trait` can leak local // scopes (in manner violating typeck). Therefore, use - // `span_delayed_bug` to allow type error over an ICE. + // `delayed_bug` to allow type error over an ICE. canonicalizer .tcx .dcx() diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index e66ea6f2ca9..2a4eefb7f77 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -194,16 +194,16 @@ impl<'tcx> Queries<'tcx> { let Some((def_id, _)) = tcx.entry_fn(()) else { return }; for attr in tcx.get_attrs(def_id, sym::rustc_error) { match attr.meta_item_list() { - // Check if there is a `#[rustc_error(span_delayed_bug_from_inside_query)]`. + // Check if there is a `#[rustc_error(delayed_bug_from_inside_query)]`. Some(list) if list.iter().any(|list_item| { matches!( list_item.ident().map(|i| i.name), - Some(sym::span_delayed_bug_from_inside_query) + Some(sym::delayed_bug_from_inside_query) ) }) => { - tcx.ensure().trigger_span_delayed_bug(def_id); + tcx.ensure().trigger_delayed_bug(def_id); } // Bare `#[rustc_error]`. diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 938fba0ed09..5fe736c9047 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -108,9 +108,9 @@ pub use plumbing::{IntoQueryParam, TyCtxtAt, TyCtxtEnsure, TyCtxtEnsureWithValue // Queries marked with `fatal_cycle` do not need the latter implementation, // as they will raise an fatal error on query cycles instead. rustc_queries! { - /// This exists purely for testing the interactions between span_delayed_bug and incremental. - query trigger_span_delayed_bug(key: DefId) { - desc { "triggering a span delayed bug for testing incremental" } + /// This exists purely for testing the interactions between delayed bugs and incremental. + query trigger_delayed_bug(key: DefId) { + desc { "triggering a delayed bug for testing incremental" } } /// Collects the list of all tools registered using `#![register_tool]`. diff --git a/compiler/rustc_middle/src/ty/region.rs b/compiler/rustc_middle/src/ty/region.rs index 75be380704e..1191d7fca32 100644 --- a/compiler/rustc_middle/src/ty/region.rs +++ b/compiler/rustc_middle/src/ty/region.rs @@ -82,8 +82,8 @@ impl<'tcx> Region<'tcx> { tcx.intern_region(ty::ReError(reported)) } - /// Constructs a `RegionKind::ReError` region and registers a `span_delayed_bug` to ensure it - /// gets used. + /// Constructs a `RegionKind::ReError` region and registers a delayed bug to ensure it gets + /// used. #[track_caller] pub fn new_error_misc(tcx: TyCtxt<'tcx>) -> Region<'tcx> { Region::new_error_with_message( @@ -93,8 +93,8 @@ impl<'tcx> Region<'tcx> { ) } - /// Constructs a `RegionKind::ReError` region and registers a `span_delayed_bug` with the given - /// `msg` to ensure it gets used. + /// Constructs a `RegionKind::ReError` region and registers a delayed bug with the given `msg` + /// to ensure it gets used. #[track_caller] pub fn new_error_with_message>( tcx: TyCtxt<'tcx>, diff --git a/compiler/rustc_middle/src/ty/visit.rs b/compiler/rustc_middle/src/ty/visit.rs index a750a86d257..7acdb931f1a 100644 --- a/compiler/rustc_middle/src/ty/visit.rs +++ b/compiler/rustc_middle/src/ty/visit.rs @@ -55,7 +55,7 @@ pub trait TypeVisitableExt<'tcx>: TypeVisitable> { } fn error_reported(&self) -> Result<(), ErrorGuaranteed> { if self.references_error() { - // We must include lint errors and span delayed bugs here. + // We must include lint errors and delayed bugs here. if let Some(reported) = ty::tls::with(|tcx| tcx.dcx().has_errors_or_lint_errors_or_delayed_bugs()) { diff --git a/compiler/rustc_middle/src/util/bug.rs b/compiler/rustc_middle/src/util/bug.rs index d1a3dbbf5d3..a67ec991582 100644 --- a/compiler/rustc_middle/src/util/bug.rs +++ b/compiler/rustc_middle/src/util/bug.rs @@ -38,16 +38,16 @@ fn opt_span_bug_fmt>( }) } -/// A query to trigger a `span_delayed_bug`. Clearly, if one has a `tcx` one can already trigger a -/// `span_delayed_bug`, so what is the point of this? It exists to help us test `span_delayed_bug`'s -/// interactions with the query system and incremental. -pub fn trigger_span_delayed_bug(tcx: TyCtxt<'_>, key: rustc_hir::def_id::DefId) { +/// A query to trigger a delayed bug. Clearly, if one has a `tcx` one can already trigger a +/// delayed bug, so what is the point of this? It exists to help us test the interaction of delayed +/// bugs with the query system and incremental. +pub fn trigger_delayed_bug(tcx: TyCtxt<'_>, key: rustc_hir::def_id::DefId) { tcx.dcx().span_delayed_bug( tcx.def_span(key), - "delayed span bug triggered by #[rustc_error(span_delayed_bug_from_inside_query)]", + "delayed bug triggered by #[rustc_error(delayed_bug_from_inside_query)]", ); } pub fn provide(providers: &mut crate::query::Providers) { - *providers = crate::query::Providers { trigger_span_delayed_bug, ..*providers }; + *providers = crate::query::Providers { trigger_delayed_bug, ..*providers }; } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index aa912c93c08..83eca6c032b 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -655,6 +655,7 @@ symbols! { default_method_body_is_const, default_type_parameter_fallback, default_type_params, + delayed_bug_from_inside_query, deny, deprecated, deprecated_safe, @@ -1579,7 +1580,6 @@ symbols! { slice_patterns, slicing_syntax, soft, - span_delayed_bug_from_inside_query, specialization, speed, spotlight, diff --git a/src/tools/clippy/tests/integration.rs b/src/tools/clippy/tests/integration.rs index 267f095f9c2..7f4500826ff 100644 --- a/src/tools/clippy/tests/integration.rs +++ b/src/tools/clippy/tests/integration.rs @@ -77,7 +77,7 @@ fn integration_test() { // the repo basically just contains a span_delayed_bug that forces rustc/clippy to panic: /* #![feature(rustc_attrs)] - #[rustc_error(span_delayed_bug_from_inside_query)] + #[rustc_error(delayed_bug_from_inside_query)] fn main() {} */ diff --git a/src/tools/rust-analyzer/crates/hir-def/src/attr/builtin.rs b/src/tools/rust-analyzer/crates/hir-def/src/attr/builtin.rs index b20ee9e5bf6..55b9a1dfdcb 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/attr/builtin.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/attr/builtin.rs @@ -650,7 +650,7 @@ pub const INERT_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_attr!(TEST, rustc_regions, Normal, template!(Word), WarnFollowing), rustc_attr!( TEST, rustc_error, Normal, - template!(Word, List: "span_delayed_bug_from_inside_query"), WarnFollowingWordOnly + template!(Word, List: "delayed_bug_from_inside_query"), WarnFollowingWordOnly ), rustc_attr!(TEST, rustc_dump_user_args, Normal, template!(Word), WarnFollowing), rustc_attr!(TEST, rustc_evaluate_where_clauses, Normal, template!(Word), WarnFollowing), diff --git a/tests/incremental/delayed_span_bug.rs b/tests/incremental/delayed_span_bug.rs index cc9831fff96..5c9d3c2c1d5 100644 --- a/tests/incremental/delayed_span_bug.rs +++ b/tests/incremental/delayed_span_bug.rs @@ -1,8 +1,8 @@ // revisions: cfail1 cfail2 // should-ice -// error-pattern: delayed span bug triggered by #[rustc_error(span_delayed_bug_from_inside_query)] +// error-pattern: delayed bug triggered by #[rustc_error(delayed_bug_from_inside_query)] #![feature(rustc_attrs)] -#[rustc_error(span_delayed_bug_from_inside_query)] +#[rustc_error(delayed_bug_from_inside_query)] fn main() {} diff --git a/tests/ui/treat-err-as-bug/span_delayed_bug.rs b/tests/ui/treat-err-as-bug/span_delayed_bug.rs index 8b9526bf3f9..1ea14aee6c4 100644 --- a/tests/ui/treat-err-as-bug/span_delayed_bug.rs +++ b/tests/ui/treat-err-as-bug/span_delayed_bug.rs @@ -1,12 +1,12 @@ // compile-flags: -Ztreat-err-as-bug -Zeagerly-emit-delayed-bugs // 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: [trigger_delayed_bug] triggering a delayed bug for testing incremental // normalize-stderr-test "note: .*\n\n" -> "" // normalize-stderr-test "thread 'rustc' panicked.*:\n.*\n" -> "" // rustc-env:RUST_BACKTRACE=0 #![feature(rustc_attrs)] -#[rustc_error(span_delayed_bug_from_inside_query)] +#[rustc_error(delayed_bug_from_inside_query)] fn main() {} diff --git a/tests/ui/treat-err-as-bug/span_delayed_bug.stderr b/tests/ui/treat-err-as-bug/span_delayed_bug.stderr index a61ffaea8c2..f0e8cd0ddb9 100644 --- a/tests/ui/treat-err-as-bug/span_delayed_bug.stderr +++ b/tests/ui/treat-err-as-bug/span_delayed_bug.stderr @@ -1,4 +1,4 @@ -error: internal compiler error: delayed span bug triggered by #[rustc_error(span_delayed_bug_from_inside_query)] +error: internal compiler error: delayed bug triggered by #[rustc_error(delayed_bug_from_inside_query)] --> $DIR/span_delayed_bug.rs:12:1 | LL | fn main() {} @@ -7,5 +7,5 @@ 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 +#0 [trigger_delayed_bug] triggering a delayed bug for testing incremental end of query stack From 1f39c8b08f2a9d30916a77beaf518f8d58d13a62 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 12 Feb 2024 16:34:39 +1100 Subject: [PATCH 4/4] Change level used in `print_error_count`. From `Fatal` to `Error`. It has no functional effect, but `Error` makes more sense and lines up better with the `Warning` level used just above. --- compiler/rustc_errors/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 2a68443ee38..16077b3d98f 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -993,10 +993,10 @@ impl DiagCtxt { .emit_diagnostic(Diagnostic::new(Warning, DiagnosticMessage::Str(warnings))); } (_, 0) => { - inner.emit_diagnostic(Diagnostic::new(Fatal, errors)); + inner.emit_diagnostic(Diagnostic::new(Error, errors)); } (_, _) => { - inner.emit_diagnostic(Diagnostic::new(Fatal, format!("{errors}; {warnings}"))); + inner.emit_diagnostic(Diagnostic::new(Error, format!("{errors}; {warnings}"))); } }