Skip to content

Commit

Permalink
add new lint as_underscore_ptr to check for as *{const,mut} _
Browse files Browse the repository at this point in the history
  • Loading branch information
asquared31415 committed Mar 30, 2023
1 parent 58fb801 commit 033da3c
Show file tree
Hide file tree
Showing 38 changed files with 262 additions and 45 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4392,6 +4392,7 @@ Released 2018-09-13
[`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions
[`as_ptr_cast_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_ptr_cast_mut
[`as_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore
[`as_underscore_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore_ptr
[`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
[`assertions_on_result_states`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_result_states
[`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
Expand Down
47 changes: 47 additions & 0 deletions clippy_lints/src/casts/as_underscore_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use clippy_utils::diagnostics::span_lint_and_then;
use rustc_errors::Applicability;
use rustc_hir::{Expr, MutTy, Ty, TyKind};
use rustc_lint::LateContext;
use rustc_middle::ty;

use super::AS_UNDERSCORE_PTR;

pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, mut ty: &Ty<'_>) {
if let TyKind::Ptr(MutTy { mutbl, .. }) = ty.kind {
// get this before stripping the pointers, so the suggestion suggests replacing the whole type
let ty_span = ty.span;

// strip all pointers from the type
while let TyKind::Ptr(MutTy { ty: new_ty, .. }) = ty.kind {
ty = new_ty;
}

if matches!(ty.kind, TyKind::Infer) {
let mutbl_str = match mutbl {
rustc_ast::Mutability::Not => "const",
rustc_ast::Mutability::Mut => "mut",
};
span_lint_and_then(
cx,
AS_UNDERSCORE_PTR,
expr.span,
format!("using `as *{mutbl_str} _` conversion").as_str(),
|diag| {
let ty_resolved = cx.typeck_results().expr_ty(expr);
if let ty::Error(_) = ty_resolved.kind() {
diag.help("consider giving the type explicitly");
} else {
diag.span_suggestion(
ty_span,
"consider giving the type explicitly",
ty_resolved,
Applicability::MachineApplicable,
);
}
},
);
}
} else {
// not a pointer
}
}
44 changes: 44 additions & 0 deletions clippy_lints/src/casts/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod as_ptr_cast_mut;
mod as_underscore;
mod as_underscore_ptr;
mod borrow_as_ptr;
mod cast_abs_to_unsigned;
mod cast_enum_constructor;
Expand Down Expand Up @@ -555,6 +556,47 @@ declare_clippy_lint! {
"detects `as _` conversion"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for the usage of `as *const _` or `as *mut _` where the pointed to type is inferred.
///
/// ### Why is this bad?
/// When converting to a pointer, it can be dangerous to not specify the type. Type inference can
/// be affected by many things, types may not always be obvious, and when working with pointers, while
/// the pointed to type doesn't technically matter until an access, you should always know what types
/// you intend to work with. When using multiple chained `as` casts, it can be especially dangerous,
/// because `*const T as *const U` **always compiles** (for Sized types T and U).
///
/// ### Example
/// ```rust
/// struct UwU;
/// impl UwU {
/// // intent: turn a `&UwU` into a `*const u8` that points to the same data
/// fn as_ptr(&self) -> *const u8 {
/// // ⚠️ `&self` is a `&&UwU`, so this turns a double pointer into a single pointer
/// // ⚠️ This pointer is a dangling pointer to a local
/// &self as *const _ as *const u8
/// }
/// }
/// ```
/// Use instead:
/// ```rust
/// struct UwU;
/// impl UwU {
/// // intent: turn a `&UwU` into a `*const u8` that points to the same data
/// fn as_ptr(&self) -> *const u8 {
/// // ⚠️ This pointer is still a dangling pointer to a local
/// // ⚠️ But now the error is explicit in the type
/// &self as *const &UwU as *const u8
/// }
/// }
/// ```
#[clippy::version = "1.70.0"]
pub AS_UNDERSCORE_PTR,
pedantic,
"detects `as *{const,mut} _ conversions"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for the usage of `&expr as *const T` or
Expand Down Expand Up @@ -693,6 +735,7 @@ impl_lint_pass!(Casts => [
CAST_ENUM_CONSTRUCTOR,
CAST_ABS_TO_UNSIGNED,
AS_UNDERSCORE,
AS_UNDERSCORE_PTR,
BORROW_AS_PTR,
CAST_SLICE_FROM_RAW_PARTS,
AS_PTR_CAST_MUT,
Expand Down Expand Up @@ -741,6 +784,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
}

as_underscore::check(cx, expr, cast_to_hir);
as_underscore_ptr::check(cx, expr, cast_to_hir);

if self.msrv.meets(msrvs::BORROW_AS_PTR) {
borrow_as_ptr::check(cx, expr, cast_expr, cast_to_hir);
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::cargo::WILDCARD_DEPENDENCIES_INFO,
crate::casts::AS_PTR_CAST_MUT_INFO,
crate::casts::AS_UNDERSCORE_INFO,
crate::casts::AS_UNDERSCORE_PTR_INFO,
crate::casts::BORROW_AS_PTR_INFO,
crate::casts::CAST_ABS_TO_UNSIGNED_INFO,
crate::casts::CAST_ENUM_CONSTRUCTOR_INFO,
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/only_used_in_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_hir::hir_id::HirIdMap;
use rustc_hir::{Body, Expr, ExprKind, HirId, ImplItem, ImplItemKind, Node, PatKind, TraitItem, TraitItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::subst::{EarlyBinder, GenericArgKind, SubstsRef};
use rustc_middle::ty::{self, ConstKind};
use rustc_middle::ty::{self, ConstKind, List};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::{kw, Ident};
use rustc_span::Span;
Expand Down Expand Up @@ -249,7 +249,7 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
{
(
trait_item_id,
FnKind::ImplTraitFn(cx.tcx.erase_regions(trait_ref.substs) as *const _ as usize),
FnKind::ImplTraitFn(cx.tcx.erase_regions(trait_ref.substs) as *const List<_> as usize),
usize::from(sig.decl.implicit_self.has_implicit_self()),
)
} else {
Expand Down Expand Up @@ -390,6 +390,6 @@ fn has_matching_substs(kind: FnKind, substs: SubstsRef<'_>) -> bool {
GenericArgKind::Const(c) => matches!(c.kind(), ConstKind::Param(c) if c.index as usize == idx),
}),
#[allow(trivial_casts)]
FnKind::ImplTraitFn(expected_substs) => substs as *const _ as usize == expected_substs,
FnKind::ImplTraitFn(expected_substs) => substs as *const List<_> as usize == expected_substs,
}
}
2 changes: 2 additions & 0 deletions tests/ui/as_ptr_cast_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ impl<T> Covariant<T> {
fn main() {
let mut string = String::new();
let _ = string.as_ptr() as *mut u8;
#[allow(clippy::as_underscore_ptr)] // trying to test inference with a different lint
let _: *mut i8 = string.as_ptr() as *mut _;
let _ = string.as_ptr() as *const i8;
let _ = string.as_mut_ptr();
Expand All @@ -32,6 +33,7 @@ fn main() {
let _ = wrap.as_ptr() as *mut u8;

let mut local = 4;
#[allow(clippy::as_underscore_ptr)] // trying to test inference with a different lint
let ref_with_write_perm = Covariant(std::ptr::addr_of_mut!(local) as *const _);
let _ = ref_with_write_perm.as_ptr() as *mut u8;
}
2 changes: 1 addition & 1 deletion tests/ui/as_ptr_cast_mut.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | let _ = string.as_ptr() as *mut u8;
= note: `-D clippy::as-ptr-cast-mut` implied by `-D warnings`

error: casting the result of `as_ptr` to *mut i8
--> $DIR/as_ptr_cast_mut.rs:22:22
--> $DIR/as_ptr_cast_mut.rs:23:22
|
LL | let _: *mut i8 = string.as_ptr() as *mut _;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `string.as_mut_ptr()`
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/as_underscore.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,21 @@

fn foo(_n: usize) {}

fn use_ptr(_: *const *const ()) {}

fn main() {
let n: u16 = 256;
foo(n as usize);

let n = 0_u128;
let _n: u8 = n as u8;

let x = 1 as *const ();
use_ptr(x as *const _);
let x2 = x as *const _;
use_ptr(x2);

let _x3: *mut i32 = x as *mut _;

let _x4: *const () = x as *const ();
}
11 changes: 11 additions & 0 deletions tests/ui/as_underscore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,21 @@

fn foo(_n: usize) {}

fn use_ptr(_: *const *const ()) {}

fn main() {
let n: u16 = 256;
foo(n as _);

let n = 0_u128;
let _n: u8 = n as _;

let x = 1 as *const ();
use_ptr(x as *const _);
let x2 = x as *const _;
use_ptr(x2);

let _x3: *mut i32 = x as *mut _;

let _x4: *const () = x as _;
}
14 changes: 11 additions & 3 deletions tests/ui/as_underscore.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: using `as _` conversion
--> $DIR/as_underscore.rs:9:9
--> $DIR/as_underscore.rs:11:9
|
LL | foo(n as _);
| ^^^^^-
Expand All @@ -9,12 +9,20 @@ LL | foo(n as _);
= note: `-D clippy::as-underscore` implied by `-D warnings`

error: using `as _` conversion
--> $DIR/as_underscore.rs:12:18
--> $DIR/as_underscore.rs:14:18
|
LL | let _n: u8 = n as _;
| ^^^^^-
| |
| help: consider giving the type explicitly: `u8`

error: aborting due to 2 previous errors
error: using `as _` conversion
--> $DIR/as_underscore.rs:23:26
|
LL | let _x4: *const () = x as _;
| ^^^^^-
| |
| help: consider giving the type explicitly: `*const ()`

error: aborting due to 3 previous errors

19 changes: 19 additions & 0 deletions tests/ui/as_underscore_ptr.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// run-rustfix

#![allow(unused)]
#![deny(clippy::as_underscore_ptr)]

struct UwU;

impl UwU {
fn as_ptr(&self) -> *const u8 {
&self as *const &UwU as *const u8
}
}

fn use_ptr(_: *const ()) {}

fn main() {
let _: *const u8 = 1 as *const u8;
use_ptr(1 as *const ());
}
19 changes: 19 additions & 0 deletions tests/ui/as_underscore_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// run-rustfix

#![allow(unused)]
#![deny(clippy::as_underscore_ptr)]

struct UwU;

impl UwU {
fn as_ptr(&self) -> *const u8 {
&self as *const _ as *const u8
}
}

fn use_ptr(_: *const ()) {}

fn main() {
let _: *const u8 = 1 as *const _;
use_ptr(1 as *const _);
}
32 changes: 32 additions & 0 deletions tests/ui/as_underscore_ptr.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
error: using `as *const _` conversion
--> $DIR/as_underscore_ptr.rs:10:9
|
LL | &self as *const _ as *const u8
| ^^^^^^^^^--------
| |
| help: consider giving the type explicitly: `*const &UwU`
|
note: the lint level is defined here
--> $DIR/as_underscore_ptr.rs:4:9
|
LL | #![deny(clippy::as_underscore_ptr)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: using `as *const _` conversion
--> $DIR/as_underscore_ptr.rs:17:24
|
LL | let _: *const u8 = 1 as *const _;
| ^^^^^--------
| |
| help: consider giving the type explicitly: `*const u8`

error: using `as *const _` conversion
--> $DIR/as_underscore_ptr.rs:18:13
|
LL | use_ptr(1 as *const _);
| ^^^^^--------
| |
| help: consider giving the type explicitly: `*const ()`

error: aborting due to 3 previous errors

2 changes: 1 addition & 1 deletion tests/ui/author/issue_3849.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#![allow(clippy::transmute_ptr_to_ref)]
#![allow(clippy::transmuting_null)]

pub const ZPTR: *const usize = 0 as *const _;
pub const ZPTR: *const usize = 0 as *const usize;

fn main() {
unsafe {
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/borrow_deref_ref.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ mod should_not_lint2 {
mod false_negative {
fn foo() {
let x = &12;
let addr_x = &x as *const _ as usize;
let addr_y = &x as *const _ as usize; // assert ok
let addr_x = &x as *const &i32 as usize;
let addr_y = &x as *const &i32 as usize; // assert ok
// let addr_y = &x as *const _ as usize; // assert fail
assert_ne!(addr_x, addr_y);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/borrow_deref_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ mod should_not_lint2 {
mod false_negative {
fn foo() {
let x = &12;
let addr_x = &x as *const _ as usize;
let addr_y = &&*x as *const _ as usize; // assert ok
let addr_x = &x as *const &i32 as usize;
let addr_y = &&*x as *const &i32 as usize; // assert ok
// let addr_y = &x as *const _ as usize; // assert fail
assert_ne!(addr_x, addr_y);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/borrow_deref_ref.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ LL | let b = &mut &*bar(&12);
error: deref on an immutable reference
--> $DIR/borrow_deref_ref.rs:55:23
|
LL | let addr_y = &&*x as *const _ as usize; // assert ok
LL | let addr_y = &&*x as *const &i32 as usize; // assert ok
| ^^^ help: if you would like to reborrow, try removing `&*`: `x`

error: aborting due to 3 previous errors
Expand Down
1 change: 1 addition & 0 deletions tests/ui/cast_ref_to_mut.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![warn(clippy::cast_ref_to_mut)]
#![allow(clippy::no_effect, clippy::borrow_as_ptr)]
#![allow(clippy::as_underscore_ptr)] // trying to test inference with a different lint

extern "C" {
// N.B., mutability can be easily incorrect in FFI calls -- as
Expand Down
Loading

0 comments on commit 033da3c

Please sign in to comment.