Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add borrow_as_ptr lint #8210

Merged
merged 1 commit into from
Jan 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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