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 a new lint ptr_as_ptr #6542

Merged
merged 2 commits into from
Jan 5, 2021
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 @@ -2141,6 +2141,7 @@ Released 2018-09-13
[`print_with_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_with_newline
[`println_empty_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#println_empty_string
[`ptr_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg
[`ptr_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr
[`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
[`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
[`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&types::LET_UNIT_VALUE,
&types::LINKEDLIST,
&types::OPTION_OPTION,
&types::PTR_AS_PTR,
&types::RC_BUFFER,
&types::REDUNDANT_ALLOCATION,
&types::TYPE_COMPLEXITY,
Expand Down Expand Up @@ -1222,6 +1223,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box strings::StringToString);
store.register_late_pass(|| box zero_sized_map_values::ZeroSizedMapValues);
store.register_late_pass(|| box vec_init_then_push::VecInitThenPush::default());
store.register_late_pass(move || box types::PtrAsPtr::new(msrv));

store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
Expand Down Expand Up @@ -1348,6 +1350,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&types::LET_UNIT_VALUE),
LintId::of(&types::LINKEDLIST),
LintId::of(&types::OPTION_OPTION),
LintId::of(&types::PTR_AS_PTR),
LintId::of(&unicode::NON_ASCII_LITERAL),
LintId::of(&unicode::UNICODE_NOT_NFC),
LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS),
Expand Down
101 changes: 97 additions & 4 deletions clippy_lints/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::map::Map;
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::TypeFoldable;
use rustc_middle::ty::{self, InferTy, Ty, TyCtxt, TyS, TypeckResults};
use rustc_middle::ty::{self, InferTy, Ty, TyCtxt, TyS, TypeAndMut, TypeckResults};
use rustc_semver::RustcVersion;
use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
use rustc_span::hygiene::{ExpnKind, MacroKind};
use rustc_span::source_map::Span;
Expand All @@ -30,11 +31,13 @@ use rustc_typeck::hir_ty_to_ty;

use crate::consts::{constant, Constant};
use crate::utils::paths;
use crate::utils::sugg::Sugg;
use crate::utils::{
clip, comparisons, differing_macro_contexts, higher, in_constant, indent_of, int_bits, is_type_diagnostic_item,
last_path_segment, match_def_path, match_path, method_chain_args, multispan_sugg, numeric_literal::NumericLiteral,
qpath_res, reindent_multiline, sext, snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite,
span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext,
last_path_segment, match_def_path, match_path, meets_msrv, method_chain_args, multispan_sugg,
numeric_literal::NumericLiteral, qpath_res, reindent_multiline, sext, snippet, snippet_opt,
snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg,
span_lint_and_then, unsext,
};

declare_clippy_lint! {
Expand Down Expand Up @@ -2878,3 +2881,93 @@ impl<'tcx> LateLintPass<'tcx> for RefToMut {
}
}
}

const PTR_AS_PTR_MSRV: RustcVersion = RustcVersion::new(1, 38, 0);

declare_clippy_lint! {
/// **What it does:**
/// Checks for `as` casts between raw pointers without changing its mutability,
/// namely `*const T` to `*const U` and `*mut T` to `*mut U`.
///
/// **Why is this bad?**
/// Though `as` casts between raw pointers is not terrible, `pointer::cast` is safer because
/// it cannot accidentally change the pointer's mutability nor cast the pointer to other types like `usize`.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// let ptr: *const u32 = &42_u32;
/// let mut_ptr: *mut u32 = &mut 42_u32;
/// let _ = ptr as *const i32;
/// let _ = mut_ptr as *mut i32;
/// ```
/// Use instead:
/// ```rust
/// let ptr: *const u32 = &42_u32;
/// let mut_ptr: *mut u32 = &mut 42_u32;
/// let _ = ptr.cast::<i32>();
/// let _ = mut_ptr.cast::<i32>();
/// ```
pub PTR_AS_PTR,
pedantic,
"casting using `as` from and to raw pointers that doesn't change its mutability, where `pointer::cast` could take the place of `as`"
}

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

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

impl_lint_pass!(PtrAsPtr => [PTR_AS_PTR]);

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

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

if_chain! {
if let ExprKind::Cast(cast_expr, cast_to_hir_ty) = expr.kind;
let (cast_from, cast_to) = (cx.typeck_results().expr_ty(cast_expr), cx.typeck_results().expr_ty(expr));
if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind();
if let ty::RawPtr(TypeAndMut { ty: to_pointee_ty, mutbl: to_mutbl }) = cast_to.kind();
if matches!((from_mutbl, to_mutbl),
(Mutability::Not, Mutability::Not) | (Mutability::Mut, Mutability::Mut));
// The `U` in `pointer::cast` have to be `Sized`
// as explained here: https://github.com/rust-lang/rust/issues/60602.
if to_pointee_ty.is_sized(cx.tcx.at(expr.span), cx.param_env);
then {
let mut applicability = Applicability::MachineApplicable;
let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut applicability);
let turbofish = match &cast_to_hir_ty.kind {
TyKind::Infer => Cow::Borrowed(""),
TyKind::Ptr(mut_ty) if matches!(mut_ty.ty.kind, TyKind::Infer) => Cow::Borrowed(""),
_ => Cow::Owned(format!("::<{}>", to_pointee_ty)),
};
span_lint_and_sugg(
cx,
PTR_AS_PTR,
expr.span,
"`as` casting between raw pointers without changing its mutability",
"try `pointer::cast`, a safer alternative",
format!("{}.cast{}()", cast_expr_sugg.maybe_par(), turbofish),
applicability,
);
}
}
}
flip1995 marked this conversation as resolved.
Show resolved Hide resolved

extract_msrv_attr!(LateContext);
}
50 changes: 50 additions & 0 deletions tests/ui/ptr_as_ptr.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// run-rustfix

#![warn(clippy::ptr_as_ptr)]
#![feature(custom_inner_attributes)]

fn main() {
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
let ptr: *const u32 = &42_u32;
let mut_ptr: *mut u32 = &mut 42_u32;

let _ = ptr.cast::<i32>();
let _ = mut_ptr.cast::<i32>();

// Make sure the lint can handle the difference in their operator precedences.
unsafe {
let ptr_ptr: *const *const u32 = &ptr;
let _ = (*ptr_ptr).cast::<i32>();
}

// Changes in mutability. Do not lint this.
let _ = ptr as *mut i32;
let _ = mut_ptr as *const i32;

// `pointer::cast` cannot perform unsized coercions unlike `as`. Do not lint this.
let ptr_of_array: *const [u32; 4] = &[1, 2, 3, 4];
let _ = ptr_of_array as *const [u32];
let _ = ptr_of_array as *const dyn std::fmt::Debug;

// Ensure the lint doesn't produce unnecessary turbofish for inferred types.
let _: *const i32 = ptr.cast();
let _: *mut i32 = mut_ptr.cast();
}

fn _msrv_1_37() {
#![clippy::msrv = "1.37"]
let ptr: *const u32 = &42_u32;
let mut_ptr: *mut u32 = &mut 42_u32;

// `pointer::cast` was stabilized in 1.38. Do not lint this
let _ = ptr as *const i32;
let _ = mut_ptr as *mut i32;
}

fn _msrv_1_38() {
#![clippy::msrv = "1.38"]
let ptr: *const u32 = &42_u32;
let mut_ptr: *mut u32 = &mut 42_u32;

let _ = ptr.cast::<i32>();
let _ = mut_ptr.cast::<i32>();
}
50 changes: 50 additions & 0 deletions tests/ui/ptr_as_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// run-rustfix

#![warn(clippy::ptr_as_ptr)]
#![feature(custom_inner_attributes)]

fn main() {
let ptr: *const u32 = &42_u32;
let mut_ptr: *mut u32 = &mut 42_u32;

let _ = ptr as *const i32;
let _ = mut_ptr as *mut i32;

// Make sure the lint can handle the difference in their operator precedences.
unsafe {
let ptr_ptr: *const *const u32 = &ptr;
let _ = *ptr_ptr as *const i32;
}

// Changes in mutability. Do not lint this.
let _ = ptr as *mut i32;
let _ = mut_ptr as *const i32;

// `pointer::cast` cannot perform unsized coercions unlike `as`. Do not lint this.
let ptr_of_array: *const [u32; 4] = &[1, 2, 3, 4];
let _ = ptr_of_array as *const [u32];
let _ = ptr_of_array as *const dyn std::fmt::Debug;

// Ensure the lint doesn't produce unnecessary turbofish for inferred types.
let _: *const i32 = ptr as *const _;
let _: *mut i32 = mut_ptr as _;
}

fn _msrv_1_37() {
#![clippy::msrv = "1.37"]
let ptr: *const u32 = &42_u32;
let mut_ptr: *mut u32 = &mut 42_u32;

// `pointer::cast` was stabilized in 1.38. Do not lint this
let _ = ptr as *const i32;
let _ = mut_ptr as *mut i32;
}

fn _msrv_1_38() {
#![clippy::msrv = "1.38"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I didn't know that inner attributes can also be put into functions. I always assumed that they only work on crate and module level. But it makes sense, that they work for every item(-block)-like thing.

let ptr: *const u32 = &42_u32;
let mut_ptr: *mut u32 = &mut 42_u32;

let _ = ptr as *const i32;
let _ = mut_ptr as *mut i32;
}
46 changes: 46 additions & 0 deletions tests/ui/ptr_as_ptr.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
error: `as` casting between raw pointers without changing its mutability
--> $DIR/ptr_as_ptr.rs:10:13
|
LL | let _ = ptr as *const i32;
| ^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast::<i32>()`
|
= note: `-D clippy::ptr-as-ptr` implied by `-D warnings`

error: `as` casting between raw pointers without changing its mutability
--> $DIR/ptr_as_ptr.rs:11:13
|
LL | let _ = mut_ptr as *mut i32;
| ^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast::<i32>()`

error: `as` casting between raw pointers without changing its mutability
--> $DIR/ptr_as_ptr.rs:16:17
|
LL | let _ = *ptr_ptr as *const i32;
| ^^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `(*ptr_ptr).cast::<i32>()`

error: `as` casting between raw pointers without changing its mutability
--> $DIR/ptr_as_ptr.rs:29:25
|
LL | let _: *const i32 = ptr as *const _;
| ^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast()`

error: `as` casting between raw pointers without changing its mutability
--> $DIR/ptr_as_ptr.rs:30:23
|
LL | let _: *mut i32 = mut_ptr as _;
| ^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast()`

error: `as` casting between raw pointers without changing its mutability
--> $DIR/ptr_as_ptr.rs:48:13
|
LL | let _ = ptr as *const i32;
| ^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast::<i32>()`

error: `as` casting between raw pointers without changing its mutability
--> $DIR/ptr_as_ptr.rs:49:13
|
LL | let _ = mut_ptr as *mut i32;
| ^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast::<i32>()`

error: aborting due to 7 previous errors