Skip to content

Commit 63c5a84

Browse files
authored
Rollup merge of #142724 - xizheyin:avoid_overwrite_args, r=oli-obk
Add runtime check to avoid overwrite arg in `Diag` ## Origin PR description At first, I set up a `debug_assert` check for the arg method to make sure that `args` in `Diag` aren't easily overwritten, and I added the `remove_arg()` method, so that if you do need to overwrite an arg, then you can explicitly call `remove_arg()` to remove it first, then call `arg()` to overwrite it. For the code before the #142015 change, it won't compile because it will report an error ``` arg `instance`already exists. ``` This PR also modifies all diagnostics that fail the check to pass the check. There are two cases of check failure: 1. ~~Between *the parent diagnostic and the subdiagnostic*, or *between the subdiagnostics* have the same field between them. In this case, I renamed the conflicting fields.~~ 2. ~~For subdiagnostics stored in `Vec`, the rendering may iteratively write the same arg over and over again. In this case, I changed the auto-generation with `derive(SubDiagnostic)` to manually implementing `SubDiagnostic` and manually rendered it with `eagerly_translate()`, similar to #142031 (comment), and after rendering it I manually deleted useless arg with the newly added `remove_arg` method.~~ ## Final Decision After trying and discussing, we made a final decision. For `#[derive(Subdiagnostic)]`, This PR made two changes: 1. After the subdiagnostic is rendered, remove all args of this subdiagnostic, which allows for usage like `Vec<Subdiag>`. 2. Store `diag.args` before setting arguments, so that you can restore the contents of the main diagnostic after deleting the arguments after subdiagnostic is rendered, to avoid deleting the main diagnostic's arg when they have the same name args.
2 parents 131a2e4 + d2d17c6 commit 63c5a84

File tree

22 files changed

+119
-39
lines changed

22 files changed

+119
-39
lines changed

compiler/rustc_borrowck/src/diagnostics/mod.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1284,8 +1284,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
12841284
&& !spans.is_empty()
12851285
{
12861286
let mut span: MultiSpan = spans.clone().into();
1287+
err.arg("ty", param_ty.to_string());
1288+
let msg = err.dcx.eagerly_translate_to_string(
1289+
fluent::borrowck_moved_a_fn_once_in_call_def,
1290+
err.args.iter(),
1291+
);
1292+
err.remove_arg("ty");
12871293
for sp in spans {
1288-
span.push_span_label(sp, fluent::borrowck_moved_a_fn_once_in_call_def);
1294+
span.push_span_label(sp, msg.clone());
12891295
}
12901296
span.push_span_label(
12911297
fn_call_span,

compiler/rustc_builtin_macros/src/errors.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,7 @@ impl Subdiagnostic for FormatUnusedArg {
660660
fn add_to_diag<G: EmissionGuarantee>(self, diag: &mut Diag<'_, G>) {
661661
diag.arg("named", self.named);
662662
let msg = diag.eagerly_translate(crate::fluent_generated::builtin_macros_format_unused_arg);
663+
diag.remove_arg("named");
663664
diag.span_label(self.span, msg);
664665
}
665666
}

compiler/rustc_const_eval/src/errors.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,9 @@ impl Subdiagnostic for FrameNote {
291291
span.push_span_label(self.span, fluent::const_eval_frame_note_last);
292292
}
293293
let msg = diag.eagerly_translate(fluent::const_eval_frame_note);
294+
diag.remove_arg("times");
295+
diag.remove_arg("where_");
296+
diag.remove_arg("instance");
294297
diag.span_note(span, msg);
295298
}
296299
}

compiler/rustc_errors/src/diagnostic.rs

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,9 @@ pub struct DiagInner {
289289
pub suggestions: Suggestions,
290290
pub args: DiagArgMap,
291291

292+
// This is used to store args and restore them after a subdiagnostic is rendered.
293+
pub reserved_args: DiagArgMap,
294+
292295
/// This is not used for highlighting or rendering any error message. Rather, it can be used
293296
/// as a sort key to sort a buffer of diagnostics. By default, it is the primary span of
294297
/// `span` if there is one. Otherwise, it is `DUMMY_SP`.
@@ -319,6 +322,7 @@ impl DiagInner {
319322
children: vec![],
320323
suggestions: Suggestions::Enabled(vec![]),
321324
args: Default::default(),
325+
reserved_args: Default::default(),
322326
sort_span: DUMMY_SP,
323327
is_lint: None,
324328
long_ty_path: None,
@@ -390,7 +394,27 @@ impl DiagInner {
390394
}
391395

392396
pub(crate) fn arg(&mut self, name: impl Into<DiagArgName>, arg: impl IntoDiagArg) {
393-
self.args.insert(name.into(), arg.into_diag_arg(&mut self.long_ty_path));
397+
let name = name.into();
398+
let value = arg.into_diag_arg(&mut self.long_ty_path);
399+
// This assertion is to avoid subdiagnostics overwriting an existing diagnostic arg.
400+
debug_assert!(
401+
!self.args.contains_key(&name) || self.args.get(&name) == Some(&value),
402+
"arg {} already exists",
403+
name
404+
);
405+
self.args.insert(name, value);
406+
}
407+
408+
pub fn remove_arg(&mut self, name: &str) {
409+
self.args.swap_remove(name);
410+
}
411+
412+
pub fn store_args(&mut self) {
413+
self.reserved_args = self.args.clone();
414+
}
415+
416+
pub fn restore_args(&mut self) {
417+
self.args = std::mem::take(&mut self.reserved_args);
394418
}
395419

396420
/// Fields used for Hash, and PartialEq trait.
@@ -1423,6 +1447,12 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
14231447
self.downgrade_to_delayed_bug();
14241448
self.emit()
14251449
}
1450+
1451+
pub fn remove_arg(&mut self, name: &str) {
1452+
if let Some(diag) = self.diag.as_mut() {
1453+
diag.remove_arg(name);
1454+
}
1455+
}
14261456
}
14271457

14281458
/// Destructor bomb: every `Diag` must be consumed (emitted, cancelled, etc.)

compiler/rustc_hir_analysis/src/errors.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ pub(crate) enum AssocItemNotFoundSugg<'a> {
127127
SimilarInOtherTrait {
128128
#[primary_span]
129129
span: Span,
130+
trait_name: &'a str,
130131
assoc_kind: &'static str,
131132
suggested_name: Symbol,
132133
},

compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
309309
// change the associated item.
310310
err.sugg = Some(errors::AssocItemNotFoundSugg::SimilarInOtherTrait {
311311
span: assoc_ident.span,
312+
trait_name: &trait_name,
312313
assoc_kind: assoc_kind_str,
313314
suggested_name,
314315
});

compiler/rustc_lint/src/levels.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,8 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
482482
let name = lint_name.as_str();
483483
let suggestion = RenamedLintSuggestion::WithoutSpan { replace };
484484
let requested_level = RequestedLevel { level, lint_name };
485-
let lint = RenamedLintFromCommandLine { name, suggestion, requested_level };
485+
let lint =
486+
RenamedLintFromCommandLine { name, replace, suggestion, requested_level };
486487
self.emit_lint(RENAMED_AND_REMOVED_LINTS, lint);
487488
}
488489
CheckLintNameResult::Removed(ref reason) => {
@@ -824,7 +825,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
824825
RenamedLintSuggestion::WithSpan { suggestion: sp, replace };
825826
let name =
826827
tool_ident.map(|tool| format!("{tool}::{name}")).unwrap_or(name);
827-
let lint = RenamedLint { name: name.as_str(), suggestion };
828+
let lint = RenamedLint { name: name.as_str(), replace, suggestion };
828829
self.emit_span_lint(RENAMED_AND_REMOVED_LINTS, sp.into(), lint);
829830
}
830831

compiler/rustc_lint/src/lifetime_syntax.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,12 +422,12 @@ fn build_mismatch_suggestion(
422422
lifetime_name: &str,
423423
infos: &[&Info<'_>],
424424
) -> lints::MismatchedLifetimeSyntaxesSuggestion {
425-
let lifetime_name = lifetime_name.to_owned();
425+
let lifetime_name_sugg = lifetime_name.to_owned();
426426

427427
let suggestions = infos.iter().map(|info| info.suggestion(&lifetime_name)).collect();
428428

429429
lints::MismatchedLifetimeSyntaxesSuggestion::Explicit {
430-
lifetime_name,
430+
lifetime_name_sugg,
431431
suggestions,
432432
tool_only: false,
433433
}

compiler/rustc_lint/src/lints.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,7 @@ pub(crate) struct DeprecatedLintNameFromCommandLine<'a> {
10891089
#[diag(lint_renamed_lint)]
10901090
pub(crate) struct RenamedLint<'a> {
10911091
pub name: &'a str,
1092+
pub replace: &'a str,
10921093
#[subdiagnostic]
10931094
pub suggestion: RenamedLintSuggestion<'a>,
10941095
}
@@ -1109,6 +1110,7 @@ pub(crate) enum RenamedLintSuggestion<'a> {
11091110
#[diag(lint_renamed_lint)]
11101111
pub(crate) struct RenamedLintFromCommandLine<'a> {
11111112
pub name: &'a str,
1113+
pub replace: &'a str,
11121114
#[subdiagnostic]
11131115
pub suggestion: RenamedLintSuggestion<'a>,
11141116
#[subdiagnostic]
@@ -3244,7 +3246,7 @@ pub(crate) enum MismatchedLifetimeSyntaxesSuggestion {
32443246
},
32453247

32463248
Explicit {
3247-
lifetime_name: String,
3249+
lifetime_name_sugg: String,
32483250
suggestions: Vec<(Span, String)>,
32493251
tool_only: bool,
32503252
},
@@ -3298,13 +3300,12 @@ impl Subdiagnostic for MismatchedLifetimeSyntaxesSuggestion {
32983300
);
32993301
}
33003302

3301-
Explicit { lifetime_name, suggestions, tool_only } => {
3302-
diag.arg("lifetime_name", lifetime_name);
3303-
3303+
Explicit { lifetime_name_sugg, suggestions, tool_only } => {
3304+
diag.arg("lifetime_name_sugg", lifetime_name_sugg);
33043305
let msg = diag.eagerly_translate(
33053306
fluent::lint_mismatched_lifetime_syntaxes_suggestion_explicit,
33063307
);
3307-
3308+
diag.remove_arg("lifetime_name_sugg");
33083309
diag.multipart_suggestion_with_style(
33093310
msg,
33103311
suggestions,

compiler/rustc_macros/src/diagnostics/subdiagnostic.rs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> {
220220
}
221221

222222
/// Generates the code for a field with no attributes.
223-
fn generate_field_arg(&mut self, binding_info: &BindingInfo<'_>) -> TokenStream {
223+
fn generate_field_arg(&mut self, binding_info: &BindingInfo<'_>) -> (TokenStream, TokenStream) {
224224
let diag = &self.parent.diag;
225225

226226
let field = binding_info.ast();
@@ -230,12 +230,16 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> {
230230
let ident = field.ident.as_ref().unwrap();
231231
let ident = format_ident!("{}", ident); // strip `r#` prefix, if present
232232

233-
quote! {
233+
let args = quote! {
234234
#diag.arg(
235235
stringify!(#ident),
236236
#field_binding
237237
);
238-
}
238+
};
239+
let remove_args = quote! {
240+
#diag.remove_arg(stringify!(#ident));
241+
};
242+
(args, remove_args)
239243
}
240244

241245
/// Generates the necessary code for all attributes on a field.
@@ -600,8 +604,13 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> {
600604

601605
calls.extend(call);
602606
}
603-
604-
let plain_args: TokenStream = self
607+
let store_args = quote! {
608+
#diag.store_args();
609+
};
610+
let restore_args = quote! {
611+
#diag.restore_args();
612+
};
613+
let (plain_args, remove_args): (TokenStream, TokenStream) = self
605614
.variant
606615
.bindings()
607616
.iter()
@@ -610,12 +619,23 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> {
610619
.collect();
611620

612621
let formatting_init = &self.formatting_init;
622+
623+
// For #[derive(Subdiagnostic)]
624+
//
625+
// - Store args of the main diagnostic for later restore.
626+
// - add args of subdiagnostic.
627+
// - Generate the calls, such as note, label, etc.
628+
// - Remove the arguments for allowing Vec<Subdiagnostic> to be used.
629+
// - Restore the arguments for allowing main and subdiagnostic share the same fields.
613630
Ok(quote! {
614631
#init
615632
#formatting_init
616633
#attr_args
634+
#store_args
617635
#plain_args
618636
#calls
637+
#remove_args
638+
#restore_args
619639
})
620640
}
621641
}

0 commit comments

Comments
 (0)