Skip to content

Commit b0e37f3

Browse files
committed
add ptr_offset_by_literal lint
1 parent 7a12684 commit b0e37f3

11 files changed

+425
-4
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6376,6 +6376,7 @@ Released 2018-09-13
63766376
[`ptr_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr
63776377
[`ptr_cast_constness`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_cast_constness
63786378
[`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
6379+
[`ptr_offset_by_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_by_literal
63796380
[`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
63806381
[`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names
63816382
[`pub_underscore_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
448448
crate::methods::OR_THEN_UNWRAP_INFO,
449449
crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
450450
crate::methods::PATH_ENDS_WITH_EXT_INFO,
451+
crate::methods::PTR_OFFSET_BY_LITERAL_INFO,
451452
crate::methods::PTR_OFFSET_WITH_CAST_INFO,
452453
crate::methods::RANGE_ZIP_WITH_LEN_INFO,
453454
crate::methods::READONLY_WRITE_LOCK_INFO,

clippy_lints/src/methods/mod.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ mod or_fun_call;
9191
mod or_then_unwrap;
9292
mod path_buf_push_overwrite;
9393
mod path_ends_with_ext;
94+
mod ptr_offset_by_literal;
9495
mod ptr_offset_with_cast;
9596
mod range_zip_with_len;
9697
mod read_line_without_trim;
@@ -1726,6 +1727,40 @@ declare_clippy_lint! {
17261727
"Check for offset calculations on raw pointers to zero-sized types"
17271728
}
17281729

1730+
declare_clippy_lint! {
1731+
/// ### What it does
1732+
/// Checks for usage of the `offset` pointer method with an integer
1733+
/// literal.
1734+
///
1735+
/// ### Why is this bad?
1736+
/// The `add` and `sub` methods more accurately express the intent.
1737+
///
1738+
/// ### Example
1739+
/// ```no_run
1740+
/// let vec = vec![b'a', b'b', b'c'];
1741+
/// let ptr = vec.as_ptr();
1742+
///
1743+
/// unsafe {
1744+
/// ptr.offset(-8);
1745+
/// }
1746+
/// ```
1747+
///
1748+
/// Could be written:
1749+
///
1750+
/// ```no_run
1751+
/// let vec = vec![b'a', b'b', b'c'];
1752+
/// let ptr = vec.as_ptr();
1753+
///
1754+
/// unsafe {
1755+
/// ptr.sub(8);
1756+
/// }
1757+
/// ```
1758+
#[clippy::version = "1.92.0"]
1759+
pub PTR_OFFSET_BY_LITERAL,
1760+
complexity,
1761+
"unneeded pointer offset"
1762+
}
1763+
17291764
declare_clippy_lint! {
17301765
/// ### What it does
17311766
/// Checks for usage of the `offset` pointer method with a `usize` casted to an
@@ -4703,6 +4738,7 @@ impl_lint_pass!(Methods => [
47034738
UNINIT_ASSUMED_INIT,
47044739
MANUAL_SATURATING_ARITHMETIC,
47054740
ZST_OFFSET,
4741+
PTR_OFFSET_BY_LITERAL,
47064742
PTR_OFFSET_WITH_CAST,
47074743
FILETYPE_IS_FILE,
47084744
OPTION_AS_REF_DEREF,
@@ -5374,6 +5410,7 @@ impl Methods {
53745410
zst_offset::check(cx, expr, recv);
53755411

53765412
ptr_offset_with_cast::check(cx, name, expr, recv, arg, self.msrv);
5413+
ptr_offset_by_literal::check(cx, expr, self.msrv);
53775414
},
53785415
(sym::ok_or_else, [arg]) => {
53795416
unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or");
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::msrvs::{self, Msrv};
3+
use clippy_utils::source::SpanRangeExt;
4+
use clippy_utils::sym;
5+
use rustc_ast::LitKind;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{Expr, ExprKind, UnOp};
8+
use rustc_lint::LateContext;
9+
use std::cmp::Ordering;
10+
use std::fmt;
11+
12+
use super::PTR_OFFSET_BY_LITERAL;
13+
14+
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Msrv) {
15+
// `pointer::add` and `pointer::wrapping_add` are only stable since 1.26.0. These functions
16+
// became const-stable in 1.61.0, the same version that `pointer::offset` became const-stable.
17+
if !msrv.meets(cx, msrvs::POINTER_ADD_SUB_METHODS) {
18+
return;
19+
}
20+
21+
let ExprKind::MethodCall(method_name, recv, [arg_expr], _) = expr.kind else {
22+
return;
23+
};
24+
25+
let method = match method_name.ident.name {
26+
sym::offset => Method::Offset,
27+
sym::wrapping_offset => Method::WrappingOffset,
28+
_ => return,
29+
};
30+
31+
if !cx.typeck_results().expr_ty_adjusted(recv).is_raw_ptr() {
32+
return;
33+
}
34+
35+
// Check if the argument to the method call is a (negated) literal.
36+
let Some((literal, literal_text)) = expr_as_literal(cx, arg_expr) else {
37+
return;
38+
};
39+
40+
match method.suggestion(literal) {
41+
None => {
42+
let msg = format!("use of `{method}` with zero");
43+
span_lint_and_then(cx, PTR_OFFSET_BY_LITERAL, expr.span, msg, |diag| {
44+
diag.span_suggestion(
45+
expr.span.with_lo(recv.span.hi()),
46+
format!("remove the call to `{method}`"),
47+
String::new(),
48+
Applicability::MachineApplicable,
49+
);
50+
});
51+
},
52+
Some(method_suggestion) => {
53+
let msg = format!("use of `{method}` with a literal");
54+
span_lint_and_then(cx, PTR_OFFSET_BY_LITERAL, expr.span, msg, |diag| {
55+
diag.multipart_suggestion(
56+
format!("use `{method_suggestion}` instead"),
57+
vec![
58+
(method_name.ident.span, method_suggestion.to_string()),
59+
(arg_expr.span, literal_text),
60+
],
61+
Applicability::MachineApplicable,
62+
);
63+
});
64+
},
65+
}
66+
}
67+
68+
fn get_literal_bits<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<u128> {
69+
let ExprKind::Lit(lit) = expr.kind else {
70+
return None;
71+
};
72+
73+
let LitKind::Int(packed_u128, _) = lit.node else {
74+
return None;
75+
};
76+
77+
Some(packed_u128.get())
78+
}
79+
80+
// If the given expression is a (negated) literal, return its value.
81+
fn expr_as_literal<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<(i128, String)> {
82+
if let Some(literal_bits) = get_literal_bits(expr) {
83+
// The value must fit in a isize, so we can't have overflow here.
84+
return Some((literal_bits.cast_signed(), format_isize_literal(cx, expr)?));
85+
}
86+
87+
if let ExprKind::Unary(UnOp::Neg, inner) = expr.kind
88+
&& let Some(literal_bits) = get_literal_bits(inner)
89+
{
90+
return Some((-(literal_bits.cast_signed()), format_isize_literal(cx, inner)?));
91+
}
92+
93+
None
94+
}
95+
96+
fn format_isize_literal<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<String> {
97+
let text = expr.span.get_source_text(cx)?;
98+
let text = match peel_parens_str(&text) {
99+
Some((_, text, _)) => text,
100+
None => &text,
101+
};
102+
103+
Some(text.trim_end_matches("isize").trim_end_matches('_').to_string())
104+
}
105+
106+
fn peel_parens_str(snippet: &str) -> Option<(usize, &str, usize)> {
107+
let trimmed = snippet.trim();
108+
if !(trimmed.starts_with('(') && trimmed.ends_with(')')) {
109+
return None;
110+
}
111+
112+
let trim_start = (snippet.len() - snippet.trim_start().len()) + 1;
113+
let trim_end = (snippet.len() - snippet.trim_end().len()) + 1;
114+
115+
let inner = snippet.get(trim_start..snippet.len() - trim_end)?;
116+
Some(match peel_parens_str(inner) {
117+
None => (trim_start, inner, trim_end),
118+
Some((start, inner, end)) => (trim_start + start, inner, trim_end + end),
119+
})
120+
}
121+
122+
#[derive(Copy, Clone)]
123+
enum Method {
124+
Offset,
125+
WrappingOffset,
126+
}
127+
128+
impl Method {
129+
fn suggestion(self, literal: i128) -> Option<&'static str> {
130+
match Ord::cmp(&literal, &0) {
131+
Ordering::Greater => match self {
132+
Method::Offset => Some("add"),
133+
Method::WrappingOffset => Some("wrapping_add"),
134+
},
135+
Ordering::Equal => None,
136+
Ordering::Less => match self {
137+
Method::Offset => Some("sub"),
138+
Method::WrappingOffset => Some("wrapping_sub"),
139+
},
140+
}
141+
}
142+
}
143+
144+
impl fmt::Display for Method {
145+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
146+
match self {
147+
Self::Offset => write!(f, "offset"),
148+
Self::WrappingOffset => write!(f, "wrapping_offset"),
149+
}
150+
}
151+
}

tests/ui/borrow_as_ptr.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@aux-build:proc_macros.rs
22
#![warn(clippy::borrow_as_ptr)]
3-
#![allow(clippy::useless_vec)]
3+
#![allow(clippy::useless_vec, clippy::ptr_offset_by_literal)]
44

55
extern crate proc_macros;
66

tests/ui/borrow_as_ptr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@aux-build:proc_macros.rs
22
#![warn(clippy::borrow_as_ptr)]
3-
#![allow(clippy::useless_vec)]
3+
#![allow(clippy::useless_vec, clippy::ptr_offset_by_literal)]
44

55
extern crate proc_macros;
66

tests/ui/crashes/ice-4579.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@ check-pass
22

3-
#![allow(clippy::single_match)]
3+
#![allow(clippy::single_match, clippy::ptr_offset_by_literal)]
44

55
use std::ptr;
66

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
#![allow(clippy::inconsistent_digit_grouping)]
2+
3+
fn main() {
4+
let arr = [b'a', b'b', b'c'];
5+
let ptr = arr.as_ptr();
6+
7+
let var = 32;
8+
const CONST: isize = 42;
9+
10+
unsafe {
11+
let _ = ptr;
12+
//~^ ptr_offset_by_literal
13+
let _ = ptr;
14+
//~^ ptr_offset_by_literal
15+
16+
let _ = ptr.add(5);
17+
//~^ ptr_offset_by_literal
18+
let _ = ptr.sub(5);
19+
//~^ ptr_offset_by_literal
20+
21+
let _ = ptr.offset(var);
22+
let _ = ptr.offset(CONST);
23+
24+
let _ = ptr.wrapping_add(5);
25+
//~^ ptr_offset_by_literal
26+
let _ = ptr.wrapping_sub(5);
27+
//~^ ptr_offset_by_literal
28+
29+
let _ = ptr.sub(5);
30+
//~^ ptr_offset_by_literal
31+
let _ = ptr.wrapping_sub(5);
32+
//~^ ptr_offset_by_literal
33+
34+
// isize::MAX and isize::MIN on 64-bit systems.
35+
let _ = ptr.add(9_223_372_036_854_775_807);
36+
//~^ ptr_offset_by_literal
37+
let _ = ptr.sub(9_223_372_036_854_775_808);
38+
//~^ ptr_offset_by_literal
39+
40+
let _ = ptr.add(5_0);
41+
//~^ ptr_offset_by_literal
42+
let _ = ptr.sub(5_0);
43+
//~^ ptr_offset_by_literal
44+
}
45+
}

tests/ui/ptr_offset_by_literal.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
#![allow(clippy::inconsistent_digit_grouping)]
2+
3+
fn main() {
4+
let arr = [b'a', b'b', b'c'];
5+
let ptr = arr.as_ptr();
6+
7+
let var = 32;
8+
const CONST: isize = 42;
9+
10+
unsafe {
11+
let _ = ptr.offset(0);
12+
//~^ ptr_offset_by_literal
13+
let _ = ptr.offset(-0);
14+
//~^ ptr_offset_by_literal
15+
16+
let _ = ptr.offset(5);
17+
//~^ ptr_offset_by_literal
18+
let _ = ptr.offset(-5);
19+
//~^ ptr_offset_by_literal
20+
21+
let _ = ptr.offset(var);
22+
let _ = ptr.offset(CONST);
23+
24+
let _ = ptr.wrapping_offset(5isize);
25+
//~^ ptr_offset_by_literal
26+
let _ = ptr.wrapping_offset(-5isize);
27+
//~^ ptr_offset_by_literal
28+
29+
let _ = ptr.offset(-(5));
30+
//~^ ptr_offset_by_literal
31+
let _ = ptr.wrapping_offset(-(5));
32+
//~^ ptr_offset_by_literal
33+
34+
// isize::MAX and isize::MIN on 64-bit systems.
35+
let _ = ptr.offset(9_223_372_036_854_775_807isize);
36+
//~^ ptr_offset_by_literal
37+
let _ = ptr.offset(-9_223_372_036_854_775_808isize);
38+
//~^ ptr_offset_by_literal
39+
40+
let _ = ptr.offset(5_0__isize);
41+
//~^ ptr_offset_by_literal
42+
let _ = ptr.offset(-5_0__isize);
43+
//~^ ptr_offset_by_literal
44+
}
45+
}

0 commit comments

Comments
 (0)