Skip to content

Commit 5017b8a

Browse files
committed
add ptr_offset_by_literal lint
1 parent 5ac9657 commit 5017b8a

File tree

11 files changed

+305
-4
lines changed

11 files changed

+305
-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
@@ -625,6 +625,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
625625
crate::ptr::MUT_FROM_REF_INFO,
626626
crate::ptr::PTR_ARG_INFO,
627627
crate::ptr::PTR_EQ_INFO,
628+
crate::ptr_offset_by_literal::PTR_OFFSET_BY_LITERAL_INFO,
628629
crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO,
629630
crate::pub_underscore_fields::PUB_UNDERSCORE_FIELDS_INFO,
630631
crate::pub_use::PUB_USE_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ mod permissions_set_readonly_false;
302302
mod pointers_in_nomem_asm_block;
303303
mod precedence;
304304
mod ptr;
305+
mod ptr_offset_by_literal;
305306
mod ptr_offset_with_cast;
306307
mod pub_underscore_fields;
307308
mod pub_use;
@@ -592,6 +593,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
592593
store.register_late_pass(|_| Box::new(unwrap::Unwrap));
593594
store.register_late_pass(move |_| Box::new(indexing_slicing::IndexingSlicing::new(conf)));
594595
store.register_late_pass(move |tcx| Box::new(non_copy_const::NonCopyConst::new(tcx, conf)));
596+
store.register_late_pass(|_| Box::new(ptr_offset_by_literal::PtrOffsetByLiteral));
595597
store.register_late_pass(|_| Box::new(ptr_offset_with_cast::PtrOffsetWithCast));
596598
store.register_late_pass(|_| Box::new(redundant_clone::RedundantClone));
597599
store.register_late_pass(|_| Box::new(slow_vector_initialization::SlowVectorInit));
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
2+
use clippy_utils::source::SpanRangeExt;
3+
use clippy_utils::sym;
4+
use rustc_ast::LitKind;
5+
use rustc_errors::Applicability;
6+
use rustc_hir::{Expr, ExprKind, UnOp};
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_session::declare_lint_pass;
9+
use std::cmp::Ordering;
10+
use std::fmt;
11+
12+
declare_clippy_lint! {
13+
/// ### What it does
14+
/// Checks for usage of the `offset` pointer method with an integer
15+
/// literal.
16+
///
17+
/// ### Why is this bad?
18+
/// The `add` and `sub` methods more accurately express the intent.
19+
///
20+
/// ### Example
21+
/// ```no_run
22+
/// let vec = vec![b'a', b'b', b'c'];
23+
/// let ptr = vec.as_ptr();
24+
///
25+
/// unsafe {
26+
/// ptr.offset(-8);
27+
/// }
28+
/// ```
29+
///
30+
/// Could be written:
31+
///
32+
/// ```no_run
33+
/// let vec = vec![b'a', b'b', b'c'];
34+
/// let ptr = vec.as_ptr();
35+
///
36+
/// unsafe {
37+
/// ptr.sub(8);
38+
/// }
39+
/// ```
40+
#[clippy::version = "CURRENT_RUSTC_VERSION"]
41+
pub PTR_OFFSET_BY_LITERAL,
42+
complexity,
43+
"unneeded pointer offset"
44+
}
45+
46+
declare_lint_pass!(PtrOffsetByLiteral => [PTR_OFFSET_BY_LITERAL]);
47+
48+
impl<'tcx> LateLintPass<'tcx> for PtrOffsetByLiteral {
49+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
50+
// Check if the expressions is a ptr.offset or ptr.wrapping_offset method call
51+
let Some((receiver_expr, arg_expr, method)) = expr_as_ptr_offset_call(cx, expr) else {
52+
return;
53+
};
54+
55+
// Check if the argument to the method call is a (negated) literal.
56+
let Some(literal) = expr_as_literal(arg_expr) else {
57+
return;
58+
};
59+
60+
let msg = format!("use of `{method}` with a literal");
61+
if let Some(sugg) = build_suggestion(cx, method, receiver_expr, literal) {
62+
span_lint_and_sugg(
63+
cx,
64+
PTR_OFFSET_BY_LITERAL,
65+
expr.span,
66+
msg,
67+
"try",
68+
sugg,
69+
Applicability::MachineApplicable,
70+
);
71+
} else {
72+
span_lint(cx, PTR_OFFSET_BY_LITERAL, expr.span, msg);
73+
}
74+
}
75+
}
76+
77+
fn get_literal_bits<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<u128> {
78+
let ExprKind::Lit(lit) = expr.kind else {
79+
return None;
80+
};
81+
82+
let LitKind::Int(packed_u128, _) = lit.node else {
83+
return None;
84+
};
85+
86+
Some(packed_u128.get())
87+
}
88+
89+
// If the given expression is a (negated) literal, return its value.
90+
fn expr_as_literal<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<i128> {
91+
if let Some(literal_bits) = get_literal_bits(expr) {
92+
// The value must fit in a isize, so we can't have overflow here.
93+
return Some(literal_bits as i128);
94+
}
95+
96+
if let ExprKind::Unary(UnOp::Neg, inner) = expr.kind {
97+
if let Some(literal_bits) = get_literal_bits(inner) {
98+
return Some(-1 * literal_bits as i128);
99+
}
100+
}
101+
102+
None
103+
}
104+
105+
// If the given expression is a ptr::offset or ptr::wrapping_offset method call, return the
106+
// receiver, the arg of the method call, and the method.
107+
fn expr_as_ptr_offset_call<'tcx>(
108+
cx: &LateContext<'tcx>,
109+
expr: &'tcx Expr<'_>,
110+
) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>, Method)> {
111+
if let ExprKind::MethodCall(path_segment, arg_0, [arg_1], _) = &expr.kind
112+
&& cx.typeck_results().expr_ty(arg_0).is_raw_ptr()
113+
{
114+
if path_segment.ident.name == sym::offset {
115+
return Some((arg_0, arg_1, Method::Offset));
116+
}
117+
if path_segment.ident.name == sym::wrapping_offset {
118+
return Some((arg_0, arg_1, Method::WrappingOffset));
119+
}
120+
}
121+
None
122+
}
123+
124+
fn build_suggestion(cx: &LateContext<'_>, method: Method, receiver_expr: &Expr<'_>, literal: i128) -> Option<String> {
125+
let receiver = receiver_expr.span.get_source_text(cx)?;
126+
127+
let new_method = match Ord::cmp(&literal, &0) {
128+
Ordering::Greater => match method {
129+
Method::Offset => "add",
130+
Method::WrappingOffset => "wrapping_add",
131+
},
132+
Ordering::Equal => return Some(format!("{receiver}")),
133+
Ordering::Less => match method {
134+
Method::Offset => "sub",
135+
Method::WrappingOffset => "wrapping_sub",
136+
},
137+
};
138+
139+
let literal = literal.unsigned_abs();
140+
Some(format!("{receiver}.{new_method}({literal})"))
141+
}
142+
143+
#[derive(Copy, Clone)]
144+
enum Method {
145+
Offset,
146+
WrappingOffset,
147+
}
148+
149+
impl fmt::Display for Method {
150+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
151+
match self {
152+
Self::Offset => write!(f, "offset"),
153+
Self::WrappingOffset => write!(f, "wrapping_offset"),
154+
}
155+
}
156+
}

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: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
fn main() {
2+
let arr = [b'a', b'b', b'c'];
3+
let ptr = arr.as_ptr();
4+
5+
let var = 32;
6+
const CONST: isize = 42;
7+
8+
unsafe {
9+
let _ = ptr;
10+
//~^ ptr_offset_by_literal
11+
let _ = ptr;
12+
//~^ ptr_offset_by_literal
13+
14+
let _ = ptr.add(5);
15+
//~^ ptr_offset_by_literal
16+
let _ = ptr.sub(5);
17+
//~^ ptr_offset_by_literal
18+
19+
let _ = ptr.offset(var);
20+
let _ = ptr.offset(CONST);
21+
22+
let _ = ptr.wrapping_add(5);
23+
//~^ ptr_offset_by_literal
24+
let _ = ptr.wrapping_sub(5);
25+
//~^ ptr_offset_by_literal
26+
27+
let _ = ptr.sub(5);
28+
//~^ ptr_offset_by_literal
29+
let _ = ptr.wrapping_sub(5);
30+
//~^ ptr_offset_by_literal
31+
32+
// isize::MAX and isize::MIN on 64-bit systems.
33+
let _ = ptr.add(9223372036854775807);
34+
//~^ ptr_offset_by_literal
35+
let _ = ptr.sub(9223372036854775808);
36+
//~^ ptr_offset_by_literal
37+
}
38+
}

tests/ui/ptr_offset_by_literal.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
fn main() {
2+
let arr = [b'a', b'b', b'c'];
3+
let ptr = arr.as_ptr();
4+
5+
let var = 32;
6+
const CONST: isize = 42;
7+
8+
unsafe {
9+
let _ = ptr.offset(0);
10+
//~^ ptr_offset_by_literal
11+
let _ = ptr.offset(-0);
12+
//~^ ptr_offset_by_literal
13+
14+
let _ = ptr.offset(5);
15+
//~^ ptr_offset_by_literal
16+
let _ = ptr.offset(-5);
17+
//~^ ptr_offset_by_literal
18+
19+
let _ = ptr.offset(var);
20+
let _ = ptr.offset(CONST);
21+
22+
let _ = ptr.wrapping_offset(5isize);
23+
//~^ ptr_offset_by_literal
24+
let _ = ptr.wrapping_offset(-5isize);
25+
//~^ ptr_offset_by_literal
26+
27+
let _ = ptr.offset(-(5));
28+
//~^ ptr_offset_by_literal
29+
let _ = ptr.wrapping_offset(-(5));
30+
//~^ ptr_offset_by_literal
31+
32+
// isize::MAX and isize::MIN on 64-bit systems.
33+
let _ = ptr.offset(9_223_372_036_854_775_807isize);
34+
//~^ ptr_offset_by_literal
35+
let _ = ptr.offset(-9_223_372_036_854_775_808isize);
36+
//~^ ptr_offset_by_literal
37+
}
38+
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
error: use of `offset` with a literal
2+
--> tests/ui/ptr_offset_by_literal.rs:9:17
3+
|
4+
LL | let _ = ptr.offset(0);
5+
| ^^^^^^^^^^^^^ help: try: `ptr`
6+
|
7+
= note: `-D clippy::ptr-offset-by-literal` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::ptr_offset_by_literal)]`
9+
10+
error: use of `offset` with a literal
11+
--> tests/ui/ptr_offset_by_literal.rs:11:17
12+
|
13+
LL | let _ = ptr.offset(-0);
14+
| ^^^^^^^^^^^^^^ help: try: `ptr`
15+
16+
error: use of `offset` with a literal
17+
--> tests/ui/ptr_offset_by_literal.rs:14:17
18+
|
19+
LL | let _ = ptr.offset(5);
20+
| ^^^^^^^^^^^^^ help: try: `ptr.add(5)`
21+
22+
error: use of `offset` with a literal
23+
--> tests/ui/ptr_offset_by_literal.rs:16:17
24+
|
25+
LL | let _ = ptr.offset(-5);
26+
| ^^^^^^^^^^^^^^ help: try: `ptr.sub(5)`
27+
28+
error: use of `wrapping_offset` with a literal
29+
--> tests/ui/ptr_offset_by_literal.rs:22:17
30+
|
31+
LL | let _ = ptr.wrapping_offset(5isize);
32+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.wrapping_add(5)`
33+
34+
error: use of `wrapping_offset` with a literal
35+
--> tests/ui/ptr_offset_by_literal.rs:24:17
36+
|
37+
LL | let _ = ptr.wrapping_offset(-5isize);
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.wrapping_sub(5)`
39+
40+
error: use of `offset` with a literal
41+
--> tests/ui/ptr_offset_by_literal.rs:27:17
42+
|
43+
LL | let _ = ptr.offset(-(5));
44+
| ^^^^^^^^^^^^^^^^ help: try: `ptr.sub(5)`
45+
46+
error: use of `wrapping_offset` with a literal
47+
--> tests/ui/ptr_offset_by_literal.rs:29:17
48+
|
49+
LL | let _ = ptr.wrapping_offset(-(5));
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.wrapping_sub(5)`
51+
52+
error: use of `offset` with a literal
53+
--> tests/ui/ptr_offset_by_literal.rs:33:17
54+
|
55+
LL | let _ = ptr.offset(9_223_372_036_854_775_807isize);
56+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.add(9223372036854775807)`
57+
58+
error: use of `offset` with a literal
59+
--> tests/ui/ptr_offset_by_literal.rs:35:17
60+
|
61+
LL | let _ = ptr.offset(-9_223_372_036_854_775_808isize);
62+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.sub(9223372036854775808)`
63+
64+
error: aborting due to 10 previous errors
65+

0 commit comments

Comments
 (0)