Special-case suggestions for null pointers constness cast

This commit is contained in:
Samuel Tardieu 2024-09-07 16:18:45 +02:00
parent d9c4523e6c
commit 9e9526c6ab
5 changed files with 97 additions and 19 deletions

View file

@ -410,19 +410,23 @@ declare_clippy_lint! {
/// ### Why is this bad? /// ### Why is this bad?
/// Though `as` casts between raw pointers are not terrible, `pointer::cast_mut` and /// Though `as` casts between raw pointers are not terrible, `pointer::cast_mut` and
/// `pointer::cast_const` are safer because they cannot accidentally cast the pointer to another /// `pointer::cast_const` are safer because they cannot accidentally cast the pointer to another
/// type. /// type. Or, when null pointers are involved, `null()` and `null_mut()` can be used directly.
/// ///
/// ### Example /// ### Example
/// ```no_run /// ```no_run
/// let ptr: *const u32 = &42_u32; /// let ptr: *const u32 = &42_u32;
/// let mut_ptr = ptr as *mut u32; /// let mut_ptr = ptr as *mut u32;
/// let ptr = mut_ptr as *const u32; /// let ptr = mut_ptr as *const u32;
/// let ptr1 = std::ptr::null::<u32>() as *mut u32;
/// let ptr2 = std::ptr::null_mut::<u32>() as *const u32;
/// ``` /// ```
/// Use instead: /// Use instead:
/// ```no_run /// ```no_run
/// let ptr: *const u32 = &42_u32; /// let ptr: *const u32 = &42_u32;
/// let mut_ptr = ptr.cast_mut(); /// let mut_ptr = ptr.cast_mut();
/// let ptr = mut_ptr.cast_const(); /// let ptr = mut_ptr.cast_const();
/// let ptr1 = std::ptr::null_mut::<u32>();
/// let ptr2 = std::ptr::null::<u32>();
/// ``` /// ```
#[clippy::version = "1.72.0"] #[clippy::version = "1.72.0"]
pub PTR_CAST_CONSTNESS, pub PTR_CAST_CONSTNESS,

View file

@ -1,10 +1,12 @@
use clippy_config::msrvs::{self, Msrv}; use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::std_or_core;
use clippy_utils::sugg::Sugg; use clippy_utils::sugg::Sugg;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::{Expr, Mutability}; use rustc_hir::{Expr, ExprKind, Mutability, QPath};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty, TypeVisitableExt}; use rustc_middle::ty::{self, Ty, TypeVisitableExt};
use rustc_span::sym;
use super::PTR_CAST_CONSTNESS; use super::PTR_CAST_CONSTNESS;
@ -16,8 +18,7 @@ pub(super) fn check<'tcx>(
cast_to: Ty<'tcx>, cast_to: Ty<'tcx>,
msrv: &Msrv, msrv: &Msrv,
) { ) {
if msrv.meets(msrvs::POINTER_CAST_CONSTNESS) if let ty::RawPtr(from_ty, from_mutbl) = cast_from.kind()
&& let ty::RawPtr(from_ty, from_mutbl) = cast_from.kind()
&& let ty::RawPtr(to_ty, to_mutbl) = cast_to.kind() && let ty::RawPtr(to_ty, to_mutbl) = cast_to.kind()
&& matches!( && matches!(
(from_mutbl, to_mutbl), (from_mutbl, to_mutbl),
@ -26,20 +27,47 @@ pub(super) fn check<'tcx>(
&& from_ty == to_ty && from_ty == to_ty
&& !from_ty.has_erased_regions() && !from_ty.has_erased_regions()
{ {
let sugg = Sugg::hir(cx, cast_expr, "_"); if let ExprKind::Call(func, []) = cast_expr.kind
let constness = match *to_mutbl { && let ExprKind::Path(QPath::Resolved(None, path)) = func.kind
Mutability::Not => "const", && let Some(defid) = path.res.opt_def_id()
Mutability::Mut => "mut", && let Some(prefix) = std_or_core(cx)
}; && let mut app = Applicability::MachineApplicable
&& let sugg = format!("{}", Sugg::hir_with_applicability(cx, cast_expr, "_", &mut app))
&& let Some((_, after_lt)) = sugg.split_once("::<")
&& let Some((source, target, target_func)) = match cx.tcx.get_diagnostic_name(defid) {
Some(sym::ptr_null) => Some(("const", "mutable", "null_mut")),
Some(sym::ptr_null_mut) => Some(("mutable", "const", "null")),
_ => None,
}
{
span_lint_and_sugg(
cx,
PTR_CAST_CONSTNESS,
expr.span,
format!("`as` casting to make a {source} null pointer into a {target} null pointer"),
format!("use `{target_func}()` directly instead"),
format!("{prefix}::ptr::{target_func}::<{after_lt}"),
app,
);
return;
}
span_lint_and_sugg( if msrv.meets(msrvs::POINTER_CAST_CONSTNESS) {
cx, let sugg = Sugg::hir(cx, cast_expr, "_");
PTR_CAST_CONSTNESS, let constness = match *to_mutbl {
expr.span, Mutability::Not => "const",
"`as` casting between raw pointers while changing only its constness", Mutability::Mut => "mut",
format!("try `pointer::cast_{constness}`, a safer alternative"), };
format!("{}.cast_{constness}()", sugg.maybe_par()),
Applicability::MachineApplicable, span_lint_and_sugg(
); cx,
PTR_CAST_CONSTNESS,
expr.span,
"`as` casting between raw pointers while changing only its constness",
format!("try `pointer::cast_{constness}`, a safer alternative"),
format!("{}.cast_{constness}()", sugg.maybe_par()),
Applicability::MachineApplicable,
);
}
} }
} }

View file

@ -68,3 +68,16 @@ fn _msrv_1_65() {
let _ = ptr.cast_mut(); let _ = ptr.cast_mut();
let _ = mut_ptr.cast_const(); let _ = mut_ptr.cast_const();
} }
#[inline_macros]
fn null_pointers() {
use std::ptr;
let _ = std::ptr::null_mut::<String>();
let _ = std::ptr::null::<u32>();
// Make sure the lint is triggered inside a macro
let _ = inline!(std::ptr::null_mut::<u32>());
// Do not lint inside macros from external crates
let _ = external!(ptr::null::<u32>() as *mut u32);
}

View file

@ -68,3 +68,16 @@ fn _msrv_1_65() {
let _ = ptr as *mut u32; let _ = ptr as *mut u32;
let _ = mut_ptr as *const u32; let _ = mut_ptr as *const u32;
} }
#[inline_macros]
fn null_pointers() {
use std::ptr;
let _ = ptr::null::<String>() as *mut String;
let _ = ptr::null_mut::<u32>() as *const u32;
// Make sure the lint is triggered inside a macro
let _ = inline!(ptr::null::<u32>() as *mut u32);
// Do not lint inside macros from external crates
let _ = external!(ptr::null::<u32>() as *mut u32);
}

View file

@ -43,5 +43,25 @@ error: `as` casting between raw pointers while changing only its constness
LL | let _ = mut_ptr as *const u32; LL | let _ = mut_ptr as *const u32;
| ^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast_const`, a safer alternative: `mut_ptr.cast_const()` | ^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast_const`, a safer alternative: `mut_ptr.cast_const()`
error: aborting due to 7 previous errors error: `as` casting to make a const null pointer into a mutable null pointer
--> tests/ui/ptr_cast_constness.rs:75:13
|
LL | let _ = ptr::null::<String>() as *mut String;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `null_mut()` directly instead: `std::ptr::null_mut::<String>()`
error: `as` casting to make a mutable null pointer into a const null pointer
--> tests/ui/ptr_cast_constness.rs:76:13
|
LL | let _ = ptr::null_mut::<u32>() as *const u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `null()` directly instead: `std::ptr::null::<u32>()`
error: `as` casting to make a const null pointer into a mutable null pointer
--> tests/ui/ptr_cast_constness.rs:79:21
|
LL | let _ = inline!(ptr::null::<u32>() as *mut u32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `null_mut()` directly instead: `std::ptr::null_mut::<u32>()`
|
= note: this error originates in the macro `__inline_mac_fn_null_pointers` (in Nightly builds, run with -Z macro-backtrace for more info)
error: aborting due to 10 previous errors