Skip to content

Commit 65c339e

Browse files
authored
add ptr_offset_by_literal lint (#15606)
Related to https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast (and very much modelled off of its implementation). This lint is motivated by cleaning up the output of [c2rust](https://c2rust.com/), which contains many `.offset` calls with by literal. The lint also nicely handles offsets by zero, which are unnecessary. I'm aware that the feature freeze is still in place, but this was top-of-mind now. I'll patiently wait until the freeze is lifted. changelog: [`ptr_offset_by_literal`]: add `ptr_offset_by_literal` lint
2 parents 2c95550 + a6162c3 commit 65c339e

File tree

11 files changed

+422
-4
lines changed

11 files changed

+422
-4
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6766,6 +6766,7 @@ Released 2018-09-13
67666766
[`ptr_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr
67676767
[`ptr_cast_constness`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_cast_constness
67686768
[`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
6769+
[`ptr_offset_by_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_by_literal
67696770
[`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
67706771
[`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names
67716772
[`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
@@ -444,6 +444,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
444444
crate::methods::OR_THEN_UNWRAP_INFO,
445445
crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
446446
crate::methods::PATH_ENDS_WITH_EXT_INFO,
447+
crate::methods::PTR_OFFSET_BY_LITERAL_INFO,
447448
crate::methods::PTR_OFFSET_WITH_CAST_INFO,
448449
crate::methods::RANGE_ZIP_WITH_LEN_INFO,
449450
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+
pedantic,
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: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
#![warn(clippy::ptr_offset_by_literal)]
2+
#![allow(clippy::inconsistent_digit_grouping)]
3+
4+
fn main() {
5+
let arr = [b'a', b'b', b'c'];
6+
let ptr = arr.as_ptr();
7+
8+
let var = 32;
9+
const CONST: isize = 42;
10+
11+
unsafe {
12+
let _ = ptr;
13+
//~^ ptr_offset_by_literal
14+
let _ = ptr;
15+
//~^ ptr_offset_by_literal
16+
17+
let _ = ptr.add(5);
18+
//~^ ptr_offset_by_literal
19+
let _ = ptr.sub(5);
20+
//~^ ptr_offset_by_literal
21+
22+
let _ = ptr.offset(var);
23+
let _ = ptr.offset(CONST);
24+
25+
let _ = ptr.wrapping_add(5);
26+
//~^ ptr_offset_by_literal
27+
let _ = ptr.wrapping_sub(5);
28+
//~^ ptr_offset_by_literal
29+
30+
let _ = ptr.sub(5);
31+
//~^ ptr_offset_by_literal
32+
let _ = ptr.wrapping_sub(5);
33+
//~^ ptr_offset_by_literal
34+
35+
// isize::MAX and isize::MIN on 32-bit systems.
36+
let _ = ptr.add(2_147_483_647);
37+
//~^ ptr_offset_by_literal
38+
let _ = ptr.sub(2_147_483_648);
39+
//~^ ptr_offset_by_literal
40+
41+
let _ = ptr.add(5_0);
42+
//~^ ptr_offset_by_literal
43+
let _ = ptr.sub(5_0);
44+
//~^ ptr_offset_by_literal
45+
46+
macro_rules! offs { { $e:expr, $offs:expr } => { $e.offset($offs) }; }
47+
offs!(ptr, 6);
48+
offs!(ptr, var);
49+
}
50+
}

tests/ui/ptr_offset_by_literal.rs

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

0 commit comments

Comments
 (0)