Skip to content

Change api of formatting diagnostic attribute strings. #154858

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
rust-bors merged 2 commits into from
Apr 6, 2026
Merged

Change api of formatting diagnostic attribute strings. #154858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c97a92
Force all diagnostic attr formatting through `eval`.
mejrs Apr 5, 2026
7791ac7
Rename `OnUnimplementedNote` to `CustomDiagnostic`
mejrs Apr 5, 2026
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
24 changes: 12 additions & 12 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@ use std::ops::ControlFlow;
use either::Either;
use hir::{ClosureKind, Path};
use rustc_data_structures::fx::FxIndexSet;
use rustc_data_structures::thin_vec::ThinVec;
use rustc_errors::codes::*;
use rustc_errors::{Applicability, Diag, MultiSpan, struct_span_code_err};
use rustc_hir as hir;
use rustc_hir::attrs::diagnostic::FormatArgs;
use rustc_hir::attrs::diagnostic::{CustomDiagnostic, FormatArgs};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::{Visitor, walk_block, walk_expr};
use rustc_hir::{
Expand Down Expand Up @@ -146,7 +145,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
self.body.local_decls[moved_place.local].ty.kind()
&& let Some(Some(directive)) = find_attr!(self.infcx.tcx, item_def.did(), OnMove { directive, .. } => directive)
{
let item_name = self.infcx.tcx.item_name(item_def.did()).to_string();
let this = self.infcx.tcx.item_name(item_def.did()).to_string();
let mut generic_args: Vec<_> = self
.infcx
.tcx
Expand All @@ -155,21 +154,22 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
.iter()
.filter_map(|param| Some((param.name, args[param.index as usize].to_string())))
.collect();
generic_args.push((kw::SelfUpper, item_name));
generic_args.push((kw::SelfUpper, this.clone()));

let args = FormatArgs {
this: String::new(),
trait_sugared: String::new(),
this,
// Unused
this_sugared: String::new(),
// Unused
item_context: "",
generic_args,
};
(
directive.message.as_ref().map(|e| e.1.format(&args)),
directive.label.as_ref().map(|e| e.1.format(&args)),
directive.notes.iter().map(|e| e.format(&args)).collect(),
)
let CustomDiagnostic { message, label, notes, parent_label: _ } =
directive.eval(None, &args);

(message, label, notes)
} else {
(None, None, ThinVec::new())
(None, None, Vec::new())
};

let mut err = self.cannot_act_on_moved_value(
Expand Down
48 changes: 32 additions & 16 deletions compiler/rustc_hir/src/attrs/diagnostic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,30 +52,40 @@ impl Directive {
}
}

pub fn evaluate_directive(
pub fn eval(
&self,
trait_name: impl Debug,
condition_options: &ConditionOptions,
condition_options: Option<&ConditionOptions>,
args: &FormatArgs,
) -> OnUnimplementedNote {
) -> CustomDiagnostic {
let this = &args.this;
info!("eval({self:?}, this={this}, options={condition_options:?}, args ={args:?})");
Copy link
Copy Markdown
Member

@jieyouxu jieyouxu Apr 6, 2026

Choose a reason for hiding this comment

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

Question: does this need to be info! instead of say, debug!?

Copy link
Copy Markdown
Contributor

@JonathanBrouwer JonathanBrouwer Apr 6, 2026

Choose a reason for hiding this comment

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

Hmmm yeah debug! is definitely more appropriate here, oops
@mejrs could you open a new pr to change it?

Copy link
Copy Markdown
Contributor Author

@mejrs mejrs Apr 6, 2026

Choose a reason for hiding this comment

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

Sure

Copy link
Copy Markdown
Contributor Author

@mejrs mejrs Apr 6, 2026

Choose a reason for hiding this comment

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

#154898


let Some(condition_options) = condition_options else {
debug_assert!(
!self.is_rustc_attr,
"Directive::eval called for `rustc_on_unimplemented` without `condition_options`"
);
return CustomDiagnostic {
label: self.label.as_ref().map(|l| l.1.format(args)),
message: self.message.as_ref().map(|m| m.1.format(args)),
notes: self.notes.iter().map(|n| n.format(args)).collect(),
parent_label: None,
};
};
let mut message = None;
let mut label = None;
let mut notes = Vec::new();
let mut parent_label = None;
info!(
"evaluate_directive({:?}, trait_ref={:?}, options={:?}, args ={:?})",
self, trait_name, condition_options, args
);

for command in self.subcommands.iter().chain(Some(self)).rev() {
debug!(?command);
if let Some(ref condition) = command.condition
&& !condition.matches_predicate(condition_options)
{
debug!("evaluate_directive: skipping {:?} due to condition", command);
debug!("eval: skipping {command:?} due to condition");
continue;
}
debug!("evaluate_directive: {:?} succeeded", command);
debug!("eval: {command:?} succeeded");
if let Some(ref message_) = command.message {
message = Some(message_.clone());
}
Expand All @@ -91,7 +101,7 @@ impl Directive {
}
}

OnUnimplementedNote {
CustomDiagnostic {
label: label.map(|l| l.1.format(args)),
message: message.map(|m| m.1.format(args)),
notes: notes.into_iter().map(|n| n.format(args)).collect(),
Expand All @@ -100,8 +110,9 @@ impl Directive {
}
}

/// A custom diagnostic, created from a diagnostic attribute.
#[derive(Default, Debug)]
pub struct OnUnimplementedNote {
pub struct CustomDiagnostic {
pub message: Option<String>,
pub label: Option<String>,
pub notes: Vec<String>,
Expand All @@ -116,8 +127,13 @@ pub struct FormatString {
pub span: Span,
pub pieces: ThinVec<Piece>,
}

impl FormatString {
pub fn format(&self, args: &FormatArgs) -> String {
/// Formats the format string.
///
/// This is a private method, use `Directive::eval` instead. A diagnostic attribute being used
/// should issue a `tracing` event, which `Directive::eval` does.
fn format(&self, args: &FormatArgs) -> String {
let mut ret = String::new();
for piece in &self.pieces {
match piece {
Expand Down Expand Up @@ -147,7 +163,7 @@ impl FormatString {
// It's only `rustc_onunimplemented` from here
Piece::Arg(FormatArg::This) => ret.push_str(&args.this),
Piece::Arg(FormatArg::Trait) => {
let _ = fmt::write(&mut ret, format_args!("{}", &args.trait_sugared));
let _ = fmt::write(&mut ret, format_args!("{}", &args.this_sugared));
}
Piece::Arg(FormatArg::ItemContext) => ret.push_str(args.item_context),
}
Expand Down Expand Up @@ -193,15 +209,15 @@ impl FormatString {
/// ```rust,ignore (just an example)
/// FormatArgs {
/// this: "FromResidual",
/// trait_sugared: "FromResidual<Option<Infallible>>",
/// this_sugared: "FromResidual<Option<Infallible>>",
/// item_context: "an async function",
/// generic_args: [("Self", "u32"), ("R", "Option<Infallible>")],
/// }
/// ```
#[derive(Debug)]
pub struct FormatArgs {
pub this: String,
pub trait_sugared: String,
pub this_sugared: String,
pub item_context: &'static str,
pub generic_args: Vec<(Symbol, String)>,
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc_errors::codes::*;
use rustc_errors::{
Applicability, Diag, MultiSpan, StashKey, StringPart, listify, pluralize, struct_span_code_err,
};
use rustc_hir::attrs::diagnostic::OnUnimplementedNote;
use rustc_hir::attrs::diagnostic::CustomDiagnostic;
use rustc_hir::def::{CtorKind, DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{self, Visitor};
Expand Down Expand Up @@ -2067,7 +2067,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Avoid crashing.
return (None, None, Vec::new());
}
let OnUnimplementedNote { message, label, notes, .. } = self
let CustomDiagnostic { message, label, notes, .. } = self
.err_ctxt()
.on_unimplemented_note(trait_ref, &obligation, err.long_ty_path());
(message, label, notes)
Expand Down
11 changes: 5 additions & 6 deletions compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_errors::{
Applicability, Diag, ErrorGuaranteed, Level, MultiSpan, StashKey, StringPart, Suggestions, msg,
pluralize, struct_span_code_err,
};
use rustc_hir::attrs::diagnostic::OnUnimplementedNote;
use rustc_hir::attrs::diagnostic::CustomDiagnostic;
use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
use rustc_hir::intravisit::Visitor;
use rustc_hir::{self as hir, LangItem, Node, find_attr};
Expand Down Expand Up @@ -188,7 +188,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
})
.unwrap_or_default();

let OnUnimplementedNote {
let CustomDiagnostic {
message,
label,
notes,
Expand Down Expand Up @@ -907,12 +907,11 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
);

if let Some(command) = find_attr!(self.tcx, impl_did, OnConst {directive, ..} => directive.as_deref()).flatten(){
let note = command.evaluate_directive(
predicate.skip_binder().trait_ref,
&condition_options,
let note = command.eval(
Some(&condition_options),
&format_args,
);
let OnUnimplementedNote {
let CustomDiagnostic {
message,
label,
notes,
Expand Down
17 changes: 8 additions & 9 deletions compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::path::PathBuf;

use rustc_hir as hir;
use rustc_hir::attrs::diagnostic::{ConditionOptions, FormatArgs, OnUnimplementedNote};
use rustc_hir::attrs::diagnostic::{ConditionOptions, CustomDiagnostic, FormatArgs};
use rustc_hir::def_id::LocalDefId;
use rustc_hir::find_attr;
pub use rustc_hir::lints::FormatWarning;
Expand Down Expand Up @@ -37,20 +37,19 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
trait_pred: ty::PolyTraitPredicate<'tcx>,
obligation: &PredicateObligation<'tcx>,
long_ty_path: &mut Option<PathBuf>,
) -> OnUnimplementedNote {
) -> CustomDiagnostic {
if trait_pred.polarity() != ty::PredicatePolarity::Positive {
return OnUnimplementedNote::default();
return CustomDiagnostic::default();
}
let (condition_options, format_args) =
self.on_unimplemented_components(trait_pred, obligation, long_ty_path);
if let Some(command) = find_attr!(self.tcx, trait_pred.def_id(), OnUnimplemented {directive, ..} => directive.as_deref()).flatten() {
command.evaluate_directive(
trait_pred.skip_binder().trait_ref,
&condition_options,
command.eval(
Some(&condition_options),
&format_args,
)
} else {
OnUnimplementedNote::default()
CustomDiagnostic::default()
}
}

Expand Down Expand Up @@ -211,7 +210,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
}));

let this = self.tcx.def_path_str(trait_pred.trait_ref.def_id);
let trait_sugared = trait_pred.trait_ref.print_trait_sugared().to_string();
let this_sugared = trait_pred.trait_ref.print_trait_sugared().to_string();

let condition_options = ConditionOptions {
self_types,
Expand Down Expand Up @@ -249,7 +248,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
})
.collect();

let format_args = FormatArgs { this, trait_sugared, generic_args, item_context };
let format_args = FormatArgs { this, this_sugared, generic_args, item_context };
(condition_options, format_args)
}
}
Loading