From b306a3046dd39942e7762a6aa8c7d484b1b4f875 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jana=20D=C3=B6nszelmann?= Date: Sat, 18 Oct 2025 13:20:15 +0200 Subject: [PATCH 1/3] introduce test for 146808 --- .../attribute/auxiliary/all_spans_same.rs | 26 +++++++++++++++++++ .../attribute/invalid-delimeter-ice-146808.rs | 7 +++++ 2 files changed, 33 insertions(+) create mode 100644 tests/ui/parser/attribute/auxiliary/all_spans_same.rs create mode 100644 tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs diff --git a/tests/ui/parser/attribute/auxiliary/all_spans_same.rs b/tests/ui/parser/attribute/auxiliary/all_spans_same.rs new file mode 100644 index 0000000000000..deddbdff5536d --- /dev/null +++ b/tests/ui/parser/attribute/auxiliary/all_spans_same.rs @@ -0,0 +1,26 @@ +extern crate proc_macro; +use proc_macro::*; + +fn spans_callsite(ts: TokenStream) -> TokenStream { + let mut new_ts = TokenStream::new(); + + for i in ts { + let new_token = i.clone(); + match new_token { + TokenTree::Group(g) => { + new_ts.extend([Group::new(g.delimiter(), spans_callsite(g.stream()))]) + } + mut other => { + other.set_span(Span::call_site()); + new_ts.extend([other]); + } + } + } + + new_ts +} + +#[proc_macro_attribute] +pub fn all_spans_same(_: TokenStream, ts: TokenStream) -> TokenStream { + spans_callsite(ts) +} diff --git a/tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs new file mode 100644 index 0000000000000..091962e044b94 --- /dev/null +++ b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs @@ -0,0 +1,7 @@ +// regression test for #146808 +//@ proc-macro: all_spans_same.rs +extern crate all_spans_same; + +#[all_spans_same::all_spans_same] +#[allow{}] +fn main() {} From 81ccf1452d66d8db6186ec2d501de870c709cc83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jana=20D=C3=B6nszelmann?= Date: Sat, 18 Oct 2025 14:20:28 +0200 Subject: [PATCH 2/3] don't emit suggestions with overlapping spans and count omitted suggestions --- compiler/rustc_errors/src/emitter.rs | 17 ++++++++-- compiler/rustc_errors/src/json.rs | 16 +++++++++ compiler/rustc_errors/src/lib.rs | 34 ++++++++++++++++--- .../invalid-delimeter-ice-146808.fixed | 11 ++++++ .../attribute/invalid-delimeter-ice-146808.rs | 4 +++ .../invalid-delimeter-ice-146808.stderr | 11 ++++++ 6 files changed, 86 insertions(+), 7 deletions(-) create mode 100644 tests/ui/parser/attribute/invalid-delimeter-ice-146808.fixed create mode 100644 tests/ui/parser/attribute/invalid-delimeter-ice-146808.stderr diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 9e32a85e361a5..c609dfd90edf4 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -2038,20 +2038,31 @@ impl HumanEmitter { }; // Render the replacements for each suggestion - let suggestions = suggestion.splice_lines(sm); + let (suggestions, num_omitted) = suggestion.splice_lines(sm); debug!(?suggestions); + let mut buffer = StyledBuffer::new(); + + if num_omitted.0 > 0 { + // in this case, do push that we omitted some + buffer.append( + 0, + &format!("rendering error: omitted {} suggestion{} that failed to render, likely because of macro expansions", num_omitted.0, if num_omitted.0 > 1 {"s"} else {""}), + Style::Level(Level::Error), + ); + } + if suggestions.is_empty() { // Here we check if there are suggestions that have actual code changes. We sometimes // suggest the same code that is already there, instead of changing how we produce the // suggestions and filtering there, we just don't emit the suggestion. // Suggestions coming from macros can also have malformed spans. This is a heavy handed // approach to avoid ICEs by ignoring the suggestion outright. + + emit_to_destination(&buffer.render(), &Level::Note, &mut self.dst, self.short_message)?; return Ok(()); } - let mut buffer = StyledBuffer::new(); - // Render the suggestion message buffer.append(0, level.to_str(), Style::Level(*level)); buffer.append(0, ": ", Style::HeaderMsg); diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 03ce1d82ef3c4..3ea35609260de 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -24,6 +24,7 @@ use rustc_span::Span; use rustc_span::hygiene::ExpnData; use rustc_span::source_map::{FilePathMapping, SourceMap}; use serde::Serialize; +use tracing::debug; use crate::diagnostic::IsLint; use crate::emitter::{ @@ -543,6 +544,21 @@ impl DiagnosticSpan { suggestion .substitutions .iter() + .filter_map(|substitution| { + let mut parts = substitution.parts.clone(); + // check that all spans are distjoint, otherwise rustfix doesn't know what to do. + // suggestions that don't have that have a hint about omitted suggestions in them + parts.sort_by_key(|part| part.span.lo()); + // Verify the assumption that all spans are disjoint + if parts.array_windows().find(|[a, b]| a.span.overlaps(b.span)).is_some() { + debug!( + "from_suggestion: suggestion contains an invalid span: {substitution:?}" + ); + None + } else { + Some(substitution) + } + }) .flat_map(|substitution| { substitution.parts.iter().map(move |suggestion_inner| { let span_label = diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 17cd466f96b87..9ca071cf0c226 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -322,20 +322,27 @@ fn as_substr<'a>(original: &'a str, suggestion: &'a str) -> Option<(usize, &'a s } } +/// Represents suggestions that were emitted because their span was ambiguous. +pub(crate) struct SuggestionsOmitted(usize); + impl CodeSuggestion { /// Returns the assembled code suggestions, whether they should be shown with an underline /// and whether the substitution only differs in capitalization. pub(crate) fn splice_lines( &self, sm: &SourceMap, - ) -> Vec<(String, Vec, Vec>, ConfusionType)> - { + ) -> ( + Vec<(String, Vec, Vec>, ConfusionType)>, + SuggestionsOmitted, + ) { // For the `Vec>` value, the first level of the vector // corresponds to the output snippet's lines, while the second level corresponds to the // substrings within that line that should be highlighted. use rustc_span::{CharPos, Pos}; + let num_omitted = Cell::new(0); + /// Extracts a substring from the provided `line_opt` based on the specified low and high /// indices, appends it to the given buffer `buf`, and returns the count of newline /// characters in the substring for accurate highlighting. If `line_opt` is `None`, a @@ -384,13 +391,15 @@ impl CodeSuggestion { assert!(!self.substitutions.is_empty()); - self.substitutions + let res = self + .substitutions .iter() .filter(|subst| { // Suggestions coming from macros can have malformed spans. This is a heavy // handed approach to avoid ICEs by ignoring the suggestion outright. let invalid = subst.parts.iter().any(|item| sm.is_valid_span(item.span).is_err()); if invalid { + num_omitted.update(|i| i + 1); debug!("splice_lines: suggestion contains an invalid span: {:?}", subst); } !invalid @@ -400,6 +409,21 @@ impl CodeSuggestion { // Assumption: all spans are in the same file, and all spans // are disjoint. Sort in ascending order. substitution.parts.sort_by_key(|part| part.span.lo()); + if substitution + .parts + .array_windows() + .find(|[a, b]| a.span.overlaps(b.span)) + .is_some() + { + debug!("splice_lines: suggestion contains an invalid span: {substitution:?}"); + num_omitted.update(|i| i + 1); + return None; + } + + // Account for cases where we are suggesting the same code that's already + // there. This shouldn't happen often, but in some cases for multipart + // suggestions it's much easier to handle it here than in the origin. + substitution.parts.retain(|p| is_different(sm, &p.snippet, p.span)); // Find the bounding span. let lo = substitution.parts.iter().map(|part| part.span.lo()).min()?; @@ -541,7 +565,9 @@ impl CodeSuggestion { Some((buf, trimmed_parts, highlights, confusion_type)) } }) - .collect() + .collect(); + + (res, SuggestionsOmitted(num_omitted.get())) } } diff --git a/tests/ui/parser/attribute/invalid-delimeter-ice-146808.fixed b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.fixed new file mode 100644 index 0000000000000..c6e88969ad1ce --- /dev/null +++ b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.fixed @@ -0,0 +1,11 @@ +// regression test for #146808 +//@ proc-macro: all_spans_same.rs +//@ run-rustfix +//@ rustfix-only-machine-applicable + +extern crate all_spans_same; + +#[all_spans_same::all_spans_same] +//~^ ERROR wrong meta list delimiters +#[allow{}] +fn main() {} diff --git a/tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs index 091962e044b94..c6e88969ad1ce 100644 --- a/tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs +++ b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs @@ -1,7 +1,11 @@ // regression test for #146808 //@ proc-macro: all_spans_same.rs +//@ run-rustfix +//@ rustfix-only-machine-applicable + extern crate all_spans_same; #[all_spans_same::all_spans_same] +//~^ ERROR wrong meta list delimiters #[allow{}] fn main() {} diff --git a/tests/ui/parser/attribute/invalid-delimeter-ice-146808.stderr b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.stderr new file mode 100644 index 0000000000000..8236830c5d467 --- /dev/null +++ b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.stderr @@ -0,0 +1,11 @@ +error: wrong meta list delimiters + --> $DIR/invalid-delimeter-ice-146808.rs:8:1 + | +LL | #[all_spans_same::all_spans_same] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the attribute macro `all_spans_same::all_spans_same` (in Nightly builds, run with -Z macro-backtrace for more info) +rendering error: omitted 1 suggestion that failed to render, likely because of macro expansions + +error: aborting due to 1 previous error + From 65a81f9e6685988f44e0be76aedce27e05631cb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jana=20D=C3=B6nszelmann?= Date: Sat, 18 Oct 2025 17:06:11 +0200 Subject: [PATCH 3/3] update compiletest to support testing if rustfix didnt crash --- src/tools/compiletest/src/directives.rs | 9 +++++++++ src/tools/compiletest/src/directives/directive_names.rs | 1 + src/tools/compiletest/src/runtest/ui.rs | 5 ++++- .../parser/attribute/invalid-delimeter-ice-146808.fixed | 4 +++- .../ui/parser/attribute/invalid-delimeter-ice-146808.rs | 4 +++- .../parser/attribute/invalid-delimeter-ice-146808.stderr | 2 +- 6 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index 0318ed2b3d119..60ff773e94427 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -181,6 +181,8 @@ pub(crate) struct TestProps { pub run_rustfix: bool, // If true, `rustfix` will only apply `MachineApplicable` suggestions. pub rustfix_only_machine_applicable: bool, + // If true, don't test the fixed program. It's still broken in some way. + pub rustfix_dont_test_fixed: bool, pub assembly_output: Option, // If true, the test is expected to ICE pub should_ice: bool, @@ -245,6 +247,7 @@ mod directives { pub const DONT_CHECK_FAILURE_STATUS: &'static str = "dont-check-failure-status"; pub const RUN_RUSTFIX: &'static str = "run-rustfix"; pub const RUSTFIX_ONLY_MACHINE_APPLICABLE: &'static str = "rustfix-only-machine-applicable"; + pub const RUSTFIX_DONT_TEST_FIXED: &'static str = "rustfix-dont-test-fixed"; pub const ASSEMBLY_OUTPUT: &'static str = "assembly-output"; pub const STDERR_PER_BITWIDTH: &'static str = "stderr-per-bitwidth"; pub const INCREMENTAL: &'static str = "incremental"; @@ -303,6 +306,7 @@ impl TestProps { dont_check_failure_status: false, run_rustfix: false, rustfix_only_machine_applicable: false, + rustfix_dont_test_fixed: false, assembly_output: None, should_ice: false, stderr_per_bitwidth: false, @@ -543,6 +547,11 @@ impl TestProps { RUSTFIX_ONLY_MACHINE_APPLICABLE, &mut self.rustfix_only_machine_applicable, ); + config.set_name_directive( + ln, + RUSTFIX_DONT_TEST_FIXED, + &mut self.rustfix_dont_test_fixed, + ); config.set_name_value_directive( ln, ASSEMBLY_OUTPUT, diff --git a/src/tools/compiletest/src/directives/directive_names.rs b/src/tools/compiletest/src/directives/directive_names.rs index 3a46dbc704e87..104d346435f3d 100644 --- a/src/tools/compiletest/src/directives/directive_names.rs +++ b/src/tools/compiletest/src/directives/directive_names.rs @@ -260,6 +260,7 @@ pub(crate) const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "run-pass", "run-rustfix", "rustc-env", + "rustfix-dont-test-fixed", "rustfix-only-machine-applicable", "should-fail", "should-ice", diff --git a/src/tools/compiletest/src/runtest/ui.rs b/src/tools/compiletest/src/runtest/ui.rs index d683a325c8666..d2767ba07312c 100644 --- a/src/tools/compiletest/src/runtest/ui.rs +++ b/src/tools/compiletest/src/runtest/ui.rs @@ -220,7 +220,10 @@ impl TestCx<'_> { self.check_all_error_patterns(&output_to_check, pattern_proc_res); self.check_forbid_output(&output_to_check, pattern_proc_res); - if self.props.run_rustfix && self.config.compare_mode.is_none() { + if self.props.run_rustfix + && self.config.compare_mode.is_none() + && !self.props.rustfix_dont_test_fixed + { // And finally, compile the fixed code and make sure it both // succeeds and has no diagnostics. let mut rustc = self.make_compile_args( diff --git a/tests/ui/parser/attribute/invalid-delimeter-ice-146808.fixed b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.fixed index c6e88969ad1ce..08c670ac2b91a 100644 --- a/tests/ui/parser/attribute/invalid-delimeter-ice-146808.fixed +++ b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.fixed @@ -1,7 +1,9 @@ // regression test for #146808 //@ proc-macro: all_spans_same.rs //@ run-rustfix -//@ rustfix-only-machine-applicable +// the fixed program is still broken, but rustfix didn't crash! +// that's what we want to test here. +//@ rustfix-dont-test-fixed extern crate all_spans_same; diff --git a/tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs index c6e88969ad1ce..08c670ac2b91a 100644 --- a/tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs +++ b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs @@ -1,7 +1,9 @@ // regression test for #146808 //@ proc-macro: all_spans_same.rs //@ run-rustfix -//@ rustfix-only-machine-applicable +// the fixed program is still broken, but rustfix didn't crash! +// that's what we want to test here. +//@ rustfix-dont-test-fixed extern crate all_spans_same; diff --git a/tests/ui/parser/attribute/invalid-delimeter-ice-146808.stderr b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.stderr index 8236830c5d467..3b385a0ab35ef 100644 --- a/tests/ui/parser/attribute/invalid-delimeter-ice-146808.stderr +++ b/tests/ui/parser/attribute/invalid-delimeter-ice-146808.stderr @@ -1,5 +1,5 @@ error: wrong meta list delimiters - --> $DIR/invalid-delimeter-ice-146808.rs:8:1 + --> $DIR/invalid-delimeter-ice-146808.rs:10:1 | LL | #[all_spans_same::all_spans_same] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^