Auto merge of #6130 - Ambroisie:lint-ptr-eq, r=Manishearth

New lint: Recommend using `ptr::eq` when possible

This is based almost entirely on the code available in the previous PR #4596. I merely updated the code to make it compile.

Fixes #3661.

- [ ] I'm not sure about the lint name, but it was the one used in the original PR.
- [X] Added passing UI tests (including committed `.stderr` file)
- [X] `cargo test` passes locally
- [X] Executed `cargo dev update_lints`
- [X] Added lint documentation
- [X] Run `cargo dev fmt`

---

changelog: none

Author: bors

CHANGELOG.md

@@ -1892,6 +1892,7 @@ Released 2018-09-13
 [`print_with_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_with_newline
 [`println_empty_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#println_empty_string
 [`ptr_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg
+[`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
 [`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
 [`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names
 [`question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#question_mark

clippy_lints/src/lib.rs

@@ -281,6 +281,7 @@ mod path_buf_push_overwrite;
 mod pattern_type_mismatch;
 mod precedence;
 mod ptr;
+mod ptr_eq;
 mod ptr_offset_with_cast;
 mod question_mark;
 mod ranges;
@@ -778,6 +779,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &ptr::CMP_NULL,
         &ptr::MUT_FROM_REF,
         &ptr::PTR_ARG,
+        &ptr_eq::PTR_EQ,
         &ptr_offset_with_cast::PTR_OFFSET_WITH_CAST,
         &question_mark::QUESTION_MARK,
         &ranges::RANGE_MINUS_ONE,
@@ -916,6 +918,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     let verbose_bit_mask_threshold = conf.verbose_bit_mask_threshold;
     store.register_late_pass(move || box bit_mask::BitMask::new(verbose_bit_mask_threshold));
     store.register_late_pass(|| box ptr::Ptr);
+    store.register_late_pass(|| box ptr_eq::PtrEq);
     store.register_late_pass(|| box needless_bool::NeedlessBool);
     store.register_late_pass(|| box needless_bool::BoolComparison);
     store.register_late_pass(|| box approx_const::ApproxConstant);
@@ -1457,6 +1460,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&ptr::CMP_NULL),
         LintId::of(&ptr::MUT_FROM_REF),
         LintId::of(&ptr::PTR_ARG),
+        LintId::of(&ptr_eq::PTR_EQ),
         LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST),
         LintId::of(&question_mark::QUESTION_MARK),
         LintId::of(&ranges::RANGE_ZIP_WITH_LEN),
@@ -1611,6 +1615,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&panic_unimplemented::PANIC_PARAMS),
         LintId::of(&ptr::CMP_NULL),
         LintId::of(&ptr::PTR_ARG),
+        LintId::of(&ptr_eq::PTR_EQ),
         LintId::of(&question_mark::QUESTION_MARK),
         LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES),
         LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),

clippy_lints/src/ptr_eq.rs

@@ -0,0 +1,96 @@
+use crate::utils;
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+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.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let a = &[1, 2, 3];
+    /// let b = &[1, 2, 3];
+    ///
+    /// assert!(a as *const _ as usize == b as *const _ as usize);
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let a = &[1, 2, 3];
+    /// let b = &[1, 2, 3];
+    ///
+    /// assert!(std::ptr::eq(a, b));
+    /// ```
+    pub PTR_EQ,
+    style,
+    "use `std::ptr::eq` when comparing raw pointers"
+}
+
+declare_lint_pass!(PtrEq => [PTR_EQ]);
+
+static LINT_MSG: &str = "use `std::ptr::eq` when comparing raw pointers";
+
+impl LateLintPass<'_> for PtrEq {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if utils::in_macro(expr.span) {
+            return;
+        }
+
+        if let ExprKind::Binary(ref op, ref left, ref right) = expr.kind {
+            if BinOpKind::Eq == op.node {
+                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),
+                };
+
+                if_chain! {
+                    if let Some(left_var) = expr_as_cast_to_raw_pointer(cx, left);
+                    if let Some(right_var) = expr_as_cast_to_raw_pointer(cx, right);
+                    if let Some(left_snip) = utils::snippet_opt(cx, left_var.span);
+                    if let Some(right_snip) = utils::snippet_opt(cx, right_var.span);
+                    then {
+                        utils::span_lint_and_sugg(
+                            cx,
+                            PTR_EQ,
+                            expr.span,
+                            LINT_MSG,
+                            "try",
+                            format!("std::ptr::eq({}, {})", left_snip, right_snip),
+                            Applicability::MachineApplicable,
+                            );
+                    }
+                }
+            }
+        }
+    }
+}
+
+// If the given expression is a cast to an 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 {
+        if let ExprKind::Cast(ref expr, _) = cast_expr.kind {
+            return Some(expr);
+        }
+    }
+    None
+}
+
+// If the given expression is a cast to a `*const` pointer, return the lhs of the cast
+// E.g., `foo as *const _` returns `foo`.
+fn expr_as_cast_to_raw_pointer<'tcx>(cx: &LateContext<'tcx>, cast_expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
+    if cx.typeck_results().expr_ty(cast_expr).is_unsafe_ptr() {
+        if let ExprKind::Cast(ref expr, _) = cast_expr.kind {
+            return Some(expr);
+        }
+    }
+    None
+}

src/lintlist/mod.rs

@@ -1844,6 +1844,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         deprecation: None,
         module: "ptr",
     },
+    Lint {
+        name: "ptr_eq",
+        group: "style",
+        desc: "use `std::ptr::eq` when comparing raw pointers",
+        deprecation: None,
+        module: "ptr_eq",
+    },
     Lint {
         name: "ptr_offset_with_cast",
         group: "complexity",

tests/ui/ptr_eq.fixed

@@ -0,0 +1,38 @@
+// run-rustfix
+#![warn(clippy::ptr_eq)]
+
+macro_rules! mac {
+    ($a:expr, $b:expr) => {
+        $a as *const _ as usize == $b as *const _ as usize
+    };
+}
+
+macro_rules! another_mac {
+    ($a:expr, $b:expr) => {
+        $a as *const _ == $b as *const _
+    };
+}
+
+fn main() {
+    let a = &[1, 2, 3];
+    let b = &[1, 2, 3];
+
+    let _ = std::ptr::eq(a, b);
+    let _ = std::ptr::eq(a, b);
+    let _ = a.as_ptr() == b as *const _;
+    let _ = a.as_ptr() == b.as_ptr();
+
+    // Do not lint
+
+    let _ = mac!(a, b);
+    let _ = another_mac!(a, b);
+
+    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 _ = a == b;
+    let _ = core::ptr::eq(a, b);
+}

tests/ui/ptr_eq.rs

@@ -0,0 +1,38 @@
+// run-rustfix
+#![warn(clippy::ptr_eq)]
+
+macro_rules! mac {
+    ($a:expr, $b:expr) => {
+        $a as *const _ as usize == $b as *const _ as usize
+    };
+}
+
+macro_rules! another_mac {
+    ($a:expr, $b:expr) => {
+        $a as *const _ == $b as *const _
+    };
+}
+
+fn main() {
+    let a = &[1, 2, 3];
+    let b = &[1, 2, 3];
+
+    let _ = a as *const _ as usize == b as *const _ as usize;
+    let _ = a as *const _ == b as *const _;
+    let _ = a.as_ptr() == b as *const _;
+    let _ = a.as_ptr() == b.as_ptr();
+
+    // Do not lint
+
+    let _ = mac!(a, b);
+    let _ = another_mac!(a, b);
+
+    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 _ = a == b;
+    let _ = core::ptr::eq(a, b);
+}

tests/ui/ptr_eq.stderr

@@ -0,0 +1,16 @@
+error: use `std::ptr::eq` when comparing raw pointers
+  --> $DIR/ptr_eq.rs:20:13
+   |
+LL |     let _ = a as *const _ as usize == b as *const _ as usize;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a, b)`
+   |
+   = note: `-D clippy::ptr-eq` implied by `-D warnings`
+
+error: use `std::ptr::eq` when comparing raw pointers
+  --> $DIR/ptr_eq.rs:21:13
+   |
+LL |     let _ = a as *const _ == b as *const _;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a, b)`
+
+error: aborting due to 2 previous errors
+