Skip to content

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Feb 20, 2024

In certain cases like #121330, it is possible to have more than one suggestion from the ambiguous_wide_pointer_comparisons lint (which before this PR are MachineApplicable). When this gets passed to rustfix, rustfix makes multiple changes according to the suggestions which result in incorrect code.

This is a temporary workaround. The real long term solution to problems like these is to address #53934.

This PR also includes a drive-by edit to the panic message emitted by compiletest because "ui" test suite now uses //@ directives.

Fixes #121330.

@rustbot
Copy link
Collaborator

rustbot commented Feb 20, 2024

r? @Nadrieril

rustbot has assigned @Nadrieril.
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

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 20, 2024
@jieyouxu jieyouxu force-pushed the ambiguous_wide_pointer_comparisons_suggestion branch from 49025fa to 0e58333 Compare February 20, 2024 12:11
@jieyouxu jieyouxu changed the title Ambiguous wide pointer comparisons suggestion Downgrade ambiguous_wide_pointer_comparisons suggestion to MaybeIncorrect Feb 20, 2024
@jieyouxu jieyouxu changed the title Downgrade ambiguous_wide_pointer_comparisons suggestion to MaybeIncorrect Downgrade ambiguous_wide_pointer_comparisons suggestions to MaybeIncorrect Feb 20, 2024
@Nadrieril
Copy link
Member

Looks good! One nit, r=me after that.

@bors rollup=always

…rrect

It is possible to have more than one valid suggestion, which when
applied together via rustfix causes the code to no longer compile.

This is a temporary workaround; the real long term solution to these
issues is to solve <rust-lang#53934>.
@jieyouxu jieyouxu force-pushed the ambiguous_wide_pointer_comparisons_suggestion branch from 0e58333 to 4d386d9 Compare February 20, 2024 17:21
@Nadrieril
Copy link
Member

Thank you for fixing this!

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 20, 2024

📌 Commit 4d386d9 has been approved by Nadrieril

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 20, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 21, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#121208 (Convert `delayed_bug`s to `bug`s.)
 - rust-lang#121288 (make rustc_expand translatable)
 - rust-lang#121304 (Add docs for extension proc-macro)
 - rust-lang#121328 (Make --verbose imply -Z write-long-types-to-disk=no)
 - rust-lang#121338 (Downgrade ambiguous_wide_pointer_comparisons suggestions to MaybeIncorrect)
 - rust-lang#121361 (diagnostic items for legacy numeric modules)
 - rust-lang#121375 (Print proper relative path for descriptive name check)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e10b3b8 into rust-lang:master Feb 21, 2024
@rustbot rustbot added this to the 1.78.0 milestone Feb 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 21, 2024
Rollup merge of rust-lang#121338 - jieyouxu:ambiguous_wide_pointer_comparisons_suggestion, r=Nadrieril

Downgrade ambiguous_wide_pointer_comparisons suggestions to MaybeIncorrect

In certain cases like rust-lang#121330, it is possible to have more than one suggestion from the `ambiguous_wide_pointer_comparisons` lint (which before this PR are `MachineApplicable`). When this gets passed to rustfix, rustfix makes *multiple* changes according to the suggestions which result in incorrect code.

This is a temporary workaround. The real long term solution to problems like these is to address <rust-lang#53934>.

This PR also includes a drive-by edit to the panic message emitted by compiletest because "ui" test suite now uses `//`@`` directives.

Fixes rust-lang#121330.
@jieyouxu jieyouxu deleted the ambiguous_wide_pointer_comparisons_suggestion branch September 25, 2024 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ambiguous_wide_pointer_comparisons: bad suggestion
5 participants