Skip to content

Commit

Permalink
New lint: Recommend using ptr::eq when possible
Browse files Browse the repository at this point in the history
  • Loading branch information
tesuji committed Oct 9, 2019
1 parent db233b0 commit e7cc373
Show file tree
Hide file tree
Showing 8 changed files with 167 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,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 321 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 322 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
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,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 @@ -461,6 +462,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
reg.register_late_lint_pass(box excessive_precision::ExcessivePrecision);
reg.register_late_lint_pass(box bit_mask::BitMask::new(conf.verbose_bit_mask_threshold));
reg.register_late_lint_pass(box ptr::Ptr);
reg.register_late_lint_pass(box ptr_eq::PtrEqLint);
reg.register_late_lint_pass(box needless_bool::NeedlessBool);
reg.register_late_lint_pass(box needless_bool::BoolComparison);
reg.register_late_lint_pass(box approx_const::ApproxConstant);
Expand Down Expand Up @@ -864,6 +866,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
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 @@ -1003,6 +1006,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
panic_unimplemented::PANIC_PARAMS,
ptr::CMP_NULL,
ptr::PTR_ARG,
ptr_eq::PTR_EQ,
question_mark::QUESTION_MARK,
redundant_field_names::REDUNDANT_FIELD_NAMES,
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::hir::{BinOpKind, Expr, ExprKind};
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint};
use rustc_errors::Applicability;

declare_clippy_lint! {
/// **What it does:** Use `std::ptr::eq` when applicable
///
/// **Why is this bad?** This 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];
///
/// let _ = a as *const _ as usize == b as *const _ as usize;
/// // Could be written
/// let _ = std::ptr::eq(a, b);
/// ```
pub PTR_EQ,
style,
"use `std::ptr::eq` when applicable"
}

declare_lint_pass!(PtrEqLint => [PTR_EQ]);

static LINT_MSG: &str = "use `std::ptr::eq` when applicable";

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PtrEqLint {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprKind::Binary(ref op, ref left, ref right) = expr.kind {
if BinOpKind::Eq == op.node {
let (left, right) = if_chain! {
if let Some(lhs) = expr_as_cast_to_usize(cx, left);
if let Some(rhs) = expr_as_cast_to_usize(cx, right);
then {
(lhs, rhs)
}
else {
(&**left, &**right)
}
};

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) {
let left_snip = utils::snippet(cx, left_var.span, "..");
let right_snip = utils::snippet(cx, right_var.span, "..");
let sugg = format!("std::ptr::eq({}, {})", left_snip, right_snip);
utils::span_lint_and_sugg(
cx,
PTR_EQ,
expr.span,
LINT_MSG,
"try",
sugg,
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; 321] = [
pub const ALL_LINTS: [Lint; 322] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -1505,6 +1505,13 @@ pub const ALL_LINTS: [Lint; 321] = [
deprecation: None,
module: "ptr",
},
Lint {
name: "ptr_eq",
group: "style",
desc: "use `std::ptr::eq` when applicable",
deprecation: None,
module: "ptr_eq",
},
Lint {
name: "ptr_offset_with_cast",
group: "complexity",
Expand Down
23 changes: 23 additions & 0 deletions tests/ui/ptr_eq.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// run-rustfix
#![warn(clippy::ptr_eq)]

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 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);
}
23 changes: 23 additions & 0 deletions tests/ui/ptr_eq.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// run-rustfix
#![warn(clippy::ptr_eq)]

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 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 applicable
--> $DIR/ptr_eq.rs:8: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 applicable
--> $DIR/ptr_eq.rs:9: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 e7cc373

Please sign in to comment.