Skip to content

Commit

Permalink
Add borrow_as_ptr lint
Browse files Browse the repository at this point in the history
Closes: #6995

Signed-off-by: Federico Guerinoni <guerinoni.federico@gmail.com>
Co-authored-by: Paolo Barbolini <paolo@paolo565.org>
  • Loading branch information
2 people authored and Federico Guerinoni committed Jan 11, 2022
1 parent fccf07b commit 3298de7
Show file tree
Hide file tree
Showing 35 changed files with 294 additions and 75 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2887,6 +2887,7 @@ Released 2018-09-13
[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
[`bool_assert_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_assert_comparison
[`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
[`borrow_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_as_ptr
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
[`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box
[`box_collection`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_collection
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are over 450 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are over 500 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.
Expand Down
97 changes: 97 additions & 0 deletions clippy_lints/src/borrow_as_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_no_std_crate;
use clippy_utils::source::snippet_opt;
use clippy_utils::{meets_msrv, msrvs};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, TyKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};

declare_clippy_lint! {
/// ### What it does
/// Checks for the usage of `&expr as *const T` or
/// `&mut expr as *mut T`, and suggest using `ptr::addr_of` or
/// `ptr::addr_of_mut` instead.
///
/// ### Why is this bad?
/// This would improve readability and avoid creating a reference
/// that points to an uninitialized value or unaligned place.
/// Read the `ptr::addr_of` docs for more information.
///
/// ### Example
/// ```rust
/// let val = 1;
/// let p = &val as *const i32;
///
/// let mut val_mut = 1;
/// let p_mut = &mut val_mut as *mut i32;
/// ```
/// Use instead:
/// ```rust
/// let val = 1;
/// let p = std::ptr::addr_of!(val);
///
/// let mut val_mut = 1;
/// let p_mut = std::ptr::addr_of_mut!(val_mut);
/// ```
#[clippy::version = "1.60.0"]
pub BORROW_AS_PTR,
pedantic,
"borrowing just to cast to a raw pointer"
}

impl_lint_pass!(BorrowAsPtr => [BORROW_AS_PTR]);

pub struct BorrowAsPtr {
msrv: Option<RustcVersion>,
}

impl BorrowAsPtr {
#[must_use]
pub fn new(msrv: Option<RustcVersion>) -> Self {
Self { msrv }
}
}

impl<'tcx> LateLintPass<'tcx> for BorrowAsPtr {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if !meets_msrv(self.msrv.as_ref(), &msrvs::BORROW_AS_PTR) {
return;
}

if expr.span.from_expansion() {
return;
}

if_chain! {
if let ExprKind::Cast(left_expr, ty) = &expr.kind;
if let TyKind::Ptr(_) = ty.kind;
if let ExprKind::AddrOf(BorrowKind::Ref, mutability, e) = &left_expr.kind;

then {
let core_or_std = if is_no_std_crate(cx) { "core" } else { "std" };
let macro_name = match mutability {
Mutability::Not => "addr_of",
Mutability::Mut => "addr_of_mut",
};

span_lint_and_sugg(
cx,
BORROW_AS_PTR,
expr.span,
"borrow as raw pointer",
"try",
format!(
"{}::ptr::{}!({})",
core_or_std,
macro_name,
snippet_opt(cx, e.span).unwrap()
),
Applicability::MachineApplicable,
);
}
}
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ store.register_lints(&[
bool_assert_comparison::BOOL_ASSERT_COMPARISON,
booleans::LOGIC_BUG,
booleans::NONMINIMAL_BOOL,
borrow_as_ptr::BORROW_AS_PTR,
bytecount::NAIVE_BYTECOUNT,
cargo_common_metadata::CARGO_COMMON_METADATA,
case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_pedantic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK),
LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
LintId::of(bit_mask::VERBOSE_BIT_MASK),
LintId::of(borrow_as_ptr::BORROW_AS_PTR),
LintId::of(bytecount::NAIVE_BYTECOUNT),
LintId::of(case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS),
LintId::of(casts::CAST_LOSSLESS),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ mod blacklisted_name;
mod blocks_in_if_conditions;
mod bool_assert_comparison;
mod booleans;
mod borrow_as_ptr;
mod bytecount;
mod cargo_common_metadata;
mod case_sensitive_file_extension_comparisons;
Expand Down Expand Up @@ -857,6 +858,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(return_self_not_must_use::ReturnSelfNotMustUse));
store.register_late_pass(|| Box::new(init_numbered_fields::NumberedFields));
store.register_early_pass(|| Box::new(single_char_lifetime_names::SingleCharLifetimeNames));
store.register_late_pass(move || Box::new(borrow_as_ptr::BorrowAsPtr::new(msrv)));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
1 change: 1 addition & 0 deletions clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ macro_rules! msrv_aliases {
msrv_aliases! {
1,53,0 { OR_PATTERNS }
1,52,0 { STR_SPLIT_ONCE }
1,51,0 { BORROW_AS_PTR }
1,50,0 { BOOL_THEN }
1,47,0 { TAU }
1,46,0 { CONST_IF_MATCH }
Expand Down
1 change: 1 addition & 0 deletions tests/ui/as_conversions.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// aux-build:macro_rules.rs

#![warn(clippy::as_conversions)]
#![allow(clippy::borrow_as_ptr)]

#[macro_use]
extern crate macro_rules;
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/as_conversions.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: using a potentially dangerous silent `as` conversion
--> $DIR/as_conversions.rs:14:13
--> $DIR/as_conversions.rs:15:13
|
LL | let i = 0u32 as u64;
| ^^^^^^^^^^^
Expand All @@ -8,15 +8,15 @@ LL | let i = 0u32 as u64;
= help: consider using a safe wrapper for this conversion

error: using a potentially dangerous silent `as` conversion
--> $DIR/as_conversions.rs:16:13
--> $DIR/as_conversions.rs:17:13
|
LL | let j = &i as *const u64 as *mut u64;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider using a safe wrapper for this conversion

error: using a potentially dangerous silent `as` conversion
--> $DIR/as_conversions.rs:16:13
--> $DIR/as_conversions.rs:17:13
|
LL | let j = &i as *const u64 as *mut u64;
| ^^^^^^^^^^^^^^^^
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/borrow_as_ptr.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// run-rustfix
#![warn(clippy::borrow_as_ptr)]

fn main() {
let val = 1;
let _p = std::ptr::addr_of!(val);

let mut val_mut = 1;
let _p_mut = std::ptr::addr_of_mut!(val_mut);
}
10 changes: 10 additions & 0 deletions tests/ui/borrow_as_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// run-rustfix
#![warn(clippy::borrow_as_ptr)]

fn main() {
let val = 1;
let _p = &val as *const i32;

let mut val_mut = 1;
let _p_mut = &mut val_mut as *mut i32;
}
16 changes: 16 additions & 0 deletions tests/ui/borrow_as_ptr.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: borrow as raw pointer
--> $DIR/borrow_as_ptr.rs:6:14
|
LL | let _p = &val as *const i32;
| ^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::addr_of!(val)`
|
= note: `-D clippy::borrow-as-ptr` implied by `-D warnings`

error: borrow as raw pointer
--> $DIR/borrow_as_ptr.rs:9:18
|
LL | let _p_mut = &mut val_mut as *mut i32;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::addr_of_mut!(val_mut)`

error: aborting due to 2 previous errors

22 changes: 22 additions & 0 deletions tests/ui/borrow_as_ptr_no_std.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// run-rustfix
#![warn(clippy::borrow_as_ptr)]
#![feature(lang_items, start, libc)]
#![no_std]

#[start]
fn main(_argc: isize, _argv: *const *const u8) -> isize {
let val = 1;
let _p = core::ptr::addr_of!(val);

let mut val_mut = 1;
let _p_mut = core::ptr::addr_of_mut!(val_mut);
0
}

#[panic_handler]
fn panic(_info: &core::panic::PanicInfo) -> ! {
loop {}
}

#[lang = "eh_personality"]
extern "C" fn eh_personality() {}
22 changes: 22 additions & 0 deletions tests/ui/borrow_as_ptr_no_std.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// run-rustfix
#![warn(clippy::borrow_as_ptr)]
#![feature(lang_items, start, libc)]
#![no_std]

#[start]
fn main(_argc: isize, _argv: *const *const u8) -> isize {
let val = 1;
let _p = &val as *const i32;

let mut val_mut = 1;
let _p_mut = &mut val_mut as *mut i32;
0
}

#[panic_handler]
fn panic(_info: &core::panic::PanicInfo) -> ! {
loop {}
}

#[lang = "eh_personality"]
extern "C" fn eh_personality() {}
16 changes: 16 additions & 0 deletions tests/ui/borrow_as_ptr_no_std.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: borrow as raw pointer
--> $DIR/borrow_as_ptr_no_std.rs:9:14
|
LL | let _p = &val as *const i32;
| ^^^^^^^^^^^^^^^^^^ help: try: `core::ptr::addr_of!(val)`
|
= note: `-D clippy::borrow-as-ptr` implied by `-D warnings`

error: borrow as raw pointer
--> $DIR/borrow_as_ptr_no_std.rs:12:18
|
LL | let _p_mut = &mut val_mut as *mut i32;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `core::ptr::addr_of_mut!(val_mut)`

error: aborting due to 2 previous errors

8 changes: 7 additions & 1 deletion tests/ui/cast_alignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,13 @@
extern crate libc;

#[warn(clippy::cast_ptr_alignment)]
#[allow(clippy::no_effect, clippy::unnecessary_operation, clippy::cast_lossless)]
#[allow(
clippy::no_effect,
clippy::unnecessary_operation,
clippy::cast_lossless,
clippy::borrow_as_ptr
)]

fn main() {
/* These should be warned against */

Expand Down
8 changes: 4 additions & 4 deletions tests/ui/cast_alignment.stderr
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
error: casting from `*const u8` to a more-strictly-aligned pointer (`*const u16`) (1 < 2 bytes)
--> $DIR/cast_alignment.rs:12:5
--> $DIR/cast_alignment.rs:18:5
|
LL | (&1u8 as *const u8) as *const u16;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::cast-ptr-alignment` implied by `-D warnings`

error: casting from `*mut u8` to a more-strictly-aligned pointer (`*mut u16`) (1 < 2 bytes)
--> $DIR/cast_alignment.rs:13:5
--> $DIR/cast_alignment.rs:19:5
|
LL | (&mut 1u8 as *mut u8) as *mut u16;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: casting from `*const u8` to a more-strictly-aligned pointer (`*const u16`) (1 < 2 bytes)
--> $DIR/cast_alignment.rs:16:5
--> $DIR/cast_alignment.rs:22:5
|
LL | (&1u8 as *const u8).cast::<u16>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: casting from `*mut u8` to a more-strictly-aligned pointer (`*mut u16`) (1 < 2 bytes)
--> $DIR/cast_alignment.rs:17:5
--> $DIR/cast_alignment.rs:23:5
|
LL | (&mut 1u8 as *mut u8).cast::<u16>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/cast_ref_to_mut.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![warn(clippy::cast_ref_to_mut)]
#![allow(clippy::no_effect)]
#![allow(clippy::no_effect, clippy::borrow_as_ptr)]

extern "C" {
// N.B., mutability can be easily incorrect in FFI calls -- as
Expand Down
1 change: 1 addition & 0 deletions tests/ui/mutex_atomic.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![warn(clippy::all)]
#![warn(clippy::mutex_integer)]
#![warn(clippy::mutex_atomic)]
#![allow(clippy::borrow_as_ptr)]

fn main() {
use std::sync::Mutex;
Expand Down
Loading

0 comments on commit 3298de7

Please sign in to comment.