From 4b478a5731956de7d19db9ac76fed81b2ae9db1c Mon Sep 17 00:00:00 2001 From: rail <12975677+rail-rain@users.noreply.github.com> Date: Mon, 4 Jan 2021 10:34:11 +1300 Subject: [PATCH 1/2] Add a new lint `ptr_as_ptr`, which checks for `as` casts between raw pointers without changing its mutability and suggest replacing it with `pointer::cast`. --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 3 ++ clippy_lints/src/types.rs | 99 ++++++++++++++++++++++++++++++++++++-- tests/ui/ptr_as_ptr.fixed | 30 ++++++++++++ tests/ui/ptr_as_ptr.rs | 30 ++++++++++++ tests/ui/ptr_as_ptr.stderr | 34 +++++++++++++ 6 files changed, 193 insertions(+), 4 deletions(-) create mode 100644 tests/ui/ptr_as_ptr.fixed create mode 100644 tests/ui/ptr_as_ptr.rs create mode 100644 tests/ui/ptr_as_ptr.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 38c943fec00..64864c2e278 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f57c6bd6324..37a56bc20c8 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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, @@ -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), @@ -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), diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index fd74783335d..d9cf26ad4b3 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -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; @@ -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! { @@ -2878,3 +2881,91 @@ 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::(); + /// let _ = mut_ptr.cast::(); + /// ``` + 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, +} + +impl PtrAsPtr { + #[must_use] + pub fn new(msrv: Option) -> 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, + ); + } + } + } +} diff --git a/tests/ui/ptr_as_ptr.fixed b/tests/ui/ptr_as_ptr.fixed new file mode 100644 index 00000000000..e0b79004cbd --- /dev/null +++ b/tests/ui/ptr_as_ptr.fixed @@ -0,0 +1,30 @@ +// run-rustfix + +#![warn(clippy::ptr_as_ptr)] + +fn main() { + let ptr: *const u32 = &42_u32; + let mut_ptr: *mut u32 = &mut 42_u32; + + let _ = ptr.cast::(); + let _ = mut_ptr.cast::(); + + // Make sure the lint can handle the difference in their operator precedences. + unsafe { + let ptr_ptr: *const *const u32 = &ptr; + let _ = (*ptr_ptr).cast::(); + } + + // 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(); +} diff --git a/tests/ui/ptr_as_ptr.rs b/tests/ui/ptr_as_ptr.rs new file mode 100644 index 00000000000..f31940dbd1f --- /dev/null +++ b/tests/ui/ptr_as_ptr.rs @@ -0,0 +1,30 @@ +// run-rustfix + +#![warn(clippy::ptr_as_ptr)] + +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 _; +} diff --git a/tests/ui/ptr_as_ptr.stderr b/tests/ui/ptr_as_ptr.stderr new file mode 100644 index 00000000000..22217e18c54 --- /dev/null +++ b/tests/ui/ptr_as_ptr.stderr @@ -0,0 +1,34 @@ +error: `as` casting between raw pointers without changing its mutability + --> $DIR/ptr_as_ptr.rs:9:13 + | +LL | let _ = ptr as *const i32; + | ^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast::()` + | + = 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:10:13 + | +LL | let _ = mut_ptr as *mut i32; + | ^^^^^^^^^^^^^^^^^^^ 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:15:17 + | +LL | let _ = *ptr_ptr as *const i32; + | ^^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `(*ptr_ptr).cast::()` + +error: `as` casting between raw pointers without changing its mutability + --> $DIR/ptr_as_ptr.rs:28: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:29:23 + | +LL | let _: *mut i32 = mut_ptr as _; + | ^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `mut_ptr.cast()` + +error: aborting due to 5 previous errors + From dfa5d7e818c32ea48bc141799276f700ab9d8fb7 Mon Sep 17 00:00:00 2001 From: rail <12975677+rail-rain@users.noreply.github.com> Date: Tue, 5 Jan 2021 10:17:31 +1300 Subject: [PATCH 2/2] Fix the MSRV and add the tests for MSRV --- clippy_lints/src/types.rs | 2 ++ tests/ui/ptr_as_ptr.fixed | 20 ++++++++++++++++++++ tests/ui/ptr_as_ptr.rs | 20 ++++++++++++++++++++ tests/ui/ptr_as_ptr.stderr | 24 ++++++++++++++++++------ 4 files changed, 60 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index d9cf26ad4b3..b21f81bd517 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -2968,4 +2968,6 @@ impl<'tcx> LateLintPass<'tcx> for PtrAsPtr { } } } + + extract_msrv_attr!(LateContext); } diff --git a/tests/ui/ptr_as_ptr.fixed b/tests/ui/ptr_as_ptr.fixed index e0b79004cbd..8346a9454f4 100644 --- a/tests/ui/ptr_as_ptr.fixed +++ b/tests/ui/ptr_as_ptr.fixed @@ -1,6 +1,7 @@ // run-rustfix #![warn(clippy::ptr_as_ptr)] +#![feature(custom_inner_attributes)] fn main() { let ptr: *const u32 = &42_u32; @@ -28,3 +29,22 @@ fn main() { 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::(); + let _ = mut_ptr.cast::(); +} diff --git a/tests/ui/ptr_as_ptr.rs b/tests/ui/ptr_as_ptr.rs index f31940dbd1f..b68d4bc0aac 100644 --- a/tests/ui/ptr_as_ptr.rs +++ b/tests/ui/ptr_as_ptr.rs @@ -1,6 +1,7 @@ // run-rustfix #![warn(clippy::ptr_as_ptr)] +#![feature(custom_inner_attributes)] fn main() { let ptr: *const u32 = &42_u32; @@ -28,3 +29,22 @@ fn main() { 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"] + 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; +} diff --git a/tests/ui/ptr_as_ptr.stderr b/tests/ui/ptr_as_ptr.stderr index 22217e18c54..854906dc111 100644 --- a/tests/ui/ptr_as_ptr.stderr +++ b/tests/ui/ptr_as_ptr.stderr @@ -1,5 +1,5 @@ error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:9:13 + --> $DIR/ptr_as_ptr.rs:10:13 | LL | let _ = ptr as *const i32; | ^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `ptr.cast::()` @@ -7,28 +7,40 @@ LL | let _ = ptr as *const 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:10:13 + --> $DIR/ptr_as_ptr.rs:11:13 | LL | let _ = mut_ptr as *mut i32; | ^^^^^^^^^^^^^^^^^^^ 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:15:17 + --> $DIR/ptr_as_ptr.rs:16:17 | LL | let _ = *ptr_ptr as *const i32; | ^^^^^^^^^^^^^^^^^^^^^^ help: try `pointer::cast`, a safer alternative: `(*ptr_ptr).cast::()` error: `as` casting between raw pointers without changing its mutability - --> $DIR/ptr_as_ptr.rs:28:25 + --> $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:29:23 + --> $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: aborting due to 5 previous errors +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::()` + +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::()` + +error: aborting due to 7 previous errors