Skip to content
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

Downgrade ambiguous_wide_pointer_comparisons suggestions to MaybeIncorrect #121338

Merged

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
Contributor

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
11 checks passed
@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