-
Notifications
You must be signed in to change notification settings - Fork 13.7k
fix: Filter suggestion parts that match existing code #146121
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.
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
Conversation
rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r?
to explicitly pick a reviewer
Some changes occurred in src/tools/clippy
cc @rust-lang/clippy
@bors r+
...r=petrochenkov fix: Filter suggestion parts that match existing code While testing my changes to make `rustc` use `annotate-snippets`, I encountered a new `clippy` test failure stemming from [two](https://github.com/rust-lang/rust/pull/145273/files#diff-6e8403e31463539666afbc00479cb416dc767a518f562b6e2960630953ee7da2R275-R278) [suggestion](https://github.com/rust-lang/rust/pull/145273/files#diff-6e8403e31463539666afbc00479cb416dc767a518f562b6e2960630953ee7da2R289-R292) output changes in rust-lang#145273. The new output in these two cases feels like a regression as it is not as clear as the old output, and adds unnecessary information. Before rust-lang#145273 (`Diff` style)  After rust-lang#145273 ("multi-line" style)  The reason for the change was that a new suggestion part (which matches existing code) was added on a different line than the existing parts, causing the suggestion style to change from `Diff` to "multi-line". Since this new part matches existing code, no code changes show up in the output for it, but it still makes the suggestion style "multi-line" when it doesn't need to be. To get the old output back, I made it so that suggestion parts that perfectly match existing code get filtered out.
Rollup of 8 pull requests Successful merges: - #139113 (unstable book: in a sanitizer example, check the code) - #145823 (editorconfig: don't use nonexistent syntax) - #145962 (Ensure we emit an allocator shim when only some crate types need one) - #146032 (Explicity disable LSX feature for `loongarch64-unknown-none` target) - #146090 (Derive `PartialEq` for `InvisibleOrigin`) - #146120 (Correct typo in `rustc_errors` comment) - #146121 (fix: Filter suggestion parts that match existing code) - #146134 (llvm: nvptx: Layout update to match LLVM) r? `@ghost` `@rustbot` modify labels: rollup
...r=petrochenkov fix: Filter suggestion parts that match existing code While testing my changes to make `rustc` use `annotate-snippets`, I encountered a new `clippy` test failure stemming from [two](https://github.com/rust-lang/rust/pull/145273/files#diff-6e8403e31463539666afbc00479cb416dc767a518f562b6e2960630953ee7da2R275-R278) [suggestion](https://github.com/rust-lang/rust/pull/145273/files#diff-6e8403e31463539666afbc00479cb416dc767a518f562b6e2960630953ee7da2R289-R292) output changes in rust-lang#145273. The new output in these two cases feels like a regression as it is not as clear as the old output, and adds unnecessary information. Before rust-lang#145273 (`Diff` style)  After rust-lang#145273 ("multi-line" style)  The reason for the change was that a new suggestion part (which matches existing code) was added on a different line than the existing parts, causing the suggestion style to change from `Diff` to "multi-line". Since this new part matches existing code, no code changes show up in the output for it, but it still makes the suggestion style "multi-line" when it doesn't need to be. To get the old output back, I made it so that suggestion parts that perfectly match existing code get filtered out.
...r=petrochenkov fix: Filter suggestion parts that match existing code While testing my changes to make `rustc` use `annotate-snippets`, I encountered a new `clippy` test failure stemming from [two](https://github.com/rust-lang/rust/pull/145273/files#diff-6e8403e31463539666afbc00479cb416dc767a518f562b6e2960630953ee7da2R275-R278) [suggestion](https://github.com/rust-lang/rust/pull/145273/files#diff-6e8403e31463539666afbc00479cb416dc767a518f562b6e2960630953ee7da2R289-R292) output changes in rust-lang#145273. The new output in these two cases feels like a regression as it is not as clear as the old output, and adds unnecessary information. Before rust-lang#145273 (`Diff` style)  After rust-lang#145273 ("multi-line" style)  The reason for the change was that a new suggestion part (which matches existing code) was added on a different line than the existing parts, causing the suggestion style to change from `Diff` to "multi-line". Since this new part matches existing code, no code changes show up in the output for it, but it still makes the suggestion style "multi-line" when it doesn't need to be. To get the old output back, I made it so that suggestion parts that perfectly match existing code get filtered out.
This probably also fixes (or at least suppresses tests/crashes/131762.rs
): #146139 (comment)
Could you convert it into a ui test or remove if redundant?
@bors r-
(Let me double-check)
@bors try jobs=aarch64-apple
fix: Filter suggestion parts that match existing code try-job: aarch64-apple
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💔 Test for e4abe76 failed: CI. Failed jobs:
try - aarch64-apple
(web logs, extended logs)
f696cd8
to
3f529cf
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.
Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.
I have been unable to reproduce the test failure on my Mac locally, so I might have to use try
builds to figure it out.
@bors try jobs=aarch64-apple
This comment has been minimized.
This comment has been minimized.
fix: Filter suggestion parts that match existing code try-job: aarch64-apple
The job aarch64-apple
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/131295.rs ... ok
test [crashes] tests/crashes/131373.rs ... ok
test [crashes] tests/crashes/131406.rs ... ok
test [crashes] tests/crashes/131534.rs ... ok
2025年09月03日T16:04:59.423523Z 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/131342.rs ... ok
test [crashes] tests/crashes/131787.rs ... ok
test [crashes] tests/crashes/131886.rs ... ok
test [crashes] tests/crashes/132126.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] --> /Users/runner/work/rust/rust/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
--> /Users/runner/work/rust/rust/tests/crashes/131762.rs:8:13
|
8 | ...t!((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
--> /Users/runner/work/rust/rust/tests/crashes/131762.rs:5:8
|
5 | struct FloatWrapper(f64);
| ^^^^^^^^^^^^
help: remove the extra arguments
|
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)))
8 + assert!(/* f64 */)))
|
error[E0369]: binary operation `>=` cannot be applied to type `FloatWrapper`
##[error] --> /Users/runner/work/rust/rust/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`
--> /Users/runner/work/rust/rust/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:
8: __rustc::rust_begin_unwind
💔 Test for 51348b8 failed: CI. Failed jobs:
try - aarch64-apple
(web logs, extended logs)
Uh oh!
There was an error while loading. Please reload this page.
While testing my changes to make
rustc
useannotate-snippets
, I encountered a newclippy
test failure stemming from two suggestion output changes in #145273. The new output in these two cases feels like a regression as it is not as clear as the old output, and adds unnecessary information.Before #145273 (
Diff
style)before
After #145273 ("multi-line" style)
after
The reason for the change was that a new suggestion part (which matches existing code) was added on a different line than the existing parts, causing the suggestion style to change from
Diff
to "multi-line". Since this new part matches existing code, no code changes show up in the output for it, but it still makes the suggestion style "multi-line" when it doesn't need to be.To get the old output back, I made it so that suggestion parts that perfectly match existing code get filtered out.
try-job: aarch64-apple