Skip to content

Lint more cases with ptr_eq #14339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,6 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::operators::MODULO_ONE_INFO,
crate::operators::NEEDLESS_BITWISE_BOOL_INFO,
crate::operators::OP_REF_INFO,
crate::operators::PTR_EQ_INFO,
crate::operators::REDUNDANT_COMPARISONS_INFO,
crate::operators::SELF_ASSIGNMENT_INFO,
crate::operators::VERBOSE_BIT_MASK_INFO,
Expand All @@ -641,6 +640,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::ptr::INVALID_NULL_PTR_USAGE_INFO,
crate::ptr::MUT_FROM_REF_INFO,
crate::ptr::PTR_ARG_INFO,
crate::ptr::PTR_EQ_INFO,
crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO,
crate::pub_underscore_fields::PUB_UNDERSCORE_FIELDS_INFO,
crate::pub_use::PUB_USE_INFO,
Expand Down
32 changes: 0 additions & 32 deletions clippy_lints/src/operators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ mod modulo_one;
mod needless_bitwise_bool;
mod numeric_arithmetic;
mod op_ref;
mod ptr_eq;
mod self_assignment;
mod verbose_bit_mask;

Expand Down Expand Up @@ -768,35 +767,6 @@ declare_clippy_lint! {
"Boolean expressions that use bitwise rather than lazy operators"
}

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.
///
/// ### Example
/// ```no_run
/// let a = &[1, 2, 3];
/// let b = &[1, 2, 3];
///
/// assert!(a as *const _ as usize == b as *const _ as usize);
/// ```
/// Use instead:
/// ```no_run
/// let a = &[1, 2, 3];
/// let b = &[1, 2, 3];
///
/// assert!(std::ptr::eq(a, b));
/// ```
#[clippy::version = "1.49.0"]
pub PTR_EQ,
style,
"use `std::ptr::eq` when comparing raw pointers"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for explicit self-assignments.
Expand Down Expand Up @@ -902,7 +872,6 @@ impl_lint_pass!(Operators => [
MODULO_ONE,
MODULO_ARITHMETIC,
NEEDLESS_BITWISE_BOOL,
PTR_EQ,
SELF_ASSIGNMENT,
MANUAL_MIDPOINT,
]);
Expand All @@ -921,7 +890,6 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
erasing_op::check(cx, e, op.node, lhs, rhs);
identity_op::check(cx, e, op.node, lhs, rhs);
needless_bitwise_bool::check(cx, e, op.node, lhs, rhs);
ptr_eq::check(cx, e, op.node, lhs, rhs);
manual_midpoint::check(cx, e, op.node, lhs, rhs, self.msrv);
}
self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);
Expand Down
62 changes: 0 additions & 62 deletions clippy_lints/src/operators/ptr_eq.rs

This file was deleted.

111 changes: 106 additions & 5 deletions clippy_lints/src/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,36 @@ declare_clippy_lint! {
"invalid usage of a null pointer, suggesting `NonNull::dangling()` instead"
}

declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF, INVALID_NULL_PTR_USAGE]);
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.
///
/// ### Example
/// ```no_run
/// let a = &[1, 2, 3];
/// let b = &[1, 2, 3];
///
/// assert!(a as *const _ as usize == b as *const _ as usize);
/// ```
/// Use instead:
/// ```no_run
/// let a = &[1, 2, 3];
/// let b = &[1, 2, 3];
///
/// assert!(std::ptr::eq(a, b));
/// ```
#[clippy::version = "1.49.0"]
pub PTR_EQ,
style,
"use `std::ptr::eq` when comparing raw pointers"
}

declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF, INVALID_NULL_PTR_USAGE, PTR_EQ]);

impl<'tcx> LateLintPass<'tcx> for Ptr {
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
Expand Down Expand Up @@ -253,10 +282,14 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
if let ExprKind::Binary(op, l, r) = expr.kind
&& (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne)
{
let non_null_path_snippet = match (is_null_path(cx, l), is_null_path(cx, r)) {
(true, false) if let Some(sugg) = Sugg::hir_opt(cx, r) => sugg.maybe_par(),
(false, true) if let Some(sugg) = Sugg::hir_opt(cx, l) => sugg.maybe_par(),
_ => return,
let non_null_path_snippet = match (
is_lint_allowed(cx, CMP_NULL, expr.hir_id),
is_null_path(cx, l),
is_null_path(cx, r),
) {
(false, true, false) if let Some(sugg) = Sugg::hir_opt(cx, r) => sugg.maybe_par(),
(false, false, true) if let Some(sugg) = Sugg::hir_opt(cx, l) => sugg.maybe_par(),
_ => return check_ptr_eq(cx, expr, op.node, l, r),
};

span_lint_and_sugg(
Expand Down Expand Up @@ -740,3 +773,71 @@ fn is_null_path(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
false
}
}

fn check_ptr_eq<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
op: BinOpKind,
left: &'tcx Expr<'_>,
right: &'tcx Expr<'_>,
) {
if expr.span.from_expansion() {
return;
}

// Remove one level of usize conversion if any
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),
};

// This lint concerns raw pointers
let (left_ty, right_ty) = (cx.typeck_results().expr_ty(left), cx.typeck_results().expr_ty(right));
if !left_ty.is_raw_ptr() || !right_ty.is_raw_ptr() {
return;
}

let (left_var, right_var) = (peel_raw_casts(cx, left, left_ty), peel_raw_casts(cx, right, right_ty));

if let Some(left_snip) = left_var.span.get_source_text(cx)
&& let Some(right_snip) = right_var.span.get_source_text(cx)
{
let Some(top_crate) = std_or_core(cx) else { return };
let invert = if op == BinOpKind::Eq { "" } else { "!" };
span_lint_and_sugg(
cx,
PTR_EQ,
expr.span,
format!("use `{top_crate}::ptr::eq` when comparing raw pointers"),
"try",
format!("{invert}{top_crate}::ptr::eq({left_snip}, {right_snip})"),
Applicability::MachineApplicable,
);
}
}

// If the given expression is a cast to a 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
&& let ExprKind::Cast(expr, _) = cast_expr.kind
{
Some(expr)
} else {
None
}
}

// Peel raw casts if the remaining expression can be coerced to it
fn peel_raw_casts<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, expr_ty: Ty<'tcx>) -> &'tcx Expr<'tcx> {
if let ExprKind::Cast(inner, _) = expr.kind
&& let ty::RawPtr(target_ty, _) = expr_ty.kind()
&& let inner_ty = cx.typeck_results().expr_ty(inner)
&& let ty::RawPtr(inner_target_ty, _) | ty::Ref(_, inner_target_ty, _) = inner_ty.kind()
&& target_ty == inner_target_ty
{
peel_raw_casts(cx, inner, inner_ty)
} else {
expr
}
}
23 changes: 19 additions & 4 deletions tests/ui/ptr_eq.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ fn main() {
//~^ ptr_eq
let _ = std::ptr::eq(a, b);
//~^ ptr_eq
let _ = a.as_ptr() == b as *const _;
let _ = a.as_ptr() == b.as_ptr();
let _ = std::ptr::eq(a.as_ptr(), b as *const _);
//~^ ptr_eq
let _ = std::ptr::eq(a.as_ptr(), b.as_ptr());
//~^ ptr_eq

// Do not lint

Expand All @@ -31,9 +33,22 @@ fn main() {
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 _ = std::ptr::eq(a.as_mut_ptr(), b as *mut [i32] as *mut _);
//~^ ptr_eq
let _ = std::ptr::eq(a.as_mut_ptr(), b.as_mut_ptr());
//~^ ptr_eq

let _ = a == b;
let _ = core::ptr::eq(a, b);

let (x, y) = (&0u32, &mut 1u32);
let _ = std::ptr::eq(x, y);
//~^ ptr_eq

let _ = !std::ptr::eq(x, y);
//~^ ptr_eq

#[allow(clippy::eq_op)]
let _issue14337 = std::ptr::eq(main as *const (), main as *const ());
//~^ ptr_eq
}
15 changes: 15 additions & 0 deletions tests/ui/ptr_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ fn main() {
let _ = a as *const _ == b as *const _;
//~^ ptr_eq
let _ = a.as_ptr() == b as *const _;
//~^ ptr_eq
let _ = a.as_ptr() == b.as_ptr();
//~^ ptr_eq

// Do not lint

Expand All @@ -32,8 +34,21 @@ fn main() {
let b = &mut [1, 2, 3];

let _ = a.as_mut_ptr() == b as *mut [i32] as *mut _;
//~^ ptr_eq
let _ = a.as_mut_ptr() == b.as_mut_ptr();
//~^ ptr_eq

let _ = a == b;
let _ = core::ptr::eq(a, b);

let (x, y) = (&0u32, &mut 1u32);
let _ = x as *const u32 == y as *mut u32 as *const u32;
//~^ ptr_eq

let _ = x as *const u32 != y as *mut u32 as *const u32;
//~^ ptr_eq

#[allow(clippy::eq_op)]
let _issue14337 = main as *const () == main as *const ();
//~^ ptr_eq
}
44 changes: 43 additions & 1 deletion tests/ui/ptr_eq.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,47 @@ error: use `std::ptr::eq` when comparing raw pointers
LL | let _ = a as *const _ == b as *const _;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a, b)`

error: aborting due to 2 previous errors
error: use `std::ptr::eq` when comparing raw pointers
--> tests/ui/ptr_eq.rs:23:13
|
LL | let _ = a.as_ptr() == b as *const _;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a.as_ptr(), b as *const _)`

error: use `std::ptr::eq` when comparing raw pointers
--> tests/ui/ptr_eq.rs:25:13
|
LL | let _ = a.as_ptr() == b.as_ptr();
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a.as_ptr(), b.as_ptr())`

error: use `std::ptr::eq` when comparing raw pointers
--> tests/ui/ptr_eq.rs:36:13
|
LL | let _ = a.as_mut_ptr() == b as *mut [i32] as *mut _;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a.as_mut_ptr(), b as *mut [i32] as *mut _)`

error: use `std::ptr::eq` when comparing raw pointers
--> tests/ui/ptr_eq.rs:38:13
|
LL | let _ = a.as_mut_ptr() == b.as_mut_ptr();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a.as_mut_ptr(), b.as_mut_ptr())`

error: use `std::ptr::eq` when comparing raw pointers
--> tests/ui/ptr_eq.rs:45:13
|
LL | let _ = x as *const u32 == y as *mut u32 as *const u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(x, y)`

error: use `std::ptr::eq` when comparing raw pointers
--> tests/ui/ptr_eq.rs:48:13
|
LL | let _ = x as *const u32 != y as *mut u32 as *const u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!std::ptr::eq(x, y)`

error: use `std::ptr::eq` when comparing raw pointers
--> tests/ui/ptr_eq.rs:52:23
|
LL | let _issue14337 = main as *const () == main as *const ();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(main as *const (), main as *const ())`

error: aborting due to 9 previous errors

Loading