Skip to content

Commit

Permalink
Reduce capabilities of Diagnostic.
Browse files Browse the repository at this point in the history
Currently many diagnostic modifier methods are available on both
`Diagnostic` and `DiagnosticBuilder`. This commit removes most of them
from `Diagnostic`. To minimize the diff size, it keeps them within
`diagnostic.rs` but changes the surrounding `impl Diagnostic` block to
`impl DiagnosticBuilder`. (I intend to move things around later, to give
a more sensible code layout.)

`Diagnostic` keeps a few methods that it still needs, like `sub`,
`arg`, and `replace_args`.

The `forward!` macro, which defined two additional methods per call
(e.g. `note` and `with_note`), is replaced by the `with_fn!` macro,
which defines one additional method per call (e.g. `with_note`). It's
now also only used when necessary -- not all modifier methods currently
need a `with_*` form. (New ones can be easily added as necessary.)

All this also requires changing `trait AddToDiagnostic` so its methods
take `DiagnosticBuilder` instead of `Diagnostic`, which leads to many
mechanical changes. `SubdiagnosticMessageOp` gains a type parameter `G`.

There are three subdiagnostics -- `DelayedAtWithoutNewline`,
`DelayedAtWithNewline`, and `InvalidFlushedDelayedDiagnosticLevel` --
that are created within the diagnostics machinery and appended to
external diagnostics. These are handled at the `Diagnostic` level, which
means it's now hard to construct them via `derive(Diagnostic)`, so
instead we construct them by hand. This has no effect on what they look
like when printed.

There are lots of new `allow` markers for `untranslatable_diagnostics`
and `diagnostics_outside_of_impl`. This is because
`#[rustc_lint_diagnostics]` annotations were present on the `Diagnostic`
modifier methods, but missing from the `DiagnosticBuilder` modifier
methods. They're now present.
  • Loading branch information
nnethercote committed Feb 20, 2024
1 parent b18f3e1 commit f6f8779
Show file tree
Hide file tree
Showing 40 changed files with 502 additions and 395 deletions.
9 changes: 7 additions & 2 deletions compiler/rustc_ast_lowering/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use rustc_errors::{
codes::*, AddToDiagnostic, Diagnostic, DiagnosticArgFromDisplay, SubdiagnosticMessageOp,
codes::*, AddToDiagnostic, DiagnosticArgFromDisplay, DiagnosticBuilder, EmissionGuarantee,
SubdiagnosticMessageOp,
};
use rustc_macros::{Diagnostic, Subdiagnostic};
use rustc_span::{symbol::Ident, Span, Symbol};
Expand Down Expand Up @@ -41,7 +42,11 @@ pub struct InvalidAbi {
pub struct InvalidAbiReason(pub &'static str);

impl AddToDiagnostic for InvalidAbiReason {
fn add_to_diagnostic_with<F: SubdiagnosticMessageOp>(self, diag: &mut Diagnostic, _: F) {
fn add_to_diagnostic_with<G: EmissionGuarantee, F: SubdiagnosticMessageOp<G>>(
self,
diag: &mut DiagnosticBuilder<'_, G>,
_: F,
) {
#[allow(rustc::untranslatable_diagnostic)]
diag.note(self.0);
}
Expand Down
17 changes: 14 additions & 3 deletions compiler/rustc_ast_passes/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
//! Errors emitted by ast_passes.
use rustc_ast::ParamKindOrd;
use rustc_errors::{codes::*, AddToDiagnostic, Applicability, Diagnostic, SubdiagnosticMessageOp};
use rustc_errors::{
codes::*, AddToDiagnostic, Applicability, DiagnosticBuilder, EmissionGuarantee,
SubdiagnosticMessageOp,
};
use rustc_macros::{Diagnostic, Subdiagnostic};
use rustc_span::{symbol::Ident, Span, Symbol};

Expand Down Expand Up @@ -372,7 +375,11 @@ pub struct EmptyLabelManySpans(pub Vec<Span>);

// The derive for `Vec<Span>` does multiple calls to `span_label`, adding commas between each
impl AddToDiagnostic for EmptyLabelManySpans {
fn add_to_diagnostic_with<F: SubdiagnosticMessageOp>(self, diag: &mut Diagnostic, _: F) {
fn add_to_diagnostic_with<G: EmissionGuarantee, F: SubdiagnosticMessageOp<G>>(
self,
diag: &mut DiagnosticBuilder<'_, G>,
_: F,
) {
diag.span_labels(self.0, "");
}
}
Expand Down Expand Up @@ -729,7 +736,11 @@ pub struct StableFeature {
}

impl AddToDiagnostic for StableFeature {
fn add_to_diagnostic_with<F: SubdiagnosticMessageOp>(self, diag: &mut Diagnostic, _: F) {
fn add_to_diagnostic_with<G: EmissionGuarantee, F: SubdiagnosticMessageOp<G>>(
self,
diag: &mut DiagnosticBuilder<'_, G>,
_: F,
) {
diag.arg("name", self.name);
diag.arg("since", self.since);
diag.help(fluent::ast_passes_stable_since);
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ macro_rules! gate {
}};
($visitor:expr, $feature:ident, $span:expr, $explain:expr, $help:expr) => {{
if !$visitor.features.$feature && !$span.allows_unstable(sym::$feature) {
// FIXME: make this translatable
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
feature_err(&$visitor.sess, sym::$feature, $span, $explain).with_help($help).emit();
}
}};
Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_borrowck/src/diagnostics/region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {

hrtb_bounds.iter().for_each(|bound| {
let Trait(PolyTraitRef { trait_ref, span: trait_span, .. }, _) = bound else { return; };
// FIXME: make this translatable
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
diag.span_note(
*trait_span,
"due to current limitations in the borrow checker, this implies a `'static` lifetime"
Expand Down Expand Up @@ -421,6 +424,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
/// ```
///
/// Here we would be invoked with `fr = 'a` and `outlived_fr = 'b`.
// FIXME: make this translatable
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
pub(crate) fn report_region_error(
&mut self,
fr: RegionVid,
Expand Down Expand Up @@ -685,12 +691,18 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
borrowck_errors::borrowed_data_escapes_closure(self.infcx.tcx, *span, escapes_from);

if let Some((Some(outlived_fr_name), outlived_fr_span)) = outlived_fr_name_and_span {
// FIXME: make this translatable
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
diag.span_label(
outlived_fr_span,
format!("`{outlived_fr_name}` declared here, outside of the {escapes_from} body",),
);
}

// FIXME: make this translatable
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
if let Some((Some(fr_name), fr_span)) = fr_name_and_span {
diag.span_label(
fr_span,
Expand All @@ -714,6 +726,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
let outlived_fr_region_name = self.give_region_a_name(errci.outlived_fr).unwrap();
outlived_fr_region_name.highlight_region_name(&mut diag);

// FIXME: make this translatable
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
diag.span_label(
*span,
format!(
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2497,6 +2497,8 @@ mod diags {
}
for (_, (mut diag, count)) in std::mem::take(&mut self.diags.buffered_mut_errors) {
if count > 10 {
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
diag.note(format!("...and {} other attempted mutable borrows", count - 10));
}
self.diags.buffered_diags.push(BufferedDiag::Error(diag));
Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_builtin_macros/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustc_errors::{
codes::*, AddToDiagnostic, DiagCtxt, Diagnostic, DiagnosticBuilder, EmissionGuarantee,
IntoDiagnostic, Level, MultiSpan, SingleLabelManySpans, SubdiagnosticMessageOp,
codes::*, AddToDiagnostic, DiagCtxt, DiagnosticBuilder, EmissionGuarantee, IntoDiagnostic,
Level, MultiSpan, SingleLabelManySpans, SubdiagnosticMessageOp,
};
use rustc_macros::{Diagnostic, Subdiagnostic};
use rustc_span::{symbol::Ident, Span, Symbol};
Expand Down Expand Up @@ -611,7 +611,11 @@ pub(crate) struct FormatUnusedArg {
// Allow the singular form to be a subdiagnostic of the multiple-unused
// form of diagnostic.
impl AddToDiagnostic for FormatUnusedArg {
fn add_to_diagnostic_with<F: SubdiagnosticMessageOp>(self, diag: &mut Diagnostic, f: F) {
fn add_to_diagnostic_with<G: EmissionGuarantee, F: SubdiagnosticMessageOp<G>>(
self,
diag: &mut DiagnosticBuilder<'_, G>,
f: F,
) {
diag.arg("named", self.named);
let msg = f(diag, crate::fluent_generated::builtin_macros_format_unused_arg.into());
diag.span_label(self.span, msg);
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1856,9 +1856,7 @@ impl SharedEmitterMain {
Ok(SharedEmitterMessage::Diagnostic(diag)) => {
let dcx = sess.dcx();
let mut d = rustc_errors::Diagnostic::new_with_messages(diag.lvl, diag.msgs);
if let Some(code) = diag.code {
d.code(code);
}
d.code = diag.code; // may be `None`, that's ok
d.replace_args(diag.args);
dcx.emit_diagnostic(d);
}
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_const_eval/src/transform/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ pub struct FnCallNonConst<'tcx> {
}

impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
// FIXME: make this translatable
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, _: Span) -> DiagnosticBuilder<'tcx> {
let FnCallNonConst { caller, callee, args, span, call_source, feature } = *self;
let ConstCx { tcx, param_env, .. } = *ccx;
Expand Down Expand Up @@ -321,6 +324,8 @@ impl<'tcx> NonConstOp<'tcx> for FnCallUnstable {
.dcx()
.create_err(errors::UnstableConstFn { span, def_path: ccx.tcx.def_path_str(def_id) });

// FIXME: make this translatable
#[allow(rustc::untranslatable_diagnostic)]
if ccx.is_const_stable_const_fn() {
err.help("const-stable functions can only call other const-stable functions");
} else if ccx.tcx.sess.is_nightly_build() {
Expand Down Expand Up @@ -591,6 +596,8 @@ impl<'tcx> NonConstOp<'tcx> for StaticAccess {
span,
format!("referencing statics in {}s is unstable", ccx.const_kind(),),
);
// FIXME: make this translatable
#[allow(rustc::untranslatable_diagnostic)]
err
.note("`static` and `const` variables can refer to other `const` variables. A `const` variable, however, cannot refer to a `static` variable.")
.help("to fix this, the value can be extracted to a `const` and then used.");
Expand Down
Loading

0 comments on commit f6f8779

Please sign in to comment.