From 631b4ee7a26e4e68469a607376005eb4064b3e6f Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Tue, 10 May 2022 14:21:51 -0400 Subject: [PATCH 1/3] Fix redundant_allocation warning for Rc> --- .../src/types/redundant_allocation.rs | 22 ++++++++++++++++--- tests/ui/redundant_allocation.rs | 18 +++++++++++++++ tests/ui/redundant_allocation.stderr | 11 +++++++++- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/types/redundant_allocation.rs b/clippy_lints/src/types/redundant_allocation.rs index 10d2ae2eb1d..5fdf1731495 100644 --- a/clippy_lints/src/types/redundant_allocation.rs +++ b/clippy_lints/src/types/redundant_allocation.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::{path_def_id, qpath_generic_tys}; use rustc_errors::Applicability; -use rustc_hir::{self as hir, def_id::DefId, QPath, TyKind}; +use rustc_hir::{self as hir, def, def_id::DefId, PrimTy, QPath, TyKind}; use rustc_lint::LateContext; use rustc_span::symbol::sym; @@ -54,8 +54,7 @@ pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_ }; let inner_span = match qpath_generic_tys(inner_qpath).next() { Some(ty) => { - // Box> is smaller than Box because of wide pointers - if matches!(ty.kind, TyKind::TraitObject(..)) { + if alloc_makes_pointer_thin(cx, ty) { return false; } ty.span @@ -110,3 +109,20 @@ pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_ } true } + +/// Returns `true` if the allocation would make `hir_ty` go from fat to thin. +fn alloc_makes_pointer_thin(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>) -> bool { + match &hir_ty.kind { + TyKind::TraitObject(..) => true, + TyKind::Path(ty_qpath) => { + let ty_res = cx.qpath_res(ty_qpath, hir_ty.hir_id); + if let def::Res::PrimTy(prim_ty) = ty_res { + if matches!(prim_ty, PrimTy::Str) { + return true; + } + } + false + }, + _ => false, + } +} diff --git a/tests/ui/redundant_allocation.rs b/tests/ui/redundant_allocation.rs index 80f94e5f3cb..b830a3771d5 100644 --- a/tests/ui/redundant_allocation.rs +++ b/tests/ui/redundant_allocation.rs @@ -97,4 +97,22 @@ mod box_dyn { pub fn test_rc_box(_: Rc>>) {} } +// https://github.com/rust-lang/rust-clippy/issues/8604 +mod box_str { + use std::boxed::Box; + use std::rc::Rc; + use std::sync::Arc; + + struct S { + a: Box>, + b: Rc>, + c: Arc>, + } + + pub fn test_box(_: Box>) {} + pub fn test_rc(_: Rc>) {} + pub fn test_arc(_: Arc>) {} + pub fn test_rc_box(_: Rc>>) {} +} + fn main() {} diff --git a/tests/ui/redundant_allocation.stderr b/tests/ui/redundant_allocation.stderr index c3b10e5f5e6..ae213cb8975 100644 --- a/tests/ui/redundant_allocation.stderr +++ b/tests/ui/redundant_allocation.stderr @@ -143,5 +143,14 @@ LL | pub fn test_rc_box(_: Rc>>) {} = note: `Box>` is already on the heap, `Rc>>` makes an extra allocation = help: consider using just `Rc>` or `Box>` -error: aborting due to 16 previous errors +error: usage of `Rc>>` + --> $DIR/redundant_allocation.rs:115:27 + | +LL | pub fn test_rc_box(_: Rc>>) {} + | ^^^^^^^^^^^^^^^^^ + | + = note: `Box>` is already on the heap, `Rc>>` makes an extra allocation + = help: consider using just `Rc>` or `Box>` + +error: aborting due to 17 previous errors From 0def44a1c15e624a6e51206048895021f5b1ee92 Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Thu, 12 May 2022 19:06:23 -0400 Subject: [PATCH 2/3] Catch other thinning allocations using Ty --- .../src/types/redundant_allocation.rs | 23 +++--------- tests/ui/redundant_allocation.rs | 27 +++++++++++--- tests/ui/redundant_allocation.stderr | 35 ++++++++++++++++--- 3 files changed, 57 insertions(+), 28 deletions(-) diff --git a/clippy_lints/src/types/redundant_allocation.rs b/clippy_lints/src/types/redundant_allocation.rs index 5fdf1731495..6c231fa2510 100644 --- a/clippy_lints/src/types/redundant_allocation.rs +++ b/clippy_lints/src/types/redundant_allocation.rs @@ -2,9 +2,10 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::{path_def_id, qpath_generic_tys}; use rustc_errors::Applicability; -use rustc_hir::{self as hir, def, def_id::DefId, PrimTy, QPath, TyKind}; +use rustc_hir::{self as hir, def_id::DefId, QPath, TyKind}; use rustc_lint::LateContext; use rustc_span::symbol::sym; +use rustc_typeck::hir_ty_to_ty; use super::{utils, REDUNDANT_ALLOCATION}; @@ -54,7 +55,8 @@ pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_ }; let inner_span = match qpath_generic_tys(inner_qpath).next() { Some(ty) => { - if alloc_makes_pointer_thin(cx, ty) { + // Reallocation of a fat pointer causes it to become thin + if !hir_ty_to_ty(cx.tcx, ty).is_sized(cx.tcx.at(ty.span), cx.param_env) { return false; } ty.span @@ -109,20 +111,3 @@ pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_ } true } - -/// Returns `true` if the allocation would make `hir_ty` go from fat to thin. -fn alloc_makes_pointer_thin(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>) -> bool { - match &hir_ty.kind { - TyKind::TraitObject(..) => true, - TyKind::Path(ty_qpath) => { - let ty_res = cx.qpath_res(ty_qpath, hir_ty.hir_id); - if let def::Res::PrimTy(prim_ty) = ty_res { - if matches!(prim_ty, PrimTy::Str) { - return true; - } - } - false - }, - _ => false, - } -} diff --git a/tests/ui/redundant_allocation.rs b/tests/ui/redundant_allocation.rs index b830a3771d5..cf7d8c6e349 100644 --- a/tests/ui/redundant_allocation.rs +++ b/tests/ui/redundant_allocation.rs @@ -98,21 +98,38 @@ mod box_dyn { } // https://github.com/rust-lang/rust-clippy/issues/8604 -mod box_str { +mod box_fat_ptr { use std::boxed::Box; + use std::path::Path; use std::rc::Rc; use std::sync::Arc; + pub struct DynSized { + foo: [usize], + } + struct S { a: Box>, b: Rc>, c: Arc>, + + e: Box>, + f: Box>, + g: Box>, } - pub fn test_box(_: Box>) {} - pub fn test_rc(_: Rc>) {} - pub fn test_arc(_: Arc>) {} - pub fn test_rc_box(_: Rc>>) {} + pub fn test_box_str(_: Box>) {} + pub fn test_rc_str(_: Rc>) {} + pub fn test_arc_str(_: Arc>) {} + + pub fn test_box_slice(_: Box>) {} + pub fn test_box_path(_: Box>) {} + pub fn test_box_custom(_: Box>) {} + + pub fn test_rc_box_str(_: Rc>>) {} + pub fn test_rc_box_slice(_: Rc>>) {} + pub fn test_rc_box_path(_: Rc>>) {} + pub fn test_rc_box_custom(_: Rc>>) {} } fn main() {} diff --git a/tests/ui/redundant_allocation.stderr b/tests/ui/redundant_allocation.stderr index ae213cb8975..fab1b069fcb 100644 --- a/tests/ui/redundant_allocation.stderr +++ b/tests/ui/redundant_allocation.stderr @@ -144,13 +144,40 @@ LL | pub fn test_rc_box(_: Rc>>) {} = help: consider using just `Rc>` or `Box>` error: usage of `Rc>>` - --> $DIR/redundant_allocation.rs:115:27 + --> $DIR/redundant_allocation.rs:129:31 | -LL | pub fn test_rc_box(_: Rc>>) {} - | ^^^^^^^^^^^^^^^^^ +LL | pub fn test_rc_box_str(_: Rc>>) {} + | ^^^^^^^^^^^^^^^^^ | = note: `Box>` is already on the heap, `Rc>>` makes an extra allocation = help: consider using just `Rc>` or `Box>` -error: aborting due to 17 previous errors +error: usage of `Rc>>` + --> $DIR/redundant_allocation.rs:130:33 + | +LL | pub fn test_rc_box_slice(_: Rc>>) {} + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: `Box>` is already on the heap, `Rc>>` makes an extra allocation + = help: consider using just `Rc>` or `Box>` + +error: usage of `Rc>>` + --> $DIR/redundant_allocation.rs:131:32 + | +LL | pub fn test_rc_box_path(_: Rc>>) {} + | ^^^^^^^^^^^^^^^^^^ + | + = note: `Box>` is already on the heap, `Rc>>` makes an extra allocation + = help: consider using just `Rc>` or `Box>` + +error: usage of `Rc>>` + --> $DIR/redundant_allocation.rs:132:34 + | +LL | pub fn test_rc_box_custom(_: Rc>>) {} + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `Box>` is already on the heap, `Rc>>` makes an extra allocation + = help: consider using just `Rc>` or `Box>` + +error: aborting due to 20 previous errors From ad7338e5659f5711e08502906147e22b44e41a3e Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Fri, 13 May 2022 05:55:50 -0400 Subject: [PATCH 3/3] Comment why `hir_ty_to_ty` is safe in `check_ty` --- clippy_lints/src/types/redundant_allocation.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/types/redundant_allocation.rs b/clippy_lints/src/types/redundant_allocation.rs index 6c231fa2510..a1312fcda0b 100644 --- a/clippy_lints/src/types/redundant_allocation.rs +++ b/clippy_lints/src/types/redundant_allocation.rs @@ -55,7 +55,9 @@ pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_ }; let inner_span = match qpath_generic_tys(inner_qpath).next() { Some(ty) => { - // Reallocation of a fat pointer causes it to become thin + // Reallocation of a fat pointer causes it to become thin. `hir_ty_to_ty` is safe to use + // here because `mod.rs` guarantees this lint is only run on types outside of bodies and + // is not run on locals. if !hir_ty_to_ty(cx.tcx, ty).is_sized(cx.tcx.at(ty.span), cx.param_env) { return false; }