Skip to content

Commit 88bdb21

Browse files
committed
add ptr_offset_by_literal lint
1 parent cd61be7 commit 88bdb21

File tree

11 files changed

+420
-4
lines changed

11 files changed

+420
-4
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6764,6 +6764,7 @@ Released 2018-09-13
67646764
[`ptr_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr
67656765
[`ptr_cast_constness`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_cast_constness
67666766
[`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
6767+
[`ptr_offset_by_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_by_literal
67676768
[`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
67686769
[`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names
67696770
[`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
@@ -443,6 +443,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
443443
crate::methods::OR_THEN_UNWRAP_INFO,
444444
crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
445445
crate::methods::PATH_ENDS_WITH_EXT_INFO,
446+
crate::methods::PTR_OFFSET_BY_LITERAL_INFO,
446447
crate::methods::PTR_OFFSET_WITH_CAST_INFO,
447448
crate::methods::RANGE_ZIP_WITH_LEN_INFO,
448449
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
@@ -94,6 +94,7 @@ mod or_fun_call;
9494
mod or_then_unwrap;
9595
mod path_buf_push_overwrite;
9696
mod path_ends_with_ext;
97+
mod ptr_offset_by_literal;
9798
mod ptr_offset_with_cast;
9899
mod range_zip_with_len;
99100
mod read_line_without_trim;
@@ -1728,6 +1729,40 @@ declare_clippy_lint! {
17281729
"Check for offset calculations on raw pointers to zero-sized types"
17291730
}
17301731

1732+
declare_clippy_lint! {
1733+
/// ### What it does
1734+
/// Checks for usage of the `offset` pointer method with an integer
1735+
/// literal.
1736+
///
1737+
/// ### Why is this bad?
1738+
/// The `add` and `sub` methods more accurately express the intent.
1739+
///
1740+
/// ### Example
1741+
/// ```no_run
1742+
/// let vec = vec![b'a', b'b', b'c'];
1743+
/// let ptr = vec.as_ptr();
1744+
///
1745+
/// unsafe {
1746+
/// ptr.offset(-8);
1747+
/// }
1748+
/// ```
1749+
///
1750+
/// Could be written:
1751+
///
1752+
/// ```no_run
1753+
/// let vec = vec![b'a', b'b', b'c'];
1754+
/// let ptr = vec.as_ptr();
1755+
///
1756+
/// unsafe {
1757+
/// ptr.sub(8);
1758+
/// }
1759+
/// ```
1760+
#[clippy::version = "1.92.0"]
1761+
pub PTR_OFFSET_BY_LITERAL,
1762+
complexity,
1763+
"unneeded pointer offset"
1764+
}
1765+
17311766
declare_clippy_lint! {
17321767
/// ### What it does
17331768
/// Checks for usage of the `offset` pointer method with a `usize` casted to an
@@ -4803,6 +4838,7 @@ impl_lint_pass!(Methods => [
48034838
UNINIT_ASSUMED_INIT,
48044839
MANUAL_SATURATING_ARITHMETIC,
48054840
ZST_OFFSET,
4841+
PTR_OFFSET_BY_LITERAL,
48064842
PTR_OFFSET_WITH_CAST,
48074843
FILETYPE_IS_FILE,
48084844
OPTION_AS_REF_DEREF,
@@ -5426,6 +5462,7 @@ impl Methods {
54265462
zst_offset::check(cx, expr, recv);
54275463

54285464
ptr_offset_with_cast::check(cx, name, expr, recv, arg, self.msrv);
5465+
ptr_offset_by_literal::check(cx, expr, self.msrv);
54295466
},
54305467
(sym::ok_or_else, [arg]) => {
54315468
unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or");
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
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, Lit, 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+
match expr.kind {
70+
ExprKind::Lit(Lit {
71+
node: LitKind::Int(packed_u128, _),
72+
..
73+
}) => Some(packed_u128.get()),
74+
_ => None,
75+
}
76+
}
77+
78+
// If the given expression is a (negated) literal, return its value.
79+
fn expr_as_literal<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<(i128, String)> {
80+
if let Some(literal_bits) = get_literal_bits(expr) {
81+
// The value must fit in a isize, so we can't have overflow here.
82+
return Some((literal_bits.cast_signed(), format_isize_literal(cx, expr)?));
83+
}
84+
85+
if let ExprKind::Unary(UnOp::Neg, inner) = expr.kind
86+
&& let Some(literal_bits) = get_literal_bits(inner)
87+
{
88+
return Some((-(literal_bits.cast_signed()), format_isize_literal(cx, inner)?));
89+
}
90+
91+
None
92+
}
93+
94+
fn format_isize_literal<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<String> {
95+
let text = expr.span.get_source_text(cx)?;
96+
let text = peel_parens_str(&text);
97+
Some(text.trim_end_matches("isize").trim_end_matches('_').to_string())
98+
}
99+
100+
fn peel_parens_str(snippet: &str) -> &str {
101+
let mut s = snippet.trim();
102+
while let Some(next) = s.strip_prefix("(").and_then(|suf| suf.strip_suffix(")")) {
103+
s = next.trim();
104+
}
105+
s
106+
}
107+
108+
#[derive(Copy, Clone)]
109+
enum Method {
110+
Offset,
111+
WrappingOffset,
112+
}
113+
114+
impl Method {
115+
fn suggestion(self, literal: i128) -> Option<&'static str> {
116+
match Ord::cmp(&literal, &0) {
117+
Ordering::Greater => match self {
118+
Method::Offset => Some("add"),
119+
Method::WrappingOffset => Some("wrapping_add"),
120+
},
121+
// `ptr.offset(0)` is equivalent to `ptr`, so no adjustment is needed
122+
Ordering::Equal => None,
123+
Ordering::Less => match self {
124+
Method::Offset => Some("sub"),
125+
Method::WrappingOffset => Some("wrapping_sub"),
126+
},
127+
}
128+
}
129+
}
130+
131+
impl fmt::Display for Method {
132+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
133+
match self {
134+
Self::Offset => write!(f, "offset"),
135+
Self::WrappingOffset => write!(f, "wrapping_offset"),
136+
}
137+
}
138+
}

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: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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+
macro_rules! offs { { $e:expr, $offs:expr } => { $e.offset($offs) }; }
46+
offs!(ptr, 6);
47+
offs!(ptr, var);
48+
}
49+
}

tests/ui/ptr_offset_by_literal.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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+
macro_rules! offs { { $e:expr, $offs:expr } => { $e.offset($offs) }; }
46+
offs!(ptr, 6);
47+
offs!(ptr, var);
48+
}
49+
}

0 commit comments

Comments
 (0)