Skip to content

Commit 336d344

Browse files
committed
Apply ptr_eq lint only if cmp_null is not applicable
The `cmp_null` lint is more specialized than `ptr_eq`. The former should take precedence, unless the user allows it.
1 parent d7d0abd commit 336d344

File tree

7 files changed

+121
-113
lines changed

7 files changed

+121
-113
lines changed

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,6 @@ pub static LINTS: &[&crate::LintInfo] = &[
614614
crate::operators::MODULO_ONE_INFO,
615615
crate::operators::NEEDLESS_BITWISE_BOOL_INFO,
616616
crate::operators::OP_REF_INFO,
617-
crate::operators::PTR_EQ_INFO,
618617
crate::operators::REDUNDANT_COMPARISONS_INFO,
619618
crate::operators::SELF_ASSIGNMENT_INFO,
620619
crate::operators::VERBOSE_BIT_MASK_INFO,
@@ -641,6 +640,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
641640
crate::ptr::INVALID_NULL_PTR_USAGE_INFO,
642641
crate::ptr::MUT_FROM_REF_INFO,
643642
crate::ptr::PTR_ARG_INFO,
643+
crate::ptr::PTR_EQ_INFO,
644644
crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO,
645645
crate::pub_underscore_fields::PUB_UNDERSCORE_FIELDS_INFO,
646646
crate::pub_use::PUB_USE_INFO,

clippy_lints/src/operators/mod.rs

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ mod modulo_one;
1818
mod needless_bitwise_bool;
1919
mod numeric_arithmetic;
2020
mod op_ref;
21-
mod ptr_eq;
2221
mod self_assignment;
2322
mod verbose_bit_mask;
2423

@@ -768,35 +767,6 @@ declare_clippy_lint! {
768767
"Boolean expressions that use bitwise rather than lazy operators"
769768
}
770769

771-
declare_clippy_lint! {
772-
/// ### What it does
773-
/// Use `std::ptr::eq` when applicable
774-
///
775-
/// ### Why is this bad?
776-
/// `ptr::eq` can be used to compare `&T` references
777-
/// (which coerce to `*const T` implicitly) by their address rather than
778-
/// comparing the values they point to.
779-
///
780-
/// ### Example
781-
/// ```no_run
782-
/// let a = &[1, 2, 3];
783-
/// let b = &[1, 2, 3];
784-
///
785-
/// assert!(a as *const _ as usize == b as *const _ as usize);
786-
/// ```
787-
/// Use instead:
788-
/// ```no_run
789-
/// let a = &[1, 2, 3];
790-
/// let b = &[1, 2, 3];
791-
///
792-
/// assert!(std::ptr::eq(a, b));
793-
/// ```
794-
#[clippy::version = "1.49.0"]
795-
pub PTR_EQ,
796-
style,
797-
"use `std::ptr::eq` when comparing raw pointers"
798-
}
799-
800770
declare_clippy_lint! {
801771
/// ### What it does
802772
/// Checks for explicit self-assignments.
@@ -902,7 +872,6 @@ impl_lint_pass!(Operators => [
902872
MODULO_ONE,
903873
MODULO_ARITHMETIC,
904874
NEEDLESS_BITWISE_BOOL,
905-
PTR_EQ,
906875
SELF_ASSIGNMENT,
907876
MANUAL_MIDPOINT,
908877
]);
@@ -921,7 +890,6 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
921890
erasing_op::check(cx, e, op.node, lhs, rhs);
922891
identity_op::check(cx, e, op.node, lhs, rhs);
923892
needless_bitwise_bool::check(cx, e, op.node, lhs, rhs);
924-
ptr_eq::check(cx, e, op.node, lhs, rhs);
925893
manual_midpoint::check(cx, e, op.node, lhs, rhs, self.msrv);
926894
}
927895
self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);

clippy_lints/src/operators/ptr_eq.rs

Lines changed: 0 additions & 73 deletions
This file was deleted.

clippy_lints/src/ptr.rs

Lines changed: 106 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,36 @@ declare_clippy_lint! {
148148
"invalid usage of a null pointer, suggesting `NonNull::dangling()` instead"
149149
}
150150

151-
declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF, INVALID_NULL_PTR_USAGE]);
151+
declare_clippy_lint! {
152+
/// ### What it does
153+
/// Use `std::ptr::eq` when applicable
154+
///
155+
/// ### Why is this bad?
156+
/// `ptr::eq` can be used to compare `&T` references
157+
/// (which coerce to `*const T` implicitly) by their address rather than
158+
/// comparing the values they point to.
159+
///
160+
/// ### Example
161+
/// ```no_run
162+
/// let a = &[1, 2, 3];
163+
/// let b = &[1, 2, 3];
164+
///
165+
/// assert!(a as *const _ as usize == b as *const _ as usize);
166+
/// ```
167+
/// Use instead:
168+
/// ```no_run
169+
/// let a = &[1, 2, 3];
170+
/// let b = &[1, 2, 3];
171+
///
172+
/// assert!(std::ptr::eq(a, b));
173+
/// ```
174+
#[clippy::version = "1.49.0"]
175+
pub PTR_EQ,
176+
style,
177+
"use `std::ptr::eq` when comparing raw pointers"
178+
}
179+
180+
declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF, INVALID_NULL_PTR_USAGE, PTR_EQ]);
152181

153182
impl<'tcx> LateLintPass<'tcx> for Ptr {
154183
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
@@ -253,10 +282,14 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
253282
if let ExprKind::Binary(op, l, r) = expr.kind
254283
&& (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne)
255284
{
256-
let non_null_path_snippet = match (is_null_path(cx, l), is_null_path(cx, r)) {
257-
(true, false) if let Some(sugg) = Sugg::hir_opt(cx, r) => sugg.maybe_par(),
258-
(false, true) if let Some(sugg) = Sugg::hir_opt(cx, l) => sugg.maybe_par(),
259-
_ => return,
285+
let non_null_path_snippet = match (
286+
is_lint_allowed(cx, CMP_NULL, expr.hir_id),
287+
is_null_path(cx, l),
288+
is_null_path(cx, r),
289+
) {
290+
(false, true, false) if let Some(sugg) = Sugg::hir_opt(cx, r) => sugg.maybe_par(),
291+
(false, false, true) if let Some(sugg) = Sugg::hir_opt(cx, l) => sugg.maybe_par(),
292+
_ => return check_ptr_eq(cx, expr, op.node, l, r),
260293
};
261294

262295
span_lint_and_sugg(
@@ -740,3 +773,71 @@ fn is_null_path(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
740773
false
741774
}
742775
}
776+
777+
fn check_ptr_eq<'tcx>(
778+
cx: &LateContext<'tcx>,
779+
expr: &'tcx Expr<'_>,
780+
op: BinOpKind,
781+
left: &'tcx Expr<'_>,
782+
right: &'tcx Expr<'_>,
783+
) {
784+
if expr.span.from_expansion() {
785+
return;
786+
}
787+
788+
// Remove one level of usize conversion if any
789+
let (left, right) = match (expr_as_cast_to_usize(cx, left), expr_as_cast_to_usize(cx, right)) {
790+
(Some(lhs), Some(rhs)) => (lhs, rhs),
791+
_ => (left, right),
792+
};
793+
794+
// This lint concerns raw pointers
795+
let (left_ty, right_ty) = (cx.typeck_results().expr_ty(left), cx.typeck_results().expr_ty(right));
796+
if !left_ty.is_raw_ptr() || !right_ty.is_raw_ptr() {
797+
return;
798+
}
799+
800+
let (left_var, right_var) = (peel_raw_casts(cx, left, left_ty), peel_raw_casts(cx, right, right_ty));
801+
802+
if let Some(left_snip) = left_var.span.get_source_text(cx)
803+
&& let Some(right_snip) = right_var.span.get_source_text(cx)
804+
{
805+
let Some(top_crate) = std_or_core(cx) else { return };
806+
let invert = if op == BinOpKind::Eq { "" } else { "!" };
807+
span_lint_and_sugg(
808+
cx,
809+
PTR_EQ,
810+
expr.span,
811+
format!("use `{top_crate}::ptr::eq` when comparing raw pointers"),
812+
"try",
813+
format!("{invert}{top_crate}::ptr::eq({left_snip}, {right_snip})"),
814+
Applicability::MachineApplicable,
815+
);
816+
}
817+
}
818+
819+
// If the given expression is a cast to a usize, return the lhs of the cast
820+
// E.g., `foo as *const _ as usize` returns `foo as *const _`.
821+
fn expr_as_cast_to_usize<'tcx>(cx: &LateContext<'tcx>, cast_expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
822+
if cx.typeck_results().expr_ty(cast_expr) == cx.tcx.types.usize
823+
&& let ExprKind::Cast(expr, _) = cast_expr.kind
824+
{
825+
Some(expr)
826+
} else {
827+
None
828+
}
829+
}
830+
831+
// Peel raw casts if the remaining expression can be coerced to it
832+
fn peel_raw_casts<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, expr_ty: Ty<'tcx>) -> &'tcx Expr<'tcx> {
833+
if let ExprKind::Cast(inner, _) = expr.kind
834+
&& let ty::RawPtr(target_ty, _) = expr_ty.kind()
835+
&& let inner_ty = cx.typeck_results().expr_ty(inner)
836+
&& let ty::RawPtr(inner_target_ty, _) | ty::Ref(_, inner_target_ty, _) = inner_ty.kind()
837+
&& target_ty == inner_target_ty
838+
{
839+
peel_raw_casts(cx, inner, inner_ty)
840+
} else {
841+
expr
842+
}
843+
}

tests/ui/ptr_eq.fixed

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ fn main() {
4545
let _ = std::ptr::eq(x, y);
4646
//~^ ptr_eq
4747

48+
let _ = !std::ptr::eq(x, y);
49+
//~^ ptr_eq
50+
4851
#[allow(clippy::eq_op)]
4952
let _issue14337 = std::ptr::eq(main as *const (), main as *const ());
5053
//~^ ptr_eq

tests/ui/ptr_eq.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ fn main() {
4545
let _ = x as *const u32 == y as *mut u32 as *const u32;
4646
//~^ ptr_eq
4747

48+
let _ = x as *const u32 != y as *mut u32 as *const u32;
49+
//~^ ptr_eq
50+
4851
#[allow(clippy::eq_op)]
4952
let _issue14337 = main as *const () == main as *const ();
5053
//~^ ptr_eq

tests/ui/ptr_eq.stderr

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,16 @@ LL | let _ = x as *const u32 == y as *mut u32 as *const u32;
4444
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(x, y)`
4545

4646
error: use `std::ptr::eq` when comparing raw pointers
47-
--> tests/ui/ptr_eq.rs:49:23
47+
--> tests/ui/ptr_eq.rs:48:13
48+
|
49+
LL | let _ = x as *const u32 != y as *mut u32 as *const u32;
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!std::ptr::eq(x, y)`
51+
52+
error: use `std::ptr::eq` when comparing raw pointers
53+
--> tests/ui/ptr_eq.rs:52:23
4854
|
4955
LL | let _issue14337 = main as *const () == main as *const ();
5056
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(main as *const (), main as *const ())`
5157

52-
error: aborting due to 8 previous errors
58+
error: aborting due to 9 previous errors
5359

0 commit comments

Comments
 (0)