Skip to content

Commit

Permalink
New lint: Recommend using ptr::eq when possible
Browse files Browse the repository at this point in the history
Co-Authored-By: Phil Hansch <[email protected]>
  • Loading branch information
tesuji and phansch committed Dec 10, 2019
1 parent b245fbd commit edc9371
Show file tree
Hide file tree
Showing 8 changed files with 198 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 340 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 341 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ pub mod partialeq_ne_impl;
pub mod path_buf_push_overwrite;
pub mod precedence;
pub mod ptr;
pub mod ptr_eq;
pub mod ptr_offset_with_cast;
pub mod question_mark;
pub mod ranges;
Expand Down Expand Up @@ -698,6 +699,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &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::ITERATOR_STEP_BY_ZERO,
Expand Down Expand Up @@ -809,6 +811,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &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::PtrEqLint);
store.register_late_pass(|| box needless_bool::NeedlessBool);
store.register_late_pass(|| box needless_bool::BoolComparison);
store.register_late_pass(|| box approx_const::ApproxConstant);
Expand Down Expand Up @@ -1243,6 +1246,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &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::ITERATOR_STEP_BY_ZERO),
Expand Down Expand Up @@ -1386,6 +1390,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &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_pattern_matching::REDUNDANT_PATTERN_MATCHING),
Expand Down
91 changes: 91 additions & 0 deletions clippy_lints/src/ptr_eq.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
use crate::utils;
use if_chain::if_chain;
use rustc::declare_lint_pass;
use rustc::hir::{BinOpKind, Expr, ExprKind};
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc_errors::Applicability;
use rustc_session::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);
/// // Could be written
/// assert!(std::ptr::eq(a, b));
/// ```
pub PTR_EQ,
style,
"use `std::ptr::eq` when comparing raw pointers"
}

declare_lint_pass!(PtrEqLint => [PTR_EQ]);

static LINT_MSG: &str = "use `std::ptr::eq` when comparing raw pointers";

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PtrEqLint {
fn check_expr(&mut self, cx: &LateContext<'a, '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<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cast_expr: &'tcx Expr) -> Option<&'tcx Expr> {
if cx.tables.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<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cast_expr: &'tcx Expr) -> Option<&'tcx Expr> {
if cx.tables.expr_ty(cast_expr).is_unsafe_ptr() {
if let ExprKind::Cast(ref expr, _) = cast_expr.kind {
return Some(expr);
}
}
None
}
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 340] = [
pub const ALL_LINTS: [Lint; 341] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -1575,6 +1575,13 @@ pub const ALL_LINTS: [Lint; 340] = [
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",
Expand Down
38 changes: 38 additions & 0 deletions tests/ui/ptr_eq.fixed
Original file line number Diff line number Diff line change
@@ -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);
}
38 changes: 38 additions & 0 deletions tests/ui/ptr_eq.rs
Original file line number Diff line number Diff line change
@@ -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);
}
16 changes: 16 additions & 0 deletions tests/ui/ptr_eq.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit edc9371

Please sign in to comment.