Refactor ptr_offset_with_cast (#15613)
Putting this lint in `/methods` is simpler and probably slightly more efficient. changelog: [`ptr_offset_with_cast`]: respect MSRV when suggesting fix, and lint even more cases
This commit is contained in:
commit
7a12684741
9 changed files with 209 additions and 164 deletions
|
|
@ -448,6 +448,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
|
|||
crate::methods::OR_THEN_UNWRAP_INFO,
|
||||
crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
|
||||
crate::methods::PATH_ENDS_WITH_EXT_INFO,
|
||||
crate::methods::PTR_OFFSET_WITH_CAST_INFO,
|
||||
crate::methods::RANGE_ZIP_WITH_LEN_INFO,
|
||||
crate::methods::READONLY_WRITE_LOCK_INFO,
|
||||
crate::methods::READ_LINE_WITHOUT_TRIM_INFO,
|
||||
|
|
@ -624,7 +625,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
|
|||
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,
|
||||
crate::question_mark::QUESTION_MARK_INFO,
|
||||
|
|
|
|||
|
|
@ -302,7 +302,6 @@ mod permissions_set_readonly_false;
|
|||
mod pointers_in_nomem_asm_block;
|
||||
mod precedence;
|
||||
mod ptr;
|
||||
mod ptr_offset_with_cast;
|
||||
mod pub_underscore_fields;
|
||||
mod pub_use;
|
||||
mod question_mark;
|
||||
|
|
@ -593,7 +592,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
|
|||
store.register_late_pass(|_| Box::new(unwrap::Unwrap));
|
||||
store.register_late_pass(move |_| Box::new(indexing_slicing::IndexingSlicing::new(conf)));
|
||||
store.register_late_pass(move |tcx| Box::new(non_copy_const::NonCopyConst::new(tcx, conf)));
|
||||
store.register_late_pass(|_| Box::new(ptr_offset_with_cast::PtrOffsetWithCast));
|
||||
store.register_late_pass(|_| Box::new(redundant_clone::RedundantClone));
|
||||
store.register_late_pass(|_| Box::new(slow_vector_initialization::SlowVectorInit));
|
||||
store.register_late_pass(move |_| Box::new(unnecessary_wraps::UnnecessaryWraps::new(conf)));
|
||||
|
|
|
|||
|
|
@ -91,6 +91,7 @@ mod or_fun_call;
|
|||
mod or_then_unwrap;
|
||||
mod path_buf_push_overwrite;
|
||||
mod path_ends_with_ext;
|
||||
mod ptr_offset_with_cast;
|
||||
mod range_zip_with_len;
|
||||
mod read_line_without_trim;
|
||||
mod readonly_write_lock;
|
||||
|
|
@ -1725,6 +1726,43 @@ declare_clippy_lint! {
|
|||
"Check for offset calculations on raw pointers to zero-sized types"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for usage of the `offset` pointer method with a `usize` casted to an
|
||||
/// `isize`.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// If we’re always increasing the pointer address, we can avoid the numeric
|
||||
/// cast by using the `add` method instead.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```no_run
|
||||
/// let vec = vec![b'a', b'b', b'c'];
|
||||
/// let ptr = vec.as_ptr();
|
||||
/// let offset = 1_usize;
|
||||
///
|
||||
/// unsafe {
|
||||
/// ptr.offset(offset as isize);
|
||||
/// }
|
||||
/// ```
|
||||
///
|
||||
/// Could be written:
|
||||
///
|
||||
/// ```no_run
|
||||
/// let vec = vec![b'a', b'b', b'c'];
|
||||
/// let ptr = vec.as_ptr();
|
||||
/// let offset = 1_usize;
|
||||
///
|
||||
/// unsafe {
|
||||
/// ptr.add(offset);
|
||||
/// }
|
||||
/// ```
|
||||
#[clippy::version = "1.30.0"]
|
||||
pub PTR_OFFSET_WITH_CAST,
|
||||
complexity,
|
||||
"unneeded pointer offset cast"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for `FileType::is_file()`.
|
||||
|
|
@ -4665,6 +4703,7 @@ impl_lint_pass!(Methods => [
|
|||
UNINIT_ASSUMED_INIT,
|
||||
MANUAL_SATURATING_ARITHMETIC,
|
||||
ZST_OFFSET,
|
||||
PTR_OFFSET_WITH_CAST,
|
||||
FILETYPE_IS_FILE,
|
||||
OPTION_AS_REF_DEREF,
|
||||
UNNECESSARY_LAZY_EVALUATIONS,
|
||||
|
|
@ -4960,10 +4999,7 @@ impl Methods {
|
|||
// Handle method calls whose receiver and arguments may not come from expansion
|
||||
if let Some((name, recv, args, span, call_span)) = method_call(expr) {
|
||||
match (name, args) {
|
||||
(
|
||||
sym::add | sym::offset | sym::sub | sym::wrapping_offset | sym::wrapping_add | sym::wrapping_sub,
|
||||
[_arg],
|
||||
) => {
|
||||
(sym::add | sym::sub | sym::wrapping_add | sym::wrapping_sub, [_arg]) => {
|
||||
zst_offset::check(cx, expr, recv);
|
||||
},
|
||||
(sym::all, [arg]) => {
|
||||
|
|
@ -5334,6 +5370,11 @@ impl Methods {
|
|||
},
|
||||
_ => iter_nth_zero::check(cx, expr, recv, n_arg),
|
||||
},
|
||||
(sym::offset | sym::wrapping_offset, [arg]) => {
|
||||
zst_offset::check(cx, expr, recv);
|
||||
|
||||
ptr_offset_with_cast::check(cx, name, expr, recv, arg, self.msrv);
|
||||
},
|
||||
(sym::ok_or_else, [arg]) => {
|
||||
unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or");
|
||||
},
|
||||
|
|
|
|||
82
clippy_lints/src/methods/ptr_offset_with_cast.rs
Normal file
82
clippy_lints/src/methods/ptr_offset_with_cast.rs
Normal file
|
|
@ -0,0 +1,82 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_then;
|
||||
use clippy_utils::msrvs::{self, Msrv};
|
||||
use clippy_utils::sym;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Expr, ExprKind};
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_span::Symbol;
|
||||
use std::fmt;
|
||||
|
||||
use super::PTR_OFFSET_WITH_CAST;
|
||||
|
||||
pub(super) fn check(
|
||||
cx: &LateContext<'_>,
|
||||
method: Symbol,
|
||||
expr: &Expr<'_>,
|
||||
recv: &Expr<'_>,
|
||||
arg: &Expr<'_>,
|
||||
msrv: Msrv,
|
||||
) {
|
||||
// `pointer::add` and `pointer::wrapping_add` are only stable since 1.26.0. These functions
|
||||
// became const-stable in 1.61.0, the same version that `pointer::offset` became const-stable.
|
||||
if !msrv.meets(cx, msrvs::POINTER_ADD_SUB_METHODS) {
|
||||
return;
|
||||
}
|
||||
|
||||
let method = match method {
|
||||
sym::offset => Method::Offset,
|
||||
sym::wrapping_offset => Method::WrappingOffset,
|
||||
_ => return,
|
||||
};
|
||||
|
||||
if !cx.typeck_results().expr_ty_adjusted(recv).is_raw_ptr() {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if the argument to the method call is a cast from usize.
|
||||
let cast_lhs_expr = match arg.kind {
|
||||
ExprKind::Cast(lhs, _) if cx.typeck_results().expr_ty(lhs).is_usize() => lhs,
|
||||
_ => return,
|
||||
};
|
||||
|
||||
let ExprKind::MethodCall(method_name, _, _, _) = expr.kind else {
|
||||
return;
|
||||
};
|
||||
|
||||
let msg = format!("use of `{method}` with a `usize` casted to an `isize`");
|
||||
span_lint_and_then(cx, PTR_OFFSET_WITH_CAST, expr.span, msg, |diag| {
|
||||
diag.multipart_suggestion(
|
||||
format!("use `{}` instead", method.suggestion()),
|
||||
vec![
|
||||
(method_name.ident.span, method.suggestion().to_string()),
|
||||
(arg.span.with_lo(cast_lhs_expr.span.hi()), String::new()),
|
||||
],
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone)]
|
||||
enum Method {
|
||||
Offset,
|
||||
WrappingOffset,
|
||||
}
|
||||
|
||||
impl Method {
|
||||
#[must_use]
|
||||
fn suggestion(self) -> &'static str {
|
||||
match self {
|
||||
Self::Offset => "add",
|
||||
Self::WrappingOffset => "wrapping_add",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for Method {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
Self::Offset => write!(f, "offset"),
|
||||
Self::WrappingOffset => write!(f, "wrapping_offset"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,151 +0,0 @@
|
|||
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
|
||||
use clippy_utils::source::SpanRangeExt;
|
||||
use clippy_utils::sym;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Expr, ExprKind};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::declare_lint_pass;
|
||||
use std::fmt;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for usage of the `offset` pointer method with a `usize` casted to an
|
||||
/// `isize`.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// If we’re always increasing the pointer address, we can avoid the numeric
|
||||
/// cast by using the `add` method instead.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```no_run
|
||||
/// let vec = vec![b'a', b'b', b'c'];
|
||||
/// let ptr = vec.as_ptr();
|
||||
/// let offset = 1_usize;
|
||||
///
|
||||
/// unsafe {
|
||||
/// ptr.offset(offset as isize);
|
||||
/// }
|
||||
/// ```
|
||||
///
|
||||
/// Could be written:
|
||||
///
|
||||
/// ```no_run
|
||||
/// let vec = vec![b'a', b'b', b'c'];
|
||||
/// let ptr = vec.as_ptr();
|
||||
/// let offset = 1_usize;
|
||||
///
|
||||
/// unsafe {
|
||||
/// ptr.add(offset);
|
||||
/// }
|
||||
/// ```
|
||||
#[clippy::version = "1.30.0"]
|
||||
pub PTR_OFFSET_WITH_CAST,
|
||||
complexity,
|
||||
"unneeded pointer offset cast"
|
||||
}
|
||||
|
||||
declare_lint_pass!(PtrOffsetWithCast => [PTR_OFFSET_WITH_CAST]);
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for PtrOffsetWithCast {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
||||
// Check if the expressions is a ptr.offset or ptr.wrapping_offset method call
|
||||
let Some((receiver_expr, arg_expr, method)) = expr_as_ptr_offset_call(cx, expr) else {
|
||||
return;
|
||||
};
|
||||
|
||||
// Check if the argument to the method call is a cast from usize
|
||||
let Some(cast_lhs_expr) = expr_as_cast_from_usize(cx, arg_expr) else {
|
||||
return;
|
||||
};
|
||||
|
||||
let msg = format!("use of `{method}` with a `usize` casted to an `isize`");
|
||||
if let Some(sugg) = build_suggestion(cx, method, receiver_expr, cast_lhs_expr) {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
PTR_OFFSET_WITH_CAST,
|
||||
expr.span,
|
||||
msg,
|
||||
"try",
|
||||
sugg,
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
} else {
|
||||
span_lint(cx, PTR_OFFSET_WITH_CAST, expr.span, msg);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// If the given expression is a cast from a usize, return the lhs of the cast
|
||||
fn expr_as_cast_from_usize<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
|
||||
if let ExprKind::Cast(cast_lhs_expr, _) = expr.kind
|
||||
&& is_expr_ty_usize(cx, cast_lhs_expr)
|
||||
{
|
||||
return Some(cast_lhs_expr);
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
// If the given expression is a ptr::offset or ptr::wrapping_offset method call, return the
|
||||
// receiver, the arg of the method call, and the method.
|
||||
fn expr_as_ptr_offset_call<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
expr: &'tcx Expr<'_>,
|
||||
) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>, Method)> {
|
||||
if let ExprKind::MethodCall(path_segment, arg_0, [arg_1], _) = &expr.kind
|
||||
&& is_expr_ty_raw_ptr(cx, arg_0)
|
||||
{
|
||||
if path_segment.ident.name == sym::offset {
|
||||
return Some((arg_0, arg_1, Method::Offset));
|
||||
}
|
||||
if path_segment.ident.name == sym::wrapping_offset {
|
||||
return Some((arg_0, arg_1, Method::WrappingOffset));
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
// Is the type of the expression a usize?
|
||||
fn is_expr_ty_usize(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
|
||||
cx.typeck_results().expr_ty(expr) == cx.tcx.types.usize
|
||||
}
|
||||
|
||||
// Is the type of the expression a raw pointer?
|
||||
fn is_expr_ty_raw_ptr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
|
||||
cx.typeck_results().expr_ty(expr).is_raw_ptr()
|
||||
}
|
||||
|
||||
fn build_suggestion(
|
||||
cx: &LateContext<'_>,
|
||||
method: Method,
|
||||
receiver_expr: &Expr<'_>,
|
||||
cast_lhs_expr: &Expr<'_>,
|
||||
) -> Option<String> {
|
||||
let receiver = receiver_expr.span.get_source_text(cx)?;
|
||||
let cast_lhs = cast_lhs_expr.span.get_source_text(cx)?;
|
||||
Some(format!("{receiver}.{}({cast_lhs})", method.suggestion()))
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone)]
|
||||
enum Method {
|
||||
Offset,
|
||||
WrappingOffset,
|
||||
}
|
||||
|
||||
impl Method {
|
||||
#[must_use]
|
||||
fn suggestion(self) -> &'static str {
|
||||
match self {
|
||||
Self::Offset => "add",
|
||||
Self::WrappingOffset => "wrapping_add",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for Method {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
Self::Offset => write!(f, "offset"),
|
||||
Self::WrappingOffset => write!(f, "wrapping_offset"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -73,7 +73,7 @@ msrv_aliases! {
|
|||
1,29,0 { ITER_FLATTEN }
|
||||
1,28,0 { FROM_BOOL, REPEAT_WITH, SLICE_FROM_REF }
|
||||
1,27,0 { ITERATOR_TRY_FOLD }
|
||||
1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN }
|
||||
1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN, POINTER_ADD_SUB_METHODS }
|
||||
1,24,0 { IS_ASCII_DIGIT, PTR_NULL }
|
||||
1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN }
|
||||
1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR }
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
#![allow(clippy::unnecessary_cast, clippy::useless_vec)]
|
||||
#![expect(clippy::unnecessary_cast, clippy::useless_vec, clippy::needless_borrow)]
|
||||
|
||||
fn main() {
|
||||
let vec = vec![b'a', b'b', b'c'];
|
||||
|
|
@ -18,5 +18,25 @@ fn main() {
|
|||
//~^ ptr_offset_with_cast
|
||||
let _ = ptr.wrapping_offset(offset_isize as isize);
|
||||
let _ = ptr.wrapping_offset(offset_u8 as isize);
|
||||
|
||||
let _ = S.offset(offset_usize as isize);
|
||||
let _ = S.wrapping_offset(offset_usize as isize);
|
||||
|
||||
let _ = (&ptr).add(offset_usize);
|
||||
//~^ ptr_offset_with_cast
|
||||
let _ = (&ptr).wrapping_add(offset_usize);
|
||||
//~^ ptr_offset_with_cast
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct S;
|
||||
|
||||
impl S {
|
||||
fn offset(self, _: isize) -> Self {
|
||||
self
|
||||
}
|
||||
fn wrapping_offset(self, _: isize) -> Self {
|
||||
self
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
#![allow(clippy::unnecessary_cast, clippy::useless_vec)]
|
||||
#![expect(clippy::unnecessary_cast, clippy::useless_vec, clippy::needless_borrow)]
|
||||
|
||||
fn main() {
|
||||
let vec = vec![b'a', b'b', b'c'];
|
||||
|
|
@ -18,5 +18,25 @@ fn main() {
|
|||
//~^ ptr_offset_with_cast
|
||||
let _ = ptr.wrapping_offset(offset_isize as isize);
|
||||
let _ = ptr.wrapping_offset(offset_u8 as isize);
|
||||
|
||||
let _ = S.offset(offset_usize as isize);
|
||||
let _ = S.wrapping_offset(offset_usize as isize);
|
||||
|
||||
let _ = (&ptr).offset(offset_usize as isize);
|
||||
//~^ ptr_offset_with_cast
|
||||
let _ = (&ptr).wrapping_offset(offset_usize as isize);
|
||||
//~^ ptr_offset_with_cast
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct S;
|
||||
|
||||
impl S {
|
||||
fn offset(self, _: isize) -> Self {
|
||||
self
|
||||
}
|
||||
fn wrapping_offset(self, _: isize) -> Self {
|
||||
self
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,16 +2,51 @@ error: use of `offset` with a `usize` casted to an `isize`
|
|||
--> tests/ui/ptr_offset_with_cast.rs:12:17
|
||||
|
|
||||
LL | let _ = ptr.offset(offset_usize as isize);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.add(offset_usize)`
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: `-D clippy::ptr-offset-with-cast` implied by `-D warnings`
|
||||
= help: to override `-D warnings` add `#[allow(clippy::ptr_offset_with_cast)]`
|
||||
help: use `add` instead
|
||||
|
|
||||
LL - let _ = ptr.offset(offset_usize as isize);
|
||||
LL + let _ = ptr.add(offset_usize);
|
||||
|
|
||||
|
||||
error: use of `wrapping_offset` with a `usize` casted to an `isize`
|
||||
--> tests/ui/ptr_offset_with_cast.rs:17:17
|
||||
|
|
||||
LL | let _ = ptr.wrapping_offset(offset_usize as isize);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.wrapping_add(offset_usize)`
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: use `wrapping_add` instead
|
||||
|
|
||||
LL - let _ = ptr.wrapping_offset(offset_usize as isize);
|
||||
LL + let _ = ptr.wrapping_add(offset_usize);
|
||||
|
|
||||
|
||||
error: aborting due to 2 previous errors
|
||||
error: use of `offset` with a `usize` casted to an `isize`
|
||||
--> tests/ui/ptr_offset_with_cast.rs:25:17
|
||||
|
|
||||
LL | let _ = (&ptr).offset(offset_usize as isize);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: use `add` instead
|
||||
|
|
||||
LL - let _ = (&ptr).offset(offset_usize as isize);
|
||||
LL + let _ = (&ptr).add(offset_usize);
|
||||
|
|
||||
|
||||
error: use of `wrapping_offset` with a `usize` casted to an `isize`
|
||||
--> tests/ui/ptr_offset_with_cast.rs:27:17
|
||||
|
|
||||
LL | let _ = (&ptr).wrapping_offset(offset_usize as isize);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: use `wrapping_add` instead
|
||||
|
|
||||
LL - let _ = (&ptr).wrapping_offset(offset_usize as isize);
|
||||
LL + let _ = (&ptr).wrapping_add(offset_usize);
|
||||
|
|
||||
|
||||
error: aborting due to 4 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue