Omit suggestions when spans are invalid #147849

jdonszelmann · 2025-10-18T15:16:20Z

introduces a new compiletest directive: //@ rustfix-dont-test-fixed which is needed because I want to run rustfix, making sure rustfix doesn't crash, but then I don't expect the fixed code to compile since the suggestion was omitted and nothing was fixed.

rustbot · 2025-10-18T15:16:22Z

rustc_errors::emitter was changed

cc @Muscraft

Some changes occurred in src/tools/compiletest

cc @jieyouxu

jdonszelmann · 2025-10-18T15:19:42Z

related MCP: rust-lang/compiler-team#929

jdonszelmann · 2025-10-18T15:22:55Z

tests/ui/parser/attribute/auxiliary/all_spans_same.rs

+
+#[proc_macro_attribute]
+pub fn all_spans_same(_: TokenStream, ts: TokenStream) -> TokenStream {
+    spans_callsite(ts)


recursively set all spans in the output to the callsite

jdonszelmann · 2025-10-18T15:23:28Z

tests/ui/parser/attribute/invalid-delimeter-ice-146808.rs

+
+#[all_spans_same::all_spans_same]
+//~^ ERROR wrong meta list delimiters
+#[allow{}]


crashes, because we suggest changing {} to () but the suggestion consists of two parts ( and ) with both the same span

jdonszelmann · 2025-10-18T15:29:15Z

compiler/rustc_errors/src/json.rs

            .substitutions
            .iter()
+            .filter_map(|substitution| {
+                let mut parts = substitution.parts.clone();


skip outputting suggestions with overlapping spans to JSON, same check as in lib.rs

jdonszelmann · 2025-10-18T15:29:48Z

compiler/rustc_errors/src/lib.rs

                // 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);


some suggestions were already silently dropped, change this to count them

jdonszelmann · 2025-10-18T15:30:08Z

compiler/rustc_errors/src/lib.rs

+                if substitution
+                    .parts
+                    .array_windows()
+                    .find(|[a, b]| a.span.overlaps(b.span))


instead of failing an assertion, count how many times this happens and show this to the user

jieyouxu · 2025-10-19T09:59:03Z

src/tools/compiletest/src/directives/directive_names.rs

@@ -259,6 +259,7 @@ pub(crate) const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
    "run-pass",
    "run-rustfix",
    "rustc-env",
+    "rustfix-dont-test-fixed",


Suggestion: if we do go with this, can you please describe this new directive in src/doc/rustc-dev-guide, and also in the UI test chapter perhaps add a little remark on what this intended to be used for?

bors · 2025-10-20T21:18:35Z

☔ The latest upstream changes (presumably #147913) made this pull request unmergeable. Please resolve the merge conflicts.

Revert "fix: Filter suggestion parts that match existing code" As requested by `@wesleywiser` in #147973 (comment) this is a revert of #146121 due to the handful of diagnostics ICEs that have been since reported, and found in the beta crater run. This should thus also be backported to beta so the ICEs don't make it to next week's stable. Works around (after backport) - #146261 - #146706 - #146834 but I didn't add a test for this allowed-by-default lint - as well as the crater run regressions from #147973 of which I only added the MCVE as a test. The proper fix would likely be #147849 but it's still currently at the MCP stage. In the meantime, this PR would still emit the same overlapping suggestions, but still use a debug-assert... r? `@wesleywiser`

Revert "fix: Filter suggestion parts that match existing code" As requested by `@wesleywiser` in rust-lang/rust#147973 (comment) this is a revert of rust-lang/rust#146121 due to the handful of diagnostics ICEs that have been since reported, and found in the beta crater run. This should thus also be backported to beta so the ICEs don't make it to next week's stable. Works around (after backport) - rust-lang/rust#146261 - rust-lang/rust#146706 - rust-lang/rust#146834 but I didn't add a test for this allowed-by-default lint - as well as the crater run regressions from rust-lang/rust#147973 of which I only added the MCVE as a test. The proper fix would likely be rust-lang/rust#147849 but it's still currently at the MCP stage. In the meantime, this PR would still emit the same overlapping suggestions, but still use a debug-assert... r? `@wesleywiser`

…stions

rust-log-analyzer · 2025-10-28T15:57:22Z

The job x86_64-gnu-gcc failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

test [crashes] tests/crashes/131406.rs ... ok
test [crashes] tests/crashes/131373.rs ... ok
test [crashes] tests/crashes/131295.rs ... ok
test [crashes] tests/crashes/131534.rs ... ok
2025-10-28T15:56:36.236679Z ERROR compiletest::runtest: fatal error, panic: "crashtest no longer crashes/triggers ICE, hooray! Please give it a meaningful name, add a doc-comment to the start of the test explaining why it exists and move it to tests/ui or wherever you see fit. Adding 'Fixes #<issueNr>' to your PR description ensures that the corresponding ticket is auto-closed upon merge. If you want to see verbose output, set `COMPILETEST_VERBOSE_CRASHES=1`."
test [crashes] tests/crashes/131762.rs ... FAILED
test [crashes] tests/crashes/131886.rs ... ok
test [crashes] tests/crashes/131787.rs ... ok
test [crashes] tests/crashes/132126.rs ... ok
test [crashes] tests/crashes/132960.rs ... ok
---

---- [crashes] tests/crashes/131762.rs stdout ----
------rustc stdout------------------------------

------rustc stderr------------------------------
error[E0061]: this struct takes 1 argument but 3 arguments were supplied
##[error] --> /checkout/tests/crashes/131762.rs:8:63
  |
8 | ...) >= FloatWrapper(size_of::<u8>, size_of::<u16>, size_of::<usize> as fn() -> usize)))
  |         ^^^^^^^^^^^^                --------------  --------------------------------- unexpected argument #3 of type `fn() -> usize`
  |                                     |
  |                                     unexpected argument #2 of type `fn() -> usize {std::mem::size_of::<u16>}`
  |
note: expected `f64`, found fn item
 --> /checkout/tests/crashes/131762.rs:8:5
  |
8 |     assert!((0.0 / 0.0 >= 0.0) == (FloatWrapper(0.0 / 0.0) >= FloatWrapper(size_of::<u8>, size_of::<u16>, size_of::<usize> as fn() -> usize)))
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  = note: expected type `f64`
          found fn item `fn() -> usize {std::mem::size_of::<u8>}`
note: tuple struct defined here
 --> /checkout/tests/crashes/131762.rs:5:8
  |
5 | struct FloatWrapper(f64);
  |        ^^^^^^^^^^^^
rendering error: omitted 1 suggestion that failed to render, likely because of macro expansions

error[E0369]: binary operation `>=` cannot be applied to type `FloatWrapper`
##[error] --> /checkout/tests/crashes/131762.rs:8:60
  |
8 | ....0) == (FloatWrapper(0.0 / 0.0) >= FloatWrapper(size_of::<u8>, size_of::<u16>, size_of::<usize> as fn() -> usize)))
  |            ----------------------- ^^ ------------------------------------------------------------------------------ FloatWrapper
  |            |
  |            FloatWrapper
  |
note: an implementation of `PartialOrd` might be missing for `FloatWrapper`
 --> /checkout/tests/crashes/131762.rs:5:1
  |
5 | struct FloatWrapper(f64);
  | ^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd`
help: consider annotating `FloatWrapper` with `#[derive(PartialEq, PartialOrd)]`
  |
5 + #[derive(PartialEq, PartialOrd)]
6 | struct FloatWrapper(f64);
  |

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0061, E0369.
For more information about an error, try `rustc --explain E0061`.

------------------------------------------

error: crashtest no longer crashes/triggers ICE, hooray! Please give it a meaningful name, add a doc-comment to the start of the test explaining why it exists and move it to tests/ui or wherever you see fit. Adding 'Fixes #<issueNr>' to your PR description ensures that the corresponding ticket is auto-closed upon merge. If you want to see verbose output, set `COMPILETEST_VERBOSE_CRASHES=1`.

thread '[crashes] tests/crashes/131762.rs' panicked at src/tools/compiletest/src/runtest/crashes.rs:17:18:
fatal error
stack backtrace:
   5: __rustc::rust_begin_unwind

For more information how to resolve CI failures of this job, visit this link.

jdonszelmann mentioned this pull request Oct 18, 2025

Omit suggestions when spans overlap rust-lang/compiler-team#929

Open

3 tasks

jdonszelmann commented Oct 18, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

jdonszelmann commented Oct 18, 2025

View reviewed changes

jdonszelmann mentioned this pull request Oct 18, 2025

ICE all spans must be disjoint (attrs) #146808

Open

jdonszelmann force-pushed the ice-without-requires branch from b876dd7 to 7d26571 Compare October 18, 2025 15:24

jdonszelmann mentioned this pull request Oct 18, 2025

Fix ICE from invalid delimeter in attrs parsing #147778

Closed

jdonszelmann commented Oct 18, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

jieyouxu self-assigned this Oct 18, 2025

jieyouxu reviewed Oct 19, 2025

View reviewed changes

jieyouxu removed their assignment Oct 20, 2025

GoldsteinE mentioned this pull request Oct 21, 2025

ICE all spans must be disjoint #147919

Open

madsmtm mentioned this pull request Oct 22, 2025

regression: ICE all spans must be disjoint #147973

Closed

lqd mentioned this pull request Oct 23, 2025

Revert "fix: Filter suggestion parts that match existing code" #148043

Merged

jdonszelmann added 2 commits October 28, 2025 12:27

introduce test for 146808

b306a30

don't emit suggestions with overlapping spans and count omitted sugge…

81ccf14

…stions

update compiletest to support testing if rustfix didnt crash

65a81f9

jdonszelmann force-pushed the ice-without-requires branch from 7d26571 to 65a81f9 Compare October 28, 2025 15:17

Uh oh!

Omit suggestions when spans are invalid #147849

Are you sure you want to change the base?

Omit suggestions when spans are invalid #147849

Conversation

jdonszelmann commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 18, 2025

Uh oh!

jdonszelmann commented Oct 18, 2025

Uh oh!

jdonszelmann Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

jdonszelmann Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

jdonszelmann Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

jdonszelmann Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

jdonszelmann Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

jieyouxu Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdonszelmann Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

bors commented Oct 20, 2025

Uh oh!

rust-log-analyzer commented Oct 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jdonszelmann commented Oct 18, 2025 •

edited

Loading

jieyouxu Oct 19, 2025 •

edited

Loading