From c8b10ac6712364c9b2ab9360ef739339d6477224 Mon Sep 17 00:00:00 2001 From: cyrgani Date: Thu, 5 Dec 2024 09:46:57 +0100 Subject: [PATCH 1/2] fix the order of emitting `ref_as_ptr` and `borrow_as_ptr` --- clippy_lints/src/casts/borrow_as_ptr.rs | 9 ++++++--- clippy_lints/src/casts/mod.rs | 9 ++++++--- tests/ui/borrow_and_ref_as_ptr.fixed | 11 +++++++++++ tests/ui/borrow_and_ref_as_ptr.rs | 11 +++++++++++ tests/ui/borrow_and_ref_as_ptr.stderr | 17 +++++++++++++++++ 5 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 tests/ui/borrow_and_ref_as_ptr.fixed create mode 100644 tests/ui/borrow_and_ref_as_ptr.rs create mode 100644 tests/ui/borrow_and_ref_as_ptr.stderr diff --git a/clippy_lints/src/casts/borrow_as_ptr.rs b/clippy_lints/src/casts/borrow_as_ptr.rs index 4dd51dcbc9a20..deecbce2c4068 100644 --- a/clippy_lints/src/casts/borrow_as_ptr.rs +++ b/clippy_lints/src/casts/borrow_as_ptr.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_context; -use clippy_utils::std_or_core; +use clippy_utils::{is_lint_allowed, std_or_core}; use rustc_errors::Applicability; use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Ty, TyKind}; use rustc_lint::LateContext; @@ -13,10 +13,11 @@ pub(super) fn check<'tcx>( expr: &'tcx Expr<'_>, cast_expr: &'tcx Expr<'_>, cast_to: &'tcx Ty<'_>, -) { +) -> bool { if matches!(cast_to.kind, TyKind::Ptr(_)) && let ExprKind::AddrOf(BorrowKind::Ref, mutability, e) = cast_expr.kind && let Some(std_or_core) = std_or_core(cx) + && !is_lint_allowed(cx, BORROW_AS_PTR, expr.hir_id) { let macro_name = match mutability { Mutability::Not => "addr_of", @@ -31,7 +32,7 @@ pub(super) fn check<'tcx>( .get(base.hir_id) .is_some_and(|x| x.iter().any(|adj| matches!(adj.kind, Adjust::Deref(_)))) }) { - return; + return false; } span_lint_and_sugg( @@ -43,5 +44,7 @@ pub(super) fn check<'tcx>( format!("{std_or_core}::ptr::{macro_name}!({snip})"), Applicability::MachineApplicable, ); + return true; } + false } diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index 8b884399f9234..6f98b335fe2a6 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -806,10 +806,13 @@ impl<'tcx> LateLintPass<'tcx> for Casts { as_underscore::check(cx, expr, cast_to_hir); - if self.msrv.meets(msrvs::PTR_FROM_REF) { + let was_borrow_as_ptr_emitted = if self.msrv.meets(msrvs::BORROW_AS_PTR) { + borrow_as_ptr::check(cx, expr, cast_from_expr, cast_to_hir) + } else { + false + }; + if self.msrv.meets(msrvs::PTR_FROM_REF) && !was_borrow_as_ptr_emitted { ref_as_ptr::check(cx, expr, cast_from_expr, cast_to_hir); - } else if self.msrv.meets(msrvs::BORROW_AS_PTR) { - borrow_as_ptr::check(cx, expr, cast_from_expr, cast_to_hir); } } diff --git a/tests/ui/borrow_and_ref_as_ptr.fixed b/tests/ui/borrow_and_ref_as_ptr.fixed new file mode 100644 index 0000000000000..2950b158deb64 --- /dev/null +++ b/tests/ui/borrow_and_ref_as_ptr.fixed @@ -0,0 +1,11 @@ +// Make sure that `ref_as_ptr` is not emitted when `borrow_as_ptr` is. + +#![warn(clippy::ref_as_ptr, clippy::borrow_as_ptr)] + +fn f(_: T) {} + +fn main() { + let mut val = 0; + f(&raw const val); + f(&raw mut val); +} diff --git a/tests/ui/borrow_and_ref_as_ptr.rs b/tests/ui/borrow_and_ref_as_ptr.rs new file mode 100644 index 0000000000000..19eb8f2923374 --- /dev/null +++ b/tests/ui/borrow_and_ref_as_ptr.rs @@ -0,0 +1,11 @@ +// Make sure that `ref_as_ptr` is not emitted when `borrow_as_ptr` is. + +#![warn(clippy::ref_as_ptr, clippy::borrow_as_ptr)] + +fn f(_: T) {} + +fn main() { + let mut val = 0; + f(&val as *const _); + f(&mut val as *mut i32); +} diff --git a/tests/ui/borrow_and_ref_as_ptr.stderr b/tests/ui/borrow_and_ref_as_ptr.stderr new file mode 100644 index 0000000000000..82a27af303c21 --- /dev/null +++ b/tests/ui/borrow_and_ref_as_ptr.stderr @@ -0,0 +1,17 @@ +error: borrow as raw pointer + --> tests/ui/borrow_and_ref_as_ptr.rs:9:7 + | +LL | f(&val as *const _); + | ^^^^^^^^^^^^^^^^ help: try: `&raw const val` + | + = note: `-D clippy::borrow-as-ptr` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::borrow_as_ptr)]` + +error: borrow as raw pointer + --> tests/ui/borrow_and_ref_as_ptr.rs:10:7 + | +LL | f(&mut val as *mut i32); + | ^^^^^^^^^^^^^^^^^^^^ help: try: `&raw mut val` + +error: aborting due to 2 previous errors + From 9925f999f6a3570a251aed2039bdb6d90de5b576 Mon Sep 17 00:00:00 2001 From: cyrgani Date: Thu, 5 Dec 2024 09:48:55 +0100 Subject: [PATCH 2/2] update `borrow_as_ptr` to suggest `&raw` when the MSRV allows it --- clippy_lints/src/casts/borrow_as_ptr.rs | 28 ++++++++++++++++++------- clippy_lints/src/casts/mod.rs | 12 +++++------ clippy_utils/src/msrvs.rs | 4 ++-- tests/ui/borrow_as_ptr_raw_ref.fixed | 19 +++++++++++++++++ tests/ui/borrow_as_ptr_raw_ref.rs | 19 +++++++++++++++++ tests/ui/borrow_as_ptr_raw_ref.stderr | 17 +++++++++++++++ 6 files changed, 84 insertions(+), 15 deletions(-) create mode 100644 tests/ui/borrow_as_ptr_raw_ref.fixed create mode 100644 tests/ui/borrow_as_ptr_raw_ref.rs create mode 100644 tests/ui/borrow_as_ptr_raw_ref.stderr diff --git a/clippy_lints/src/casts/borrow_as_ptr.rs b/clippy_lints/src/casts/borrow_as_ptr.rs index deecbce2c4068..67aa33ca06c31 100644 --- a/clippy_lints/src/casts/borrow_as_ptr.rs +++ b/clippy_lints/src/casts/borrow_as_ptr.rs @@ -1,6 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::msrvs::Msrv; use clippy_utils::source::snippet_with_context; -use clippy_utils::{is_lint_allowed, std_or_core}; +use clippy_utils::{is_lint_allowed, msrvs, std_or_core}; use rustc_errors::Applicability; use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Ty, TyKind}; use rustc_lint::LateContext; @@ -13,16 +14,12 @@ pub(super) fn check<'tcx>( expr: &'tcx Expr<'_>, cast_expr: &'tcx Expr<'_>, cast_to: &'tcx Ty<'_>, + msrv: &Msrv, ) -> bool { if matches!(cast_to.kind, TyKind::Ptr(_)) && let ExprKind::AddrOf(BorrowKind::Ref, mutability, e) = cast_expr.kind - && let Some(std_or_core) = std_or_core(cx) && !is_lint_allowed(cx, BORROW_AS_PTR, expr.hir_id) { - let macro_name = match mutability { - Mutability::Not => "addr_of", - Mutability::Mut => "addr_of_mut", - }; let mut app = Applicability::MachineApplicable; let snip = snippet_with_context(cx, e.span, cast_expr.span.ctxt(), "..", &mut app).0; // Fix #9884 @@ -35,13 +32,30 @@ pub(super) fn check<'tcx>( return false; } + let suggestion = if msrv.meets(msrvs::RAW_REF_OP) { + let operator_kind = match mutability { + Mutability::Not => "const", + Mutability::Mut => "mut", + }; + format!("&raw {operator_kind} {snip}") + } else { + let Some(std_or_core) = std_or_core(cx) else { + return false; + }; + let macro_name = match mutability { + Mutability::Not => "addr_of", + Mutability::Mut => "addr_of_mut", + }; + format!("{std_or_core}::ptr::{macro_name}!({snip})") + }; + span_lint_and_sugg( cx, BORROW_AS_PTR, expr.span, "borrow as raw pointer", "try", - format!("{std_or_core}::ptr::{macro_name}!({snip})"), + suggestion, Applicability::MachineApplicable, ); return true; diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index 6f98b335fe2a6..84fab9cb75f8a 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -574,13 +574,13 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does /// Checks for the usage of `&expr as *const T` or - /// `&mut expr as *mut T`, and suggest using `ptr::addr_of` or - /// `ptr::addr_of_mut` instead. + /// `&mut expr as *mut T`, and suggest using `&raw const` or + /// `&raw mut` instead. /// /// ### Why is this bad? /// This would improve readability and avoid creating a reference /// that points to an uninitialized value or unaligned place. - /// Read the `ptr::addr_of` docs for more information. + /// Read the `&raw` explanation in the Reference for more information. /// /// ### Example /// ```no_run @@ -593,10 +593,10 @@ declare_clippy_lint! { /// Use instead: /// ```no_run /// let val = 1; - /// let p = std::ptr::addr_of!(val); + /// let p = &raw const val; /// /// let mut val_mut = 1; - /// let p_mut = std::ptr::addr_of_mut!(val_mut); + /// let p_mut = &raw mut val_mut; /// ``` #[clippy::version = "1.60.0"] pub BORROW_AS_PTR, @@ -807,7 +807,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts { as_underscore::check(cx, expr, cast_to_hir); let was_borrow_as_ptr_emitted = if self.msrv.meets(msrvs::BORROW_AS_PTR) { - borrow_as_ptr::check(cx, expr, cast_from_expr, cast_to_hir) + borrow_as_ptr::check(cx, expr, cast_from_expr, cast_to_hir, &self.msrv) } else { false }; diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 1eb7d54e133d1..f038176070956 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -19,9 +19,9 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { 1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY } - 1,82,0 { IS_NONE_OR, REPEAT_N } + 1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP } 1,81,0 { LINT_REASONS_STABILIZATION } - 1,80,0 { BOX_INTO_ITER} + 1,80,0 { BOX_INTO_ITER } 1,77,0 { C_STR_LITERALS } 1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT } 1,73,0 { MANUAL_DIV_CEIL } diff --git a/tests/ui/borrow_as_ptr_raw_ref.fixed b/tests/ui/borrow_as_ptr_raw_ref.fixed new file mode 100644 index 0000000000000..d6842e60a3e9f --- /dev/null +++ b/tests/ui/borrow_as_ptr_raw_ref.fixed @@ -0,0 +1,19 @@ +#![warn(clippy::borrow_as_ptr)] +#![allow(clippy::useless_vec)] + +fn a() -> i32 { + 0 +} + +#[clippy::msrv = "1.82"] +fn main() { + let val = 1; + let _p = &raw const val; + let _p = &0 as *const i32; + let _p = &a() as *const i32; + let vec = vec![1]; + let _p = &vec.len() as *const usize; + + let mut val_mut = 1; + let _p_mut = &raw mut val_mut; +} diff --git a/tests/ui/borrow_as_ptr_raw_ref.rs b/tests/ui/borrow_as_ptr_raw_ref.rs new file mode 100644 index 0000000000000..3c9daed18f15a --- /dev/null +++ b/tests/ui/borrow_as_ptr_raw_ref.rs @@ -0,0 +1,19 @@ +#![warn(clippy::borrow_as_ptr)] +#![allow(clippy::useless_vec)] + +fn a() -> i32 { + 0 +} + +#[clippy::msrv = "1.82"] +fn main() { + let val = 1; + let _p = &val as *const i32; + let _p = &0 as *const i32; + let _p = &a() as *const i32; + let vec = vec![1]; + let _p = &vec.len() as *const usize; + + let mut val_mut = 1; + let _p_mut = &mut val_mut as *mut i32; +} diff --git a/tests/ui/borrow_as_ptr_raw_ref.stderr b/tests/ui/borrow_as_ptr_raw_ref.stderr new file mode 100644 index 0000000000000..5611fcae8d4b1 --- /dev/null +++ b/tests/ui/borrow_as_ptr_raw_ref.stderr @@ -0,0 +1,17 @@ +error: borrow as raw pointer + --> tests/ui/borrow_as_ptr_raw_ref.rs:11:14 + | +LL | let _p = &val as *const i32; + | ^^^^^^^^^^^^^^^^^^ help: try: `&raw const val` + | + = note: `-D clippy::borrow-as-ptr` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::borrow_as_ptr)]` + +error: borrow as raw pointer + --> tests/ui/borrow_as_ptr_raw_ref.rs:18:18 + | +LL | let _p_mut = &mut val_mut as *mut i32; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&raw mut val_mut` + +error: aborting due to 2 previous errors +