From: Bruno BELANYI Date: Wed, 7 Oct 2020 09:37:32 +0000 (+0200) Subject: New lint: Recommend using `ptr::eq` when possible X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=5bad9175fb363917ffee2c2da7223e96daa2f5ac;p=rust.git New lint: Recommend using `ptr::eq` when possible This is based almost entirely on the code available in the previous PR #4596. --- diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 826a059f92a..d11eeff8032 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -281,6 +281,7 @@ macro_rules! declare_clippy_lint { 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); @@ -1456,6 +1459,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), @@ -1612,6 +1616,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), diff --git a/clippy_lints/src/ptr_eq.rs b/clippy_lints/src/ptr_eq.rs new file mode 100644 index 00000000000..a05cb6270b7 --- /dev/null +++ b/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 +} diff --git a/tests/ui/ptr_eq.fixed b/tests/ui/ptr_eq.fixed new file mode 100644 index 00000000000..209081e6e80 --- /dev/null +++ b/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); +} diff --git a/tests/ui/ptr_eq.rs b/tests/ui/ptr_eq.rs new file mode 100644 index 00000000000..69162870807 --- /dev/null +++ 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); +} diff --git a/tests/ui/ptr_eq.stderr b/tests/ui/ptr_eq.stderr new file mode 100644 index 00000000000..45d8c60382b --- /dev/null +++ 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 +