Skip to content

Commit

Permalink
update borrow_as_ptr to suggest &raw syntax (#13689)
Browse files Browse the repository at this point in the history
This PR updates the `borrow_as_ptr` lint to no longer suggest `addr_of!`
and `addr_of_mut!` and instead use the preferred `&raw const` and `&raw
mut` syntax.

Not sure about two things:
1. Do I need to set or update a MSRV for the lint anywhere?
2. There is a `borrow_as_ptr_no_std` test as well as a `borrow_as_ptr`
test. They used to be more relevant as the lint needed to select `std`
or `core`, but that is gone now, so maybe the `borrow_as_ptr_no_std`
should be deleted?

changelog: update `borrow_as_ptr` to suggest `&raw` syntax
  • Loading branch information
Jarcho authored Dec 5, 2024
2 parents 2550530 + 9925f99 commit a5e46a6
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 19 deletions.
35 changes: 26 additions & 9 deletions clippy_lints/src/casts/borrow_as_ptr.rs
Original file line number Diff line number Diff line change
@@ -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::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;
Expand All @@ -13,15 +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
Expand All @@ -31,17 +29,36 @@ 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;
}

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;
}
false
}
19 changes: 11 additions & 8 deletions clippy_lints/src/casts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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, &self.msrv)
} 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);
}
}

Expand Down
4 changes: 2 additions & 2 deletions clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/borrow_and_ref_as_ptr.fixed
Original file line number Diff line number Diff line change
@@ -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>(_: T) {}

fn main() {
let mut val = 0;
f(&raw const val);
f(&raw mut val);
}
11 changes: 11 additions & 0 deletions tests/ui/borrow_and_ref_as_ptr.rs
Original file line number Diff line number Diff line change
@@ -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>(_: T) {}

fn main() {
let mut val = 0;
f(&val as *const _);
f(&mut val as *mut i32);
}
17 changes: 17 additions & 0 deletions tests/ui/borrow_and_ref_as_ptr.stderr
Original file line number Diff line number Diff line change
@@ -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

19 changes: 19 additions & 0 deletions tests/ui/borrow_as_ptr_raw_ref.fixed
Original file line number Diff line number Diff line change
@@ -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;
}
19 changes: 19 additions & 0 deletions tests/ui/borrow_as_ptr_raw_ref.rs
Original file line number Diff line number Diff line change
@@ -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;
}
17 changes: 17 additions & 0 deletions tests/ui/borrow_as_ptr_raw_ref.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit a5e46a6

Please sign in to comment.