Lint more cases with ptr_eq (#14339)

This PR:

- lints more case of raw pointer comparisons
- do not omit the named function to raw pointer conversion before
suggesting
- trigger the `ptr_eq` lint only if `cmp_null` doesn't trigger first, as
this is a more specialized version
- lints against `!=` in addition to `==`

The `ptr_eq` code has been moved from under `operators` to `ptr.rs`, in
order to benefit from factorization.

Fix #14337

changelog: [`ptr_eq`]: handle more cases

Author: flip1995

clippy_lints/src/declared_lints.rs

@@ -614,7 +614,6 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::operators::MODULO_ONE_INFO,
     crate::operators::NEEDLESS_BITWISE_BOOL_INFO,
     crate::operators::OP_REF_INFO,
-    crate::operators::PTR_EQ_INFO,
     crate::operators::REDUNDANT_COMPARISONS_INFO,
     crate::operators::SELF_ASSIGNMENT_INFO,
     crate::operators::VERBOSE_BIT_MASK_INFO,
@@ -641,6 +640,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::ptr::INVALID_NULL_PTR_USAGE_INFO,
     crate::ptr::MUT_FROM_REF_INFO,
     crate::ptr::PTR_ARG_INFO,
+    crate::ptr::PTR_EQ_INFO,
     crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO,
     crate::pub_underscore_fields::PUB_UNDERSCORE_FIELDS_INFO,
     crate::pub_use::PUB_USE_INFO,

clippy_lints/src/operators/mod.rs

@@ -18,7 +18,6 @@ mod modulo_one;
 mod needless_bitwise_bool;
 mod numeric_arithmetic;
 mod op_ref;
-mod ptr_eq;
 mod self_assignment;
 mod verbose_bit_mask;
 
@@ -768,35 +767,6 @@ declare_clippy_lint! {
     "Boolean expressions that use bitwise rather than lazy operators"
 }
 
-declare_clippy_lint! {
-    /// ### What it does
-    /// Use `std::ptr::eq` when applicable
-    ///
-    /// ### Why is this bad?
-    /// `ptr::eq` can be used to compare `&T` references
-    /// (which coerce to `*const T` implicitly) by their address rather than
-    /// comparing the values they point to.
-    ///
-    /// ### Example
-    /// ```no_run
-    /// let a = &[1, 2, 3];
-    /// let b = &[1, 2, 3];
-    ///
-    /// assert!(a as *const _ as usize == b as *const _ as usize);
-    /// ```
-    /// Use instead:
-    /// ```no_run
-    /// let a = &[1, 2, 3];
-    /// let b = &[1, 2, 3];
-    ///
-    /// assert!(std::ptr::eq(a, b));
-    /// ```
-    #[clippy::version = "1.49.0"]
-    pub PTR_EQ,
-    style,
-    "use `std::ptr::eq` when comparing raw pointers"
-}
-
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for explicit self-assignments.
@@ -902,7 +872,6 @@ impl_lint_pass!(Operators => [
     MODULO_ONE,
     MODULO_ARITHMETIC,
     NEEDLESS_BITWISE_BOOL,
-    PTR_EQ,
     SELF_ASSIGNMENT,
     MANUAL_MIDPOINT,
 ]);
@@ -921,7 +890,6 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
                     erasing_op::check(cx, e, op.node, lhs, rhs);
                     identity_op::check(cx, e, op.node, lhs, rhs);
                     needless_bitwise_bool::check(cx, e, op.node, lhs, rhs);
-                    ptr_eq::check(cx, e, op.node, lhs, rhs);
                     manual_midpoint::check(cx, e, op.node, lhs, rhs, self.msrv);
                 }
                 self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);

clippy_lints/src/operators/ptr_eq.rs

@@ -148,7 +148,36 @@ declare_clippy_lint! {
     "invalid usage of a null pointer, suggesting `NonNull::dangling()` instead"
 }
 
-declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF, INVALID_NULL_PTR_USAGE]);
+declare_clippy_lint! {
+    /// ### What it does
+    /// Use `std::ptr::eq` when applicable
+    ///
+    /// ### Why is this bad?
+    /// `ptr::eq` can be used to compare `&T` references
+    /// (which coerce to `*const T` implicitly) by their address rather than
+    /// comparing the values they point to.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let a = &[1, 2, 3];
+    /// let b = &[1, 2, 3];
+    ///
+    /// assert!(a as *const _ as usize == b as *const _ as usize);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let a = &[1, 2, 3];
+    /// let b = &[1, 2, 3];
+    ///
+    /// assert!(std::ptr::eq(a, b));
+    /// ```
+    #[clippy::version = "1.49.0"]
+    pub PTR_EQ,
+    style,
+    "use `std::ptr::eq` when comparing raw pointers"
+}
+
+declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF, INVALID_NULL_PTR_USAGE, PTR_EQ]);
 
 impl<'tcx> LateLintPass<'tcx> for Ptr {
     fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
@@ -253,10 +282,14 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
         if let ExprKind::Binary(op, l, r) = expr.kind
             && (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne)
         {
-            let non_null_path_snippet = match (is_null_path(cx, l), is_null_path(cx, r)) {
-                (true, false) if let Some(sugg) = Sugg::hir_opt(cx, r) => sugg.maybe_par(),
-                (false, true) if let Some(sugg) = Sugg::hir_opt(cx, l) => sugg.maybe_par(),
-                _ => return,
+            let non_null_path_snippet = match (
+                is_lint_allowed(cx, CMP_NULL, expr.hir_id),
+                is_null_path(cx, l),
+                is_null_path(cx, r),
+            ) {
+                (false, true, false) if let Some(sugg) = Sugg::hir_opt(cx, r) => sugg.maybe_par(),
+                (false, false, true) if let Some(sugg) = Sugg::hir_opt(cx, l) => sugg.maybe_par(),
+                _ => return check_ptr_eq(cx, expr, op.node, l, r),
             };
 
             span_lint_and_sugg(
@@ -740,3 +773,71 @@ fn is_null_path(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
         false
     }
 }
+
+fn check_ptr_eq<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    op: BinOpKind,
+    left: &'tcx Expr<'_>,
+    right: &'tcx Expr<'_>,
+) {
+    if expr.span.from_expansion() {
+        return;
+    }
+
+    // Remove one level of usize conversion if any
+    let (left, right) = match (expr_as_cast_to_usize(cx, left), expr_as_cast_to_usize(cx, right)) {
+        (Some(lhs), Some(rhs)) => (lhs, rhs),
+        _ => (left, right),
+    };
+
+    // This lint concerns raw pointers
+    let (left_ty, right_ty) = (cx.typeck_results().expr_ty(left), cx.typeck_results().expr_ty(right));
+    if !left_ty.is_raw_ptr() || !right_ty.is_raw_ptr() {
+        return;
+    }
+
+    let (left_var, right_var) = (peel_raw_casts(cx, left, left_ty), peel_raw_casts(cx, right, right_ty));
+
+    if let Some(left_snip) = left_var.span.get_source_text(cx)
+        && let Some(right_snip) = right_var.span.get_source_text(cx)
+    {
+        let Some(top_crate) = std_or_core(cx) else { return };
+        let invert = if op == BinOpKind::Eq { "" } else { "!" };
+        span_lint_and_sugg(
+            cx,
+            PTR_EQ,
+            expr.span,
+            format!("use `{top_crate}::ptr::eq` when comparing raw pointers"),
+            "try",
+            format!("{invert}{top_crate}::ptr::eq({left_snip}, {right_snip})"),
+            Applicability::MachineApplicable,
+        );
+    }
+}
+
+// If the given expression is a cast to a usize, return the lhs of the cast
+// E.g., `foo as *const _ as usize` returns `foo as *const _`.
+fn expr_as_cast_to_usize<'tcx>(cx: &LateContext<'tcx>, cast_expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
+    if cx.typeck_results().expr_ty(cast_expr) == cx.tcx.types.usize
+        && let ExprKind::Cast(expr, _) = cast_expr.kind
+    {
+        Some(expr)
+    } else {
+        None
+    }
+}
+
+// Peel raw casts if the remaining expression can be coerced to it
+fn peel_raw_casts<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, expr_ty: Ty<'tcx>) -> &'tcx Expr<'tcx> {
+    if let ExprKind::Cast(inner, _) = expr.kind
+        && let ty::RawPtr(target_ty, _) = expr_ty.kind()
+        && let inner_ty = cx.typeck_results().expr_ty(inner)
+        && let ty::RawPtr(inner_target_ty, _) | ty::Ref(_, inner_target_ty, _) = inner_ty.kind()
+        && target_ty == inner_target_ty
+    {
+        peel_raw_casts(cx, inner, inner_ty)
+    } else {
+        expr
+    }
+}

clippy_lints/src/ptr.rs

@@ -20,8 +20,10 @@ fn main() {
     //~^ ptr_eq
     let _ = std::ptr::eq(a, b);
     //~^ ptr_eq
-    let _ = a.as_ptr() == b as *const _;
-    let _ = a.as_ptr() == b.as_ptr();
+    let _ = std::ptr::eq(a.as_ptr(), b as *const _);
+    //~^ ptr_eq
+    let _ = std::ptr::eq(a.as_ptr(), b.as_ptr());
+    //~^ ptr_eq
 
     // Do not lint
 
@@ -31,9 +33,22 @@ fn main() {
     let a = &mut [1, 2, 3];
     let b = &mut [1, 2, 3];
 
-    let _ = a.as_mut_ptr() == b as *mut [i32] as *mut _;
-    let _ = a.as_mut_ptr() == b.as_mut_ptr();
+    let _ = std::ptr::eq(a.as_mut_ptr(), b as *mut [i32] as *mut _);
+    //~^ ptr_eq
+    let _ = std::ptr::eq(a.as_mut_ptr(), b.as_mut_ptr());
+    //~^ ptr_eq
 
     let _ = a == b;
     let _ = core::ptr::eq(a, b);
+
+    let (x, y) = (&0u32, &mut 1u32);
+    let _ = std::ptr::eq(x, y);
+    //~^ ptr_eq
+
+    let _ = !std::ptr::eq(x, y);
+    //~^ ptr_eq
+
+    #[allow(clippy::eq_op)]
+    let _issue14337 = std::ptr::eq(main as *const (), main as *const ());
+    //~^ ptr_eq
 }

tests/ui/ptr_eq.fixed

@@ -21,7 +21,9 @@ fn main() {
     let _ = a as *const _ == b as *const _;
     //~^ ptr_eq
     let _ = a.as_ptr() == b as *const _;
+    //~^ ptr_eq
     let _ = a.as_ptr() == b.as_ptr();
+    //~^ ptr_eq
 
     // Do not lint
 
@@ -32,8 +34,21 @@ fn main() {
     let b = &mut [1, 2, 3];
 
     let _ = a.as_mut_ptr() == b as *mut [i32] as *mut _;
+    //~^ ptr_eq
     let _ = a.as_mut_ptr() == b.as_mut_ptr();
+    //~^ ptr_eq
 
     let _ = a == b;
     let _ = core::ptr::eq(a, b);
+
+    let (x, y) = (&0u32, &mut 1u32);
+    let _ = x as *const u32 == y as *mut u32 as *const u32;
+    //~^ ptr_eq
+
+    let _ = x as *const u32 != y as *mut u32 as *const u32;
+    //~^ ptr_eq
+
+    #[allow(clippy::eq_op)]
+    let _issue14337 = main as *const () == main as *const ();
+    //~^ ptr_eq
 }

tests/ui/ptr_eq.rs

@@ -13,5 +13,47 @@ error: use `std::ptr::eq` when comparing raw pointers
 LL |     let _ = a as *const _ == b as *const _;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a, b)`
 
-error: aborting due to 2 previous errors
+error: use `std::ptr::eq` when comparing raw pointers
+  --> tests/ui/ptr_eq.rs:23:13
+   |
+LL |     let _ = a.as_ptr() == b as *const _;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a.as_ptr(), b as *const _)`
+
+error: use `std::ptr::eq` when comparing raw pointers
+  --> tests/ui/ptr_eq.rs:25:13
+   |
+LL |     let _ = a.as_ptr() == b.as_ptr();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a.as_ptr(), b.as_ptr())`
+
+error: use `std::ptr::eq` when comparing raw pointers
+  --> tests/ui/ptr_eq.rs:36:13
+   |
+LL |     let _ = a.as_mut_ptr() == b as *mut [i32] as *mut _;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a.as_mut_ptr(), b as *mut [i32] as *mut _)`
+
+error: use `std::ptr::eq` when comparing raw pointers
+  --> tests/ui/ptr_eq.rs:38:13
+   |
+LL |     let _ = a.as_mut_ptr() == b.as_mut_ptr();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a.as_mut_ptr(), b.as_mut_ptr())`
+
+error: use `std::ptr::eq` when comparing raw pointers
+  --> tests/ui/ptr_eq.rs:45:13
+   |
+LL |     let _ = x as *const u32 == y as *mut u32 as *const u32;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(x, y)`
+
+error: use `std::ptr::eq` when comparing raw pointers
+  --> tests/ui/ptr_eq.rs:48:13
+   |
+LL |     let _ = x as *const u32 != y as *mut u32 as *const u32;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!std::ptr::eq(x, y)`
+
+error: use `std::ptr::eq` when comparing raw pointers
+  --> tests/ui/ptr_eq.rs:52:23
+   |
+LL |     let _issue14337 = main as *const () == main as *const ();
+   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(main as *const (), main as *const ())`
+
+error: aborting due to 9 previous errors