diff --git a/clippy_lints/src/types/vec_box.rs b/clippy_lints/src/types/vec_box.rs index 887439560d12..43f72f1031d7 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 5bca554378e7..bde8eb892e92 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 67067de91cc7..0be891a92bc0 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 2b5c2c99945e..9314195b1033 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 0e68aa57dcb1..2fa5679650b3 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