Skip to content

Commit

Permalink
Special-case suggestions for null pointers constness cast
Browse files Browse the repository at this point in the history
  • Loading branch information
samueltardieu committed Sep 7, 2024
1 parent d9c4523 commit bb2b65a
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 19 deletions.
6 changes: 5 additions & 1 deletion clippy_lints/src/casts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,19 +410,23 @@ declare_clippy_lint! {
/// ### Why is this bad?
/// 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
/// type.
/// type. Or, when null pointers are involved, `null()` and `null_mut()` can be used directly.
///
/// ### Example
/// ```no_run
/// let ptr: *const u32 = &42_u32;
/// let mut_ptr = ptr as *mut u32;
/// let ptr = mut_ptr as *const u32;
/// let ptr = std::ptr::null::<u32>() as *mut u32;
/// let ptr = std::ptr::null_mut::<u32>() as *const u32;
/// ```
/// Use instead:
/// ```no_run
/// let ptr: *const u32 = &42_u32;
/// let mut_ptr = ptr.cast_mut();
/// let ptr = mut_ptr.cast_const();
/// let ptr = std::ptr::null_mut::<u32>();
/// let ptr = std::ptr::null::<u32>();
/// ```
#[clippy::version = "1.72.0"]
pub PTR_CAST_CONSTNESS,
Expand Down
68 changes: 51 additions & 17 deletions clippy_lints/src/casts/ptr_cast_constness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::sugg::Sugg;
use rustc_errors::Applicability;
use rustc_hir::{Expr, Mutability};
use rustc_hir::{Expr, ExprKind, Mutability, QPath};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty, TypeVisitableExt};
use rustc_span::sym;

use super::PTR_CAST_CONSTNESS;

Expand All @@ -16,8 +17,7 @@ pub(super) fn check<'tcx>(
cast_to: Ty<'tcx>,
msrv: &Msrv,
) {
if msrv.meets(msrvs::POINTER_CAST_CONSTNESS)
&& let ty::RawPtr(from_ty, from_mutbl) = cast_from.kind()
if let ty::RawPtr(from_ty, from_mutbl) = cast_from.kind()
&& let ty::RawPtr(to_ty, to_mutbl) = cast_to.kind()
&& matches!(
(from_mutbl, to_mutbl),
Expand All @@ -26,20 +26,54 @@ pub(super) fn check<'tcx>(
&& from_ty == to_ty
&& !from_ty.has_erased_regions()
{
let sugg = Sugg::hir(cx, cast_expr, "_");
let constness = match *to_mutbl {
Mutability::Not => "const",
Mutability::Mut => "mut",
};
if let ExprKind::Call(func, []) = cast_expr.kind
&& let ExprKind::Path(QPath::Resolved(None, path)) = func.kind
&& let Some(method_defid) = path.res.opt_def_id()
{
let mut app = Applicability::MachineApplicable;
let sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut app).to_string();
if cx.tcx.is_diagnostic_item(sym::ptr_null, method_defid) {
span_lint_and_sugg(
cx,
PTR_CAST_CONSTNESS,
expr.span,
"`as` casting to make a const null pointer into a mutable null pointer",
"use `null_mut()` directly instead",
sugg.replace("null", "null_mut"),
app,
);
return;
}
if cx.tcx.is_diagnostic_item(sym::ptr_null_mut, method_defid) {
span_lint_and_sugg(
cx,
PTR_CAST_CONSTNESS,
expr.span,
"`as` casting to make a mutable null pointer into an immutable null pointer",
"use `null()` directly instead",
sugg.replace("null_mut", "null"),
app,
);
return;
}
}

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,
);
if msrv.meets(msrvs::POINTER_CAST_CONSTNESS) {
let sugg = Sugg::hir(cx, cast_expr, "_");
let constness = match *to_mutbl {
Mutability::Not => "const",
Mutability::Mut => "mut",
};

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,
);
}
}
}
13 changes: 13 additions & 0 deletions tests/ui/ptr_cast_constness.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,16 @@ fn _msrv_1_65() {
let _ = ptr.cast_mut();
let _ = mut_ptr.cast_const();
}

#[inline_macros]
fn null_pointers() {
use std::ptr;
let _ = ptr::null_mut::<String>();
let _ = ptr::null::<u32>();

// Make sure the lint is triggered inside a macro
let _ = inline!(ptr::null_mut::<u32>());

// Do not lint inside macros from external crates
let _ = external!(ptr::null::<u32>() as *mut u32);
}
13 changes: 13 additions & 0 deletions tests/ui/ptr_cast_constness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,16 @@ fn _msrv_1_65() {
let _ = ptr as *mut 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);
}
22 changes: 21 additions & 1 deletion tests/ui/ptr_cast_constness.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,25 @@ error: `as` casting between raw pointers while changing only its constness
LL | let _ = mut_ptr as *const u32;
| ^^^^^^^^^^^^^^^^^^^^^ 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: `ptr::null_mut::<String>()`

error: `as` casting to make a mutable null pointer into an immutable null pointer
--> tests/ui/ptr_cast_constness.rs:76:13
|
LL | let _ = ptr::null_mut::<u32>() as *const u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `null()` directly instead: `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: `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

0 comments on commit bb2b65a

Please sign in to comment.