Skip to content

Commit b5a9c82

Browse files
tesujiphansch
andcommitted
New lint: Recommend using ptr::eq when possible
Co-Authored-By: Phil Hansch <[email protected]>
1 parent b245fbd commit b5a9c82

File tree

8 files changed

+198
-2
lines changed

8 files changed

+198
-2
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,7 @@ Released 2018-09-13
11561156
[`print_with_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_with_newline
11571157
[`println_empty_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#println_empty_string
11581158
[`ptr_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg
1159+
[`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
11591160
[`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
11601161
[`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names
11611162
[`question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#question_mark

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

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

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

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

clippy_lints/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,7 @@ pub mod partialeq_ne_impl;
264264
pub mod path_buf_push_overwrite;
265265
pub mod precedence;
266266
pub mod ptr;
267+
pub mod ptr_eq;
267268
pub mod ptr_offset_with_cast;
268269
pub mod question_mark;
269270
pub mod ranges;
@@ -698,6 +699,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
698699
&ptr::CMP_NULL,
699700
&ptr::MUT_FROM_REF,
700701
&ptr::PTR_ARG,
702+
&ptr_eq::PTR_EQ,
701703
&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST,
702704
&question_mark::QUESTION_MARK,
703705
&ranges::ITERATOR_STEP_BY_ZERO,
@@ -809,6 +811,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
809811
let verbose_bit_mask_threshold = conf.verbose_bit_mask_threshold;
810812
store.register_late_pass(move || box bit_mask::BitMask::new(verbose_bit_mask_threshold));
811813
store.register_late_pass(|| box ptr::Ptr);
814+
store.register_late_pass(|| box ptr_eq::PtrEqLint);
812815
store.register_late_pass(|| box needless_bool::NeedlessBool);
813816
store.register_late_pass(|| box needless_bool::BoolComparison);
814817
store.register_late_pass(|| box approx_const::ApproxConstant);
@@ -1243,6 +1246,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
12431246
LintId::of(&ptr::CMP_NULL),
12441247
LintId::of(&ptr::MUT_FROM_REF),
12451248
LintId::of(&ptr::PTR_ARG),
1249+
LintId::of(&ptr_eq::PTR_EQ),
12461250
LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST),
12471251
LintId::of(&question_mark::QUESTION_MARK),
12481252
LintId::of(&ranges::ITERATOR_STEP_BY_ZERO),
@@ -1386,6 +1390,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
13861390
LintId::of(&panic_unimplemented::PANIC_PARAMS),
13871391
LintId::of(&ptr::CMP_NULL),
13881392
LintId::of(&ptr::PTR_ARG),
1393+
LintId::of(&ptr_eq::PTR_EQ),
13891394
LintId::of(&question_mark::QUESTION_MARK),
13901395
LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES),
13911396
LintId::of(&redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING),

clippy_lints/src/ptr_eq.rs

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
use crate::utils;
2+
use if_chain::if_chain;
3+
use rustc::declare_lint_pass;
4+
use rustc::hir::{BinOpKind, Expr, ExprKind};
5+
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
6+
use rustc_errors::Applicability;
7+
use rustc_session::declare_tool_lint;
8+
9+
declare_clippy_lint! {
10+
/// **What it does:** Use `std::ptr::eq` when applicable
11+
///
12+
/// **Why is this bad?** `ptr::eq` can be used to compare `&T` references
13+
/// (which coerce to `*const T` implicitly) by their address rather than
14+
/// comparing the values they point to.
15+
///
16+
/// **Known problems:** None
17+
///
18+
/// **Example:**
19+
/// ```rust
20+
/// let a = &[1, 2, 3];
21+
/// let b = &[1, 2, 3];
22+
///
23+
/// assert!(a as *const _ as usize == b as *const _ as usize);
24+
/// // Could be written
25+
/// assert!(std::ptr::eq(a, b));
26+
/// ```
27+
pub PTR_EQ,
28+
style,
29+
"use `std::ptr::eq` when comparing raw pointers"
30+
}
31+
32+
declare_lint_pass!(PtrEqLint => [PTR_EQ]);
33+
34+
static LINT_MSG: &str = "use `std::ptr::eq` when comparing raw pointers";
35+
36+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PtrEqLint {
37+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
38+
if utils::in_macro(expr.span) {
39+
return;
40+
}
41+
42+
if let ExprKind::Binary(ref op, ref left, ref right) = expr.kind {
43+
if BinOpKind::Eq == op.node {
44+
let (left, right) = match (expr_as_cast_to_usize(cx, left), expr_as_cast_to_usize(cx, right)) {
45+
(Some(lhs), Some(rhs)) => (lhs, rhs),
46+
_ => (&**left, &**right),
47+
};
48+
49+
if_chain! {
50+
if let Some(left_var) = expr_as_cast_to_raw_pointer(cx, left);
51+
if let Some(right_var) = expr_as_cast_to_raw_pointer(cx, right);
52+
if let Some(left_snip) = utils::snippet_opt(cx, left_var.span);
53+
if let Some(right_snip) = utils::snippet_opt(cx, right_var.span);
54+
then {
55+
utils::span_lint_and_sugg(
56+
cx,
57+
PTR_EQ,
58+
expr.span,
59+
LINT_MSG,
60+
"try",
61+
format!("std::ptr::eq({}, {})", left_snip, right_snip),
62+
Applicability::MachineApplicable,
63+
);
64+
}
65+
}
66+
}
67+
}
68+
}
69+
}
70+
71+
// If the given expression is a cast to an usize, return the lhs of the cast
72+
// E.g., `foo as *const _ as usize` returns `foo as *const _`.
73+
fn expr_as_cast_to_usize<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cast_expr: &'tcx Expr) -> Option<&'tcx Expr> {
74+
if cx.tables.expr_ty(cast_expr) == cx.tcx.types.usize {
75+
if let ExprKind::Cast(ref expr, _) = cast_expr.kind {
76+
return Some(expr);
77+
}
78+
}
79+
None
80+
}
81+
82+
// If the given expression is a cast to a `*const` pointer, return the lhs of the cast
83+
// E.g., `foo as *const _` returns `foo`.
84+
fn expr_as_cast_to_raw_pointer<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cast_expr: &'tcx Expr) -> Option<&'tcx Expr> {
85+
if cx.tables.expr_ty(cast_expr).is_unsafe_ptr() {
86+
if let ExprKind::Cast(ref expr, _) = cast_expr.kind {
87+
return Some(expr);
88+
}
89+
}
90+
None
91+
}

src/lintlist/mod.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub use lint::Lint;
66
pub use lint::LINT_LEVELS;
77

88
// begin lint list, do not remove this comment, it’s used in `update_lints`
9-
pub const ALL_LINTS: [Lint; 340] = [
9+
pub const ALL_LINTS: [Lint; 341] = [
1010
Lint {
1111
name: "absurd_extreme_comparisons",
1212
group: "correctness",
@@ -1575,6 +1575,13 @@ pub const ALL_LINTS: [Lint; 340] = [
15751575
deprecation: None,
15761576
module: "ptr",
15771577
},
1578+
Lint {
1579+
name: "ptr_eq",
1580+
group: "style",
1581+
desc: "use `std::ptr::eq` when applicable",
1582+
deprecation: None,
1583+
module: "ptr_eq",
1584+
},
15781585
Lint {
15791586
name: "ptr_offset_with_cast",
15801587
group: "complexity",

tests/ui/ptr_eq.fixed

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// run-rustfix
2+
#![warn(clippy::ptr_eq)]
3+
4+
macro_rules! mac {
5+
($a:expr, $b:expr) => {
6+
$a as *const _ as usize == $b as *const _ as usize
7+
};
8+
}
9+
10+
macro_rules! another_mac {
11+
($a:expr, $b:expr) => {
12+
$a as *const _ == $b as *const _
13+
};
14+
}
15+
16+
fn main() {
17+
let a = &[1, 2, 3];
18+
let b = &[1, 2, 3];
19+
20+
let _ = std::ptr::eq(a, b);
21+
let _ = std::ptr::eq(a, b);
22+
let _ = a.as_ptr() == b as *const _;
23+
let _ = a.as_ptr() == b.as_ptr();
24+
25+
// Do not lint
26+
27+
let _ = mac!(a, b);
28+
let _ = another_mac!(a, b);
29+
30+
let a = &mut [1, 2, 3];
31+
let b = &mut [1, 2, 3];
32+
33+
let _ = a.as_mut_ptr() == b as *mut [i32] as *mut _;
34+
let _ = a.as_mut_ptr() == b.as_mut_ptr();
35+
36+
let _ = a == b;
37+
let _ = core::ptr::eq(a, b);
38+
}

tests/ui/ptr_eq.rs

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// run-rustfix
2+
#![warn(clippy::ptr_eq)]
3+
4+
macro_rules! mac {
5+
($a:expr, $b:expr) => {
6+
$a as *const _ as usize == $b as *const _ as usize
7+
};
8+
}
9+
10+
macro_rules! another_mac {
11+
($a:expr, $b:expr) => {
12+
$a as *const _ == $b as *const _
13+
};
14+
}
15+
16+
fn main() {
17+
let a = &[1, 2, 3];
18+
let b = &[1, 2, 3];
19+
20+
let _ = a as *const _ as usize == b as *const _ as usize;
21+
let _ = a as *const _ == b as *const _;
22+
let _ = a.as_ptr() == b as *const _;
23+
let _ = a.as_ptr() == b.as_ptr();
24+
25+
// Do not lint
26+
27+
let _ = mac!(a, b);
28+
let _ = another_mac!(a, b);
29+
30+
let a = &mut [1, 2, 3];
31+
let b = &mut [1, 2, 3];
32+
33+
let _ = a.as_mut_ptr() == b as *mut [i32] as *mut _;
34+
let _ = a.as_mut_ptr() == b.as_mut_ptr();
35+
36+
let _ = a == b;
37+
let _ = core::ptr::eq(a, b);
38+
}

tests/ui/ptr_eq.stderr

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: use `std::ptr::eq` when comparing raw pointers
2+
--> $DIR/ptr_eq.rs:20:13
3+
|
4+
LL | let _ = a as *const _ as usize == b as *const _ as usize;
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a, b)`
6+
|
7+
= note: `-D clippy::ptr-eq` implied by `-D warnings`
8+
9+
error: use `std::ptr::eq` when comparing raw pointers
10+
--> $DIR/ptr_eq.rs:21:13
11+
|
12+
LL | let _ = a as *const _ == b as *const _;
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a, b)`
14+
15+
error: aborting due to 2 previous errors
16+

0 commit comments

Comments
 (0)