Skip to content

Restrict the cases where ptr_eq triggers #14526

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

Merged
merged 1 commit into from
Apr 22, 2025

Conversation

samueltardieu
Copy link
Contributor

@samueltardieu samueltardieu commented Apr 2, 2025

ptr_eq was recently enhanced to lint on more cases of raw pointers comparison:

  • lint on all raw pointer comparison, by proposing to use [core|std]::ptr::eq(lhs, rhs) instead of lhs == rhs;
  • removing one symetric as usize on each size if needed
  • peeling any level of as *[const|mut] _ if the remaining expression can still be coerced into the original one (i.e., is a ref or raw pointer to the same type as before)

The current change restricts the lint to the cases where at least one level of symetric as usize, or any conversion to a raw pointer, could be removed. For example, a direct comparaison of two raw pointers will not trigger the lint anymore.

changelog: [ptr_eq]: do not lint when comparing two raw pointers directly with no casts involved

Fixes #14525

@rustbot
Copy link
Collaborator

rustbot commented Apr 2, 2025

r? @llogiq

rustbot has assigned @llogiq.
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 the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 2, 2025
@samueltardieu samueltardieu assigned Manishearth and unassigned llogiq Apr 2, 2025
@Manishearth
Copy link
Member

r? @Centri3

good to have a second perspective on this

@rustbot rustbot assigned Centri3 and unassigned Manishearth Apr 2, 2025
github-merge-queue bot pushed a commit that referenced this pull request Apr 8, 2025
We do not want to remove casts done inside macros. Also, when printing
the suggestion, take it from the same context as the origin expression
(the root context).

Problems found while working on #14526, but should be merged even if
#14526 is not.

changelog: none
`ptr_eq` was recently enhanced to lint on more cases of raw pointers
comparison:

- lint on all raw pointer comparison, by proposing to use
  `[core|std]::ptr::eq(lhs, rhs)` instead of `lhs == rhs`;
- removing one symetric `as usize` on each size if needed
- peeling any level of `as *[const|mut] _` if the remaining expression
  can still be coerced into the original one (i.e., is a ref or raw
  pointer to the same type as before)

The current change restricts the lint to the cases where at least one
level of symetric `as usize`, or any conversion to a raw pointer, could
be removed. For example, a direct comparaison of two raw pointers will
not trigger the lint anymore.
@samueltardieu
Copy link
Contributor Author

Rebased now that #14527 has been merged.

@Alexendoo Alexendoo added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 22, 2025
@Alexendoo Alexendoo changed the title DO NOT MERGE AS-IS: Restrict the cases where ptr_eq triggers Restrict the cases where ptr_eq triggers Apr 22, 2025
Copy link
Member

@Alexendoo Alexendoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gonna merge this since it's no longer blocked

@Alexendoo Alexendoo added this pull request to the merge queue Apr 22, 2025
Merged via the queue into rust-lang:master with commit 2422f0b Apr 22, 2025
13 checks passed
@Manishearth
Copy link
Member

Did we want to backport this?

@Alexendoo
Copy link
Member

Yeah I nominated it since it's fairly wide reaching

ojeda added a commit to ojeda/linux that referenced this pull request May 2, 2025
Starting with Rust 1.87.0 (expected 2025-05-15) [1], Clippy may expand
the `ptr_eq` lint, e.g.:

    error: use `core::ptr::eq` when comparing raw pointers
       --> rust/kernel/list.rs:438:12
        |
    438 |         if self.first == item {
        |            ^^^^^^^^^^^^^^^^^^ help: try: `core::ptr::eq(self.first, item)`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
        = note: `-D clippy::ptr-eq` implied by `-D warnings`
        = help: to override `-D warnings` add `#[allow(clippy::ptr_eq)]`

Thus clean the few cases we have.

This patch may not be actually needed by the time Rust 1.87.0 releases
since a PR to relax the lint has been beta nominated [2] due to reports
of being too eager (at least by default) [3].

Cc: [email protected] # Needed in 6.12.y and later (Rust is pinned in older LTSs).
Link: rust-lang/rust-clippy#14339 [1]
Link: rust-lang/rust-clippy#14526 [2]
Link: rust-lang/rust-clippy#14525 [3]
Signed-off-by: Miguel Ojeda <[email protected]>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this pull request May 2, 2025
Starting with Rust 1.87.0 (expected 2025-05-15) [1], Clippy may expand
the `ptr_eq` lint, e.g.:

    error: use `core::ptr::eq` when comparing raw pointers
       --> rust/kernel/list.rs:438:12
        |
    438 |         if self.first == item {
        |            ^^^^^^^^^^^^^^^^^^ help: try: `core::ptr::eq(self.first, item)`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
        = note: `-D clippy::ptr-eq` implied by `-D warnings`
        = help: to override `-D warnings` add `#[allow(clippy::ptr_eq)]`

Thus clean the few cases we have.

This patch may not be actually needed by the time Rust 1.87.0 releases
since a PR to relax the lint has been beta nominated [2] due to reports
of being too eager (at least by default) [3].

Cc: [email protected] # Needed in 6.12.y and later (Rust is pinned in older LTSs).
Link: rust-lang/rust-clippy#14339 [1]
Link: rust-lang/rust-clippy#14526 [2]
Link: rust-lang/rust-clippy#14525 [3]
Signed-off-by: Miguel Ojeda <[email protected]>
@Darksonn
Copy link

Darksonn commented May 5, 2025

Did the beta backport happen?

@samueltardieu
Copy link
Contributor Author

@flip1995 should be able to tell us more

@flip1995
Copy link
Member

flip1995 commented May 5, 2025

I will do it on Thursday 👍

ojeda added a commit to Rust-for-Linux/linux that referenced this pull request May 6, 2025
Starting with Rust 1.87.0 (expected 2025-05-15) [1], Clippy may expand
the `ptr_eq` lint, e.g.:

    error: use `core::ptr::eq` when comparing raw pointers
       --> rust/kernel/list.rs:438:12
        |
    438 |         if self.first == item {
        |            ^^^^^^^^^^^^^^^^^^ help: try: `core::ptr::eq(self.first, item)`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
        = note: `-D clippy::ptr-eq` implied by `-D warnings`
        = help: to override `-D warnings` add `#[allow(clippy::ptr_eq)]`

It is expected that a PR to relax the lint will be backported [2] by
the time Rust 1.87.0 releases, since the lint was considered too eager
(at least by default) [3].

Thus allow the lint temporarily just in case.

Cc: [email protected] # Needed in 6.12.y and later (Rust is pinned in older LTSs).
Link: rust-lang/rust-clippy#14339 [1]
Link: rust-lang/rust-clippy#14526 [2]
Link: rust-lang/rust-clippy#14525 [3]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
ojeda added a commit to Rust-for-Linux/linux that referenced this pull request May 6, 2025
Starting with Rust 1.87.0 (expected 2025-05-15) [1], Clippy may expand
the `ptr_eq` lint, e.g.:

    error: use `core::ptr::eq` when comparing raw pointers
       --> rust/kernel/list.rs:438:12
        |
    438 |         if self.first == item {
        |            ^^^^^^^^^^^^^^^^^^ help: try: `core::ptr::eq(self.first, item)`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
        = note: `-D clippy::ptr-eq` implied by `-D warnings`
        = help: to override `-D warnings` add `#[allow(clippy::ptr_eq)]`

It is expected that a PR to relax the lint will be backported [2] by
the time Rust 1.87.0 releases, since the lint was considered too eager
(at least by default) [3].

Thus allow the lint temporarily just in case.

Cc: [email protected] # Needed in 6.12.y and later (Rust is pinned in older LTSs).
Link: rust-lang/rust-clippy#14339 [1]
Link: rust-lang/rust-clippy#14526 [2]
Link: rust-lang/rust-clippy#14525 [3]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
ojeda added a commit to Rust-for-Linux/linux that referenced this pull request May 6, 2025
Starting with Rust 1.87.0 (expected 2025-05-15) [1], Clippy may expand
the `ptr_eq` lint, e.g.:

    error: use `core::ptr::eq` when comparing raw pointers
       --> rust/kernel/list.rs:438:12
        |
    438 |         if self.first == item {
        |            ^^^^^^^^^^^^^^^^^^ help: try: `core::ptr::eq(self.first, item)`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
        = note: `-D clippy::ptr-eq` implied by `-D warnings`
        = help: to override `-D warnings` add `#[allow(clippy::ptr_eq)]`

It is expected that a PR to relax the lint will be backported [2] by
the time Rust 1.87.0 releases, since the lint was considered too eager
(at least by default) [3].

Thus allow the lint temporarily just in case.

Cc: [email protected] # Needed in 6.12.y and later (Rust is pinned in older LTSs).
Link: rust-lang/rust-clippy#14339 [1]
Link: rust-lang/rust-clippy#14526 [2]
Link: rust-lang/rust-clippy#14525 [3]
Link: https://lore.kernel.org/r/[email protected]
[ Converted to `allow`s since backport was confirmed. - Miguel ]
Signed-off-by: Miguel Ojeda <[email protected]>
@samueltardieu samueltardieu deleted the push-rzovtxwoovyn branch May 6, 2025 22:19
@flip1995 flip1995 added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels May 8, 2025
@flip1995
Copy link
Member

flip1995 commented May 8, 2025

rust-lang/rust#140810

mj22226 pushed a commit to mj22226/linux that referenced this pull request May 12, 2025
commit a39f308 upstream.

Starting with Rust 1.87.0 (expected 2025-05-15) [1], Clippy may expand
the `ptr_eq` lint, e.g.:

    error: use `core::ptr::eq` when comparing raw pointers
       --> rust/kernel/list.rs:438:12
        |
    438 |         if self.first == item {
        |            ^^^^^^^^^^^^^^^^^^ help: try: `core::ptr::eq(self.first, item)`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
        = note: `-D clippy::ptr-eq` implied by `-D warnings`
        = help: to override `-D warnings` add `#[allow(clippy::ptr_eq)]`

It is expected that a PR to relax the lint will be backported [2] by
the time Rust 1.87.0 releases, since the lint was considered too eager
(at least by default) [3].

Thus allow the lint temporarily just in case.

Cc: [email protected] # Needed in 6.12.y and later (Rust is pinned in older LTSs).
Link: rust-lang/rust-clippy#14339 [1]
Link: rust-lang/rust-clippy#14526 [2]
Link: rust-lang/rust-clippy#14525 [3]
Link: https://lore.kernel.org/r/[email protected]
[ Converted to `allow`s since backport was confirmed. - Miguel ]
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clippy beta 1.87 lints all pointer comparisons, possibly too much?
8 participants