From 73b9841a3e39ed84597ab9548616b46c80604116 Mon Sep 17 00:00:00 2001 From: Jacherr Date: Wed, 8 Nov 2023 17:41:28 +0000 Subject: [PATCH 1/8] remove unnecessary `find_map` calls --- clippy_lints/src/types/vec_box.rs | 63 +++++++++++++------------------ 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/clippy_lints/src/types/vec_box.rs b/clippy_lints/src/types/vec_box.rs index decc183ad96..89b5bd11f9f 100644 --- a/clippy_lints/src/types/vec_box.rs +++ b/clippy_lints/src/types/vec_box.rs @@ -1,7 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::last_path_segment; use clippy_utils::source::snippet; -use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::def_id::DefId; use rustc_hir::{self as hir, GenericArg, QPath, TyKind}; @@ -21,45 +20,37 @@ pub(super) fn check( box_size_threshold: u64, ) -> bool { if cx.tcx.is_diagnostic_item(sym::Vec, def_id) { - if_chain! { + if let Some(last) = last_path_segment(qpath).args // Get the _ part of Vec<_> - if let Some(last) = last_path_segment(qpath).args; - if let Some(ty) = last.args.iter().find_map(|arg| match arg { - GenericArg::Type(ty) => Some(ty), - _ => None, - }); + && let Some(GenericArg::Type(ty)) = last.args.first() // ty is now _ at this point - if let TyKind::Path(ref ty_qpath) = ty.kind; - let res = cx.qpath_res(ty_qpath, ty.hir_id); - if let Some(def_id) = res.opt_def_id(); - if Some(def_id) == cx.tcx.lang_items().owned_box(); + && let TyKind::Path(ref ty_qpath) = ty.kind + && let res = cx.qpath_res(ty_qpath, ty.hir_id) + && let Some(def_id) = res.opt_def_id() + && Some(def_id) == cx.tcx.lang_items().owned_box() // At this point, we know ty is Box, now get T - if let Some(last) = last_path_segment(ty_qpath).args; - if let Some(boxed_ty) = last.args.iter().find_map(|arg| match arg { - GenericArg::Type(ty) => Some(ty), - _ => None, - }); - let ty_ty = hir_ty_to_ty(cx.tcx, boxed_ty); - if !ty_ty.has_escaping_bound_vars(); - if ty_ty.is_sized(cx.tcx, cx.param_env); - if let Ok(ty_ty_size) = cx.layout_of(ty_ty).map(|l| l.size.bytes()); - if ty_ty_size < box_size_threshold; - then { - span_lint_and_sugg( - cx, - VEC_BOX, - hir_ty.span, - "`Vec` is already on the heap, the boxing is unnecessary", - "try", - format!("Vec<{}>", snippet(cx, boxed_ty.span, "..")), - Applicability::MachineApplicable, - ); - true - } else { - false - } + && let Some(last) = last_path_segment(ty_qpath).args + && let Some(GenericArg::Type(boxed_ty)) = last.args.first() + && let ty_ty = hir_ty_to_ty(cx.tcx, boxed_ty) + && !ty_ty.has_escaping_bound_vars() + && ty_ty.is_sized(cx.tcx, cx.param_env) + && let Ok(ty_ty_size) = cx.layout_of(ty_ty).map(|l| l.size.bytes()) + && ty_ty_size < box_size_threshold + { + span_lint_and_sugg( + cx, + VEC_BOX, + hir_ty.span, + "`Vec` is already on the heap, the boxing is unnecessary", + "try", + format!("Vec<{}>", snippet(cx, boxed_ty.span, "..")), + Applicability::MachineApplicable, + ); + true + } else { + false } } else { false } -} +} \ No newline at end of file From 3a91a1174094b7b5da241076d26026af873408be Mon Sep 17 00:00:00 2001 From: Jacherr Date: Wed, 8 Nov 2023 18:27:33 +0000 Subject: [PATCH 2/8] add logic to check allocator matching --- clippy_lints/src/types/vec_box.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/types/vec_box.rs b/clippy_lints/src/types/vec_box.rs index 89b5bd11f9f..25e5b886681 100644 --- a/clippy_lints/src/types/vec_box.rs +++ b/clippy_lints/src/types/vec_box.rs @@ -23,6 +23,8 @@ pub(super) fn check( if let Some(last) = last_path_segment(qpath).args // Get the _ part of Vec<_> && let Some(GenericArg::Type(ty)) = last.args.first() + // extract allocator from the Vec for later + && let vec_alloc_ty = last.args.get(1) // ty is now _ at this point && let TyKind::Path(ref ty_qpath) = ty.kind && let res = cx.qpath_res(ty_qpath, ty.hir_id) @@ -30,12 +32,23 @@ pub(super) fn check( && Some(def_id) == cx.tcx.lang_items().owned_box() // At this point, we know ty is Box, now get T && let Some(last) = last_path_segment(ty_qpath).args + // extract allocator from thr Box for later && let Some(GenericArg::Type(boxed_ty)) = last.args.first() + && let boxed_alloc_ty = last.args.get(1) && let ty_ty = hir_ty_to_ty(cx.tcx, boxed_ty) && !ty_ty.has_escaping_bound_vars() && ty_ty.is_sized(cx.tcx, cx.param_env) && let Ok(ty_ty_size) = cx.layout_of(ty_ty).map(|l| l.size.bytes()) && ty_ty_size < box_size_threshold + // https://github.com/rust-lang/rust-clippy/issues/7114 + // this code also does not consider that Global can be explicitly + // defined (yet) as in Vec<_, Global> so a very slight false negative edge case + && match (vec_alloc_ty, boxed_alloc_ty) { + (None, None) => true, + (Some(GenericArg::Type(l)), Some(GenericArg::Type(r))) => + hir_ty_to_ty(cx.tcx, l) == hir_ty_to_ty(cx.tcx, r), + _ => false + } { span_lint_and_sugg( cx, @@ -53,4 +66,4 @@ pub(super) fn check( } else { false } -} \ No newline at end of file +} From 79325604da4b0000b7bd6c6fb6553f108a5983d7 Mon Sep 17 00:00:00 2001 From: Jacherr Date: Wed, 8 Nov 2023 18:42:58 +0000 Subject: [PATCH 3/8] update testcases, cleanup --- clippy_lints/src/types/vec_box.rs | 2 +- tests/ui/vec_box_sized.fixed | 17 +++++++++++++++++ tests/ui/vec_box_sized.rs | 17 +++++++++++++++++ tests/ui/vec_box_sized.stderr | 12 ++++++------ 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/types/vec_box.rs b/clippy_lints/src/types/vec_box.rs index 25e5b886681..887439560d1 100644 --- a/clippy_lints/src/types/vec_box.rs +++ b/clippy_lints/src/types/vec_box.rs @@ -32,8 +32,8 @@ pub(super) fn check( && Some(def_id) == cx.tcx.lang_items().owned_box() // At this point, we know ty is Box, now get T && let Some(last) = last_path_segment(ty_qpath).args - // extract allocator from thr Box for later && let Some(GenericArg::Type(boxed_ty)) = last.args.first() + // extract allocator from the Box for later && let boxed_alloc_ty = last.args.get(1) && let ty_ty = hir_ty_to_ty(cx.tcx, boxed_ty) && !ty_ty.has_escaping_bound_vars() diff --git a/tests/ui/vec_box_sized.fixed b/tests/ui/vec_box_sized.fixed index 4363d2224af..67067de91cc 100644 --- a/tests/ui/vec_box_sized.fixed +++ b/tests/ui/vec_box_sized.fixed @@ -1,4 +1,5 @@ #![allow(dead_code)] +#![feature(allocator_api)] struct SizedStruct(i32); struct UnsizedStruct([i32]); @@ -21,6 +22,8 @@ mod should_trigger { /// The following should not trigger the lint mod should_not_trigger { use super::{BigStruct, UnsizedStruct}; + use std::alloc::{Layout, AllocError, Allocator}; + use std::ptr::NonNull; struct C(Vec>); struct D(Vec>); @@ -33,6 +36,20 @@ mod should_not_trigger { // Regression test for #3720. This was causing an ICE. inner: Vec>, } + + struct DummyAllocator; + unsafe impl Allocator for DummyAllocator { + fn allocate(&self, layout: Layout) -> Result, AllocError> { + todo!() + } + unsafe fn deallocate(&self, ptr: NonNull, layout: Layout) { + todo!() + } + } + + fn allocator_mismatch() -> Vec> { + vec![] + } } mod inner_mod { diff --git a/tests/ui/vec_box_sized.rs b/tests/ui/vec_box_sized.rs index f4e27fe4bd5..2b5c2c99945 100644 --- a/tests/ui/vec_box_sized.rs +++ b/tests/ui/vec_box_sized.rs @@ -1,4 +1,5 @@ #![allow(dead_code)] +#![feature(allocator_api)] struct SizedStruct(i32); struct UnsizedStruct([i32]); @@ -21,6 +22,8 @@ mod should_trigger { /// The following should not trigger the lint mod should_not_trigger { use super::{BigStruct, UnsizedStruct}; + use std::alloc::{Layout, AllocError, Allocator}; + use std::ptr::NonNull; struct C(Vec>); struct D(Vec>); @@ -33,6 +36,20 @@ mod should_not_trigger { // Regression test for #3720. This was causing an ICE. inner: Vec>, } + + struct DummyAllocator; + unsafe impl Allocator for DummyAllocator { + fn allocate(&self, layout: Layout) -> Result, AllocError> { + todo!() + } + unsafe fn deallocate(&self, ptr: NonNull, layout: Layout) { + todo!() + } + } + + fn allocator_mismatch() -> Vec> { + vec![] + } } mod inner_mod { diff --git a/tests/ui/vec_box_sized.stderr b/tests/ui/vec_box_sized.stderr index 9118f284bb9..0e68aa57dcb 100644 --- a/tests/ui/vec_box_sized.stderr +++ b/tests/ui/vec_box_sized.stderr @@ -1,5 +1,5 @@ error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:10:14 + --> $DIR/vec_box_sized.rs:11:14 | LL | const C: Vec> = Vec::new(); | ^^^^^^^^^^^^^ help: try: `Vec` @@ -8,31 +8,31 @@ LL | const C: Vec> = Vec::new(); = help: to override `-D warnings` add `#[allow(clippy::vec_box)]` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:11:15 + --> $DIR/vec_box_sized.rs:12:15 | LL | static S: Vec> = Vec::new(); | ^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:14:21 + --> $DIR/vec_box_sized.rs:15:21 | LL | sized_type: Vec>, | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:17:14 + --> $DIR/vec_box_sized.rs:18:14 | LL | struct A(Vec>); | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:18:18 + --> $DIR/vec_box_sized.rs:19:18 | LL | struct B(Vec>>); | ^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:46:23 + --> $DIR/vec_box_sized.rs:63:23 | LL | pub fn f() -> Vec> { | ^^^^^^^^^^^ help: try: `Vec` From 67bb503f26ad5d695229153995bd2ab296e590cb Mon Sep 17 00:00:00 2001 From: Jacherr Date: Wed, 8 Nov 2023 21:10:27 +0000 Subject: [PATCH 4/8] add support for std::alloc::Global, add more tests --- clippy_lints/src/types/vec_box.rs | 18 +++++++++++--- clippy_utils/src/paths.rs | 1 + tests/ui/vec_box_sized.fixed | 41 ++++++++++++++++++++----------- tests/ui/vec_box_sized.rs | 41 ++++++++++++++++++++----------- tests/ui/vec_box_sized.stderr | 32 ++++++++++++++++++------ 5 files changed, 92 insertions(+), 41 deletions(-) diff --git a/clippy_lints/src/types/vec_box.rs b/clippy_lints/src/types/vec_box.rs index 887439560d1..43f72f1031d 100644 --- a/clippy_lints/src/types/vec_box.rs +++ b/clippy_lints/src/types/vec_box.rs @@ -1,5 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::last_path_segment; +use clippy_utils::{last_path_segment, match_def_path}; +use clippy_utils::paths::ALLOCATOR_GLOBAL; use clippy_utils::source::snippet; use rustc_errors::Applicability; use rustc_hir::def_id::DefId; @@ -41,10 +42,19 @@ pub(super) fn check( && let Ok(ty_ty_size) = cx.layout_of(ty_ty).map(|l| l.size.bytes()) && ty_ty_size < box_size_threshold // https://github.com/rust-lang/rust-clippy/issues/7114 - // this code also does not consider that Global can be explicitly - // defined (yet) as in Vec<_, Global> so a very slight false negative edge case && match (vec_alloc_ty, boxed_alloc_ty) { (None, None) => true, + // this is in the event that we have something like + // Vec<_, Global>, in which case is equivalent to + // Vec<_> + (None, Some(GenericArg::Type(inner))) | (Some(GenericArg::Type(inner)), None) => { + if let TyKind::Path(path) = inner.kind + && let Some(did) = cx.qpath_res(&path, inner.hir_id).opt_def_id() { + match_def_path(cx, did, &ALLOCATOR_GLOBAL) + } else { + true + } + }, (Some(GenericArg::Type(l)), Some(GenericArg::Type(r))) => hir_ty_to_ty(cx.tcx, l) == hir_ty_to_ty(cx.tcx, r), _ => false @@ -57,7 +67,7 @@ pub(super) fn check( "`Vec` is already on the heap, the boxing is unnecessary", "try", format!("Vec<{}>", snippet(cx, boxed_ty.span, "..")), - Applicability::MachineApplicable, + Applicability::Unspecified, ); true } else { diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 5bca554378e..bde8eb892e9 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -99,3 +99,4 @@ pub const OPTION_UNWRAP: [&str; 4] = ["core", "option", "Option", "unwrap"]; pub const OPTION_EXPECT: [&str; 4] = ["core", "option", "Option", "expect"]; #[expect(clippy::invalid_paths)] // not sure why it thinks this, it works so pub const BOOL_THEN: [&str; 4] = ["core", "bool", "", "then"]; +pub const ALLOCATOR_GLOBAL: [&str; 3] = ["alloc", "alloc", "Global"]; \ No newline at end of file diff --git a/tests/ui/vec_box_sized.fixed b/tests/ui/vec_box_sized.fixed index 67067de91cc..0be891a92bc 100644 --- a/tests/ui/vec_box_sized.fixed +++ b/tests/ui/vec_box_sized.fixed @@ -1,13 +1,26 @@ #![allow(dead_code)] #![feature(allocator_api)] +use std::alloc::{Layout, AllocError, Allocator}; +use std::ptr::NonNull; + struct SizedStruct(i32); struct UnsizedStruct([i32]); struct BigStruct([i32; 10000]); +struct DummyAllocator; +unsafe impl Allocator for DummyAllocator { + fn allocate(&self, layout: Layout) -> Result, AllocError> { + todo!() + } + unsafe fn deallocate(&self, ptr: NonNull, layout: Layout) { + todo!() + } +} + /// The following should trigger the lint mod should_trigger { - use super::SizedStruct; + use super::{SizedStruct, DummyAllocator}; const C: Vec = Vec::new(); static S: Vec = Vec::new(); @@ -17,13 +30,21 @@ mod should_trigger { struct A(Vec); struct B(Vec>); + + fn allocator_global_defined_vec() -> Vec { + Vec::new() + } + fn allocator_global_defined_box() -> Vec { + Vec::new() + } + fn allocator_match() -> Vec { + Vec::new_in(DummyAllocator) + } } /// The following should not trigger the lint mod should_not_trigger { - use super::{BigStruct, UnsizedStruct}; - use std::alloc::{Layout, AllocError, Allocator}; - use std::ptr::NonNull; + use super::{BigStruct, UnsizedStruct, DummyAllocator}; struct C(Vec>); struct D(Vec>); @@ -37,18 +58,8 @@ mod should_not_trigger { inner: Vec>, } - struct DummyAllocator; - unsafe impl Allocator for DummyAllocator { - fn allocate(&self, layout: Layout) -> Result, AllocError> { - todo!() - } - unsafe fn deallocate(&self, ptr: NonNull, layout: Layout) { - todo!() - } - } - fn allocator_mismatch() -> Vec> { - vec![] + Vec::new() } } diff --git a/tests/ui/vec_box_sized.rs b/tests/ui/vec_box_sized.rs index 2b5c2c99945..9314195b103 100644 --- a/tests/ui/vec_box_sized.rs +++ b/tests/ui/vec_box_sized.rs @@ -1,13 +1,26 @@ #![allow(dead_code)] #![feature(allocator_api)] +use std::alloc::{Layout, AllocError, Allocator}; +use std::ptr::NonNull; + struct SizedStruct(i32); struct UnsizedStruct([i32]); struct BigStruct([i32; 10000]); +struct DummyAllocator; +unsafe impl Allocator for DummyAllocator { + fn allocate(&self, layout: Layout) -> Result, AllocError> { + todo!() + } + unsafe fn deallocate(&self, ptr: NonNull, layout: Layout) { + todo!() + } +} + /// The following should trigger the lint mod should_trigger { - use super::SizedStruct; + use super::{SizedStruct, DummyAllocator}; const C: Vec> = Vec::new(); static S: Vec> = Vec::new(); @@ -17,13 +30,21 @@ mod should_trigger { struct A(Vec>); struct B(Vec>>); + + fn allocator_global_defined_vec() -> Vec, std::alloc::Global> { + Vec::new() + } + fn allocator_global_defined_box() -> Vec> { + Vec::new() + } + fn allocator_match() -> Vec, DummyAllocator> { + Vec::new_in(DummyAllocator) + } } /// The following should not trigger the lint mod should_not_trigger { - use super::{BigStruct, UnsizedStruct}; - use std::alloc::{Layout, AllocError, Allocator}; - use std::ptr::NonNull; + use super::{BigStruct, UnsizedStruct, DummyAllocator}; struct C(Vec>); struct D(Vec>); @@ -37,18 +58,8 @@ mod should_not_trigger { inner: Vec>, } - struct DummyAllocator; - unsafe impl Allocator for DummyAllocator { - fn allocate(&self, layout: Layout) -> Result, AllocError> { - todo!() - } - unsafe fn deallocate(&self, ptr: NonNull, layout: Layout) { - todo!() - } - } - fn allocator_mismatch() -> Vec> { - vec![] + Vec::new() } } diff --git a/tests/ui/vec_box_sized.stderr b/tests/ui/vec_box_sized.stderr index 0e68aa57dcb..2fa5679650b 100644 --- a/tests/ui/vec_box_sized.stderr +++ b/tests/ui/vec_box_sized.stderr @@ -1,5 +1,5 @@ error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:11:14 + --> $DIR/vec_box_sized.rs:24:14 | LL | const C: Vec> = Vec::new(); | ^^^^^^^^^^^^^ help: try: `Vec` @@ -8,34 +8,52 @@ LL | const C: Vec> = Vec::new(); = help: to override `-D warnings` add `#[allow(clippy::vec_box)]` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:12:15 + --> $DIR/vec_box_sized.rs:25:15 | LL | static S: Vec> = Vec::new(); | ^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:15:21 + --> $DIR/vec_box_sized.rs:28:21 | LL | sized_type: Vec>, | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:18:14 + --> $DIR/vec_box_sized.rs:31:14 | LL | struct A(Vec>); | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:19:18 + --> $DIR/vec_box_sized.rs:32:18 | LL | struct B(Vec>>); | ^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:63:23 + --> $DIR/vec_box_sized.rs:34:42 + | +LL | fn allocator_global_defined_vec() -> Vec, std::alloc::Global> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` + +error: `Vec` is already on the heap, the boxing is unnecessary + --> $DIR/vec_box_sized.rs:37:42 + | +LL | fn allocator_global_defined_box() -> Vec> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` + +error: `Vec` is already on the heap, the boxing is unnecessary + --> $DIR/vec_box_sized.rs:40:29 + | +LL | fn allocator_match() -> Vec, DummyAllocator> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` + +error: `Vec` is already on the heap, the boxing is unnecessary + --> $DIR/vec_box_sized.rs:74:23 | LL | pub fn f() -> Vec> { | ^^^^^^^^^^^ help: try: `Vec` -error: aborting due to 6 previous errors +error: aborting due to 9 previous errors From b6d56c47f9d62a456fc2affaad71f9120c1a8059 Mon Sep 17 00:00:00 2001 From: Jacherr Date: Wed, 8 Nov 2023 21:15:11 +0000 Subject: [PATCH 5/8] add no-rustfix since suggestions are invalid --- tests/ui/vec_box_sized.fixed | 85 ----------------------------------- tests/ui/vec_box_sized.rs | 2 + tests/ui/vec_box_sized.stderr | 18 ++++---- 3 files changed, 11 insertions(+), 94 deletions(-) delete mode 100644 tests/ui/vec_box_sized.fixed diff --git a/tests/ui/vec_box_sized.fixed b/tests/ui/vec_box_sized.fixed deleted file mode 100644 index 0be891a92bc..00000000000 --- a/tests/ui/vec_box_sized.fixed +++ /dev/null @@ -1,85 +0,0 @@ -#![allow(dead_code)] -#![feature(allocator_api)] - -use std::alloc::{Layout, AllocError, Allocator}; -use std::ptr::NonNull; - -struct SizedStruct(i32); -struct UnsizedStruct([i32]); -struct BigStruct([i32; 10000]); - -struct DummyAllocator; -unsafe impl Allocator for DummyAllocator { - fn allocate(&self, layout: Layout) -> Result, AllocError> { - todo!() - } - unsafe fn deallocate(&self, ptr: NonNull, layout: Layout) { - todo!() - } -} - -/// The following should trigger the lint -mod should_trigger { - use super::{SizedStruct, DummyAllocator}; - const C: Vec = Vec::new(); - static S: Vec = Vec::new(); - - struct StructWithVecBox { - sized_type: Vec, - } - - struct A(Vec); - struct B(Vec>); - - fn allocator_global_defined_vec() -> Vec { - Vec::new() - } - fn allocator_global_defined_box() -> Vec { - Vec::new() - } - fn allocator_match() -> Vec { - Vec::new_in(DummyAllocator) - } -} - -/// The following should not trigger the lint -mod should_not_trigger { - use super::{BigStruct, UnsizedStruct, DummyAllocator}; - - struct C(Vec>); - struct D(Vec>); - - struct StructWithVecBoxButItsUnsized { - unsized_type: Vec>, - } - - struct TraitVec { - // Regression test for #3720. This was causing an ICE. - inner: Vec>, - } - - fn allocator_mismatch() -> Vec> { - Vec::new() - } -} - -mod inner_mod { - mod inner { - pub struct S; - } - - mod inner2 { - use super::inner::S; - - pub fn f() -> Vec { - vec![] - } - } -} - -// https://github.com/rust-lang/rust-clippy/issues/11417 -fn in_closure() { - let _ = |_: Vec>| {}; -} - -fn main() {} diff --git a/tests/ui/vec_box_sized.rs b/tests/ui/vec_box_sized.rs index 9314195b103..af471f1eb8d 100644 --- a/tests/ui/vec_box_sized.rs +++ b/tests/ui/vec_box_sized.rs @@ -1,3 +1,5 @@ +//@no-rustfix + #![allow(dead_code)] #![feature(allocator_api)] diff --git a/tests/ui/vec_box_sized.stderr b/tests/ui/vec_box_sized.stderr index 2fa5679650b..db5674210d9 100644 --- a/tests/ui/vec_box_sized.stderr +++ b/tests/ui/vec_box_sized.stderr @@ -1,5 +1,5 @@ error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:24:14 + --> $DIR/vec_box_sized.rs:26:14 | LL | const C: Vec> = Vec::new(); | ^^^^^^^^^^^^^ help: try: `Vec` @@ -8,49 +8,49 @@ LL | const C: Vec> = Vec::new(); = help: to override `-D warnings` add `#[allow(clippy::vec_box)]` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:25:15 + --> $DIR/vec_box_sized.rs:27:15 | LL | static S: Vec> = Vec::new(); | ^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:28:21 + --> $DIR/vec_box_sized.rs:30:21 | LL | sized_type: Vec>, | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:31:14 + --> $DIR/vec_box_sized.rs:33:14 | LL | struct A(Vec>); | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:32:18 + --> $DIR/vec_box_sized.rs:34:18 | LL | struct B(Vec>>); | ^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:34:42 + --> $DIR/vec_box_sized.rs:36:42 | LL | fn allocator_global_defined_vec() -> Vec, std::alloc::Global> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:37:42 + --> $DIR/vec_box_sized.rs:39:42 | LL | fn allocator_global_defined_box() -> Vec> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:40:29 + --> $DIR/vec_box_sized.rs:42:29 | LL | fn allocator_match() -> Vec, DummyAllocator> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:74:23 + --> $DIR/vec_box_sized.rs:76:23 | LL | pub fn f() -> Vec> { | ^^^^^^^^^^^ help: try: `Vec` From 483b109e6e7f741f0827fde46c9263636498b12a Mon Sep 17 00:00:00 2001 From: Jacherr Date: Wed, 8 Nov 2023 21:17:40 +0000 Subject: [PATCH 6/8] cargo dev fmt --- clippy_lints/src/types/vec_box.rs | 2 +- clippy_utils/src/paths.rs | 2 +- tests/ui/vec_box_sized.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/types/vec_box.rs b/clippy_lints/src/types/vec_box.rs index 43f72f1031d..da4392418ca 100644 --- a/clippy_lints/src/types/vec_box.rs +++ b/clippy_lints/src/types/vec_box.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::{last_path_segment, match_def_path}; use clippy_utils::paths::ALLOCATOR_GLOBAL; use clippy_utils::source::snippet; +use clippy_utils::{last_path_segment, match_def_path}; use rustc_errors::Applicability; use rustc_hir::def_id::DefId; use rustc_hir::{self as hir, GenericArg, QPath, TyKind}; diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index bde8eb892e9..859bffd6c9c 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -99,4 +99,4 @@ pub const OPTION_UNWRAP: [&str; 4] = ["core", "option", "Option", "unwrap"]; pub const OPTION_EXPECT: [&str; 4] = ["core", "option", "Option", "expect"]; #[expect(clippy::invalid_paths)] // not sure why it thinks this, it works so pub const BOOL_THEN: [&str; 4] = ["core", "bool", "", "then"]; -pub const ALLOCATOR_GLOBAL: [&str; 3] = ["alloc", "alloc", "Global"]; \ No newline at end of file +pub const ALLOCATOR_GLOBAL: [&str; 3] = ["alloc", "alloc", "Global"]; diff --git a/tests/ui/vec_box_sized.rs b/tests/ui/vec_box_sized.rs index af471f1eb8d..e46f5bf2126 100644 --- a/tests/ui/vec_box_sized.rs +++ b/tests/ui/vec_box_sized.rs @@ -3,7 +3,7 @@ #![allow(dead_code)] #![feature(allocator_api)] -use std::alloc::{Layout, AllocError, Allocator}; +use std::alloc::{AllocError, Allocator, Layout}; use std::ptr::NonNull; struct SizedStruct(i32); @@ -22,7 +22,7 @@ unsafe impl Allocator for DummyAllocator { /// The following should trigger the lint mod should_trigger { - use super::{SizedStruct, DummyAllocator}; + use super::{DummyAllocator, SizedStruct}; const C: Vec> = Vec::new(); static S: Vec> = Vec::new(); @@ -46,7 +46,7 @@ mod should_trigger { /// The following should not trigger the lint mod should_not_trigger { - use super::{BigStruct, UnsizedStruct, DummyAllocator}; + use super::{BigStruct, DummyAllocator, UnsizedStruct}; struct C(Vec>); struct D(Vec>); From 7cdaa3b574a1c9dd94c1d0abc260f8a4d366cd98 Mon Sep 17 00:00:00 2001 From: Jacherr Date: Wed, 8 Nov 2023 21:47:58 +0000 Subject: [PATCH 7/8] replace incorrect bool --- clippy_lints/src/types/vec_box.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/types/vec_box.rs b/clippy_lints/src/types/vec_box.rs index da4392418ca..9d5066199bd 100644 --- a/clippy_lints/src/types/vec_box.rs +++ b/clippy_lints/src/types/vec_box.rs @@ -52,7 +52,7 @@ pub(super) fn check( && let Some(did) = cx.qpath_res(&path, inner.hir_id).opt_def_id() { match_def_path(cx, did, &ALLOCATOR_GLOBAL) } else { - true + false } }, (Some(GenericArg::Type(l)), Some(GenericArg::Type(r))) => From eabc64f56c4a907b942881b94815cbe2150e39db Mon Sep 17 00:00:00 2001 From: Jacherr Date: Thu, 9 Nov 2023 23:03:44 +0000 Subject: [PATCH 8/8] add additonal non-trigger testcase --- tests/ui/vec_box_sized.rs | 3 +++ tests/ui/vec_box_sized.stderr | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/ui/vec_box_sized.rs b/tests/ui/vec_box_sized.rs index e46f5bf2126..49eaf8e062a 100644 --- a/tests/ui/vec_box_sized.rs +++ b/tests/ui/vec_box_sized.rs @@ -63,6 +63,9 @@ mod should_not_trigger { fn allocator_mismatch() -> Vec> { Vec::new() } + fn allocator_mismatch_2() -> Vec, DummyAllocator> { + Vec::new_in(DummyAllocator) + } } mod inner_mod { diff --git a/tests/ui/vec_box_sized.stderr b/tests/ui/vec_box_sized.stderr index db5674210d9..d6479271fa6 100644 --- a/tests/ui/vec_box_sized.stderr +++ b/tests/ui/vec_box_sized.stderr @@ -50,7 +50,7 @@ LL | fn allocator_match() -> Vec, DummyAllocator> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec` error: `Vec` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:76:23 + --> $DIR/vec_box_sized.rs:79:23 | LL | pub fn f() -> Vec> { | ^^^^^^^^^^^ help: try: `Vec`