Skip to content

Commit 67bb503

Browse files
committed
add support for std::alloc::Global, add more tests
1 parent 7932560 commit 67bb503

File tree

5 files changed

+92
-41
lines changed

5 files changed

+92
-41
lines changed

clippy_lints/src/types/vec_box.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::last_path_segment;
2+
use clippy_utils::{last_path_segment, match_def_path};
3+
use clippy_utils::paths::ALLOCATOR_GLOBAL;
34
use clippy_utils::source::snippet;
45
use rustc_errors::Applicability;
56
use rustc_hir::def_id::DefId;
@@ -41,10 +42,19 @@ pub(super) fn check(
4142
&& let Ok(ty_ty_size) = cx.layout_of(ty_ty).map(|l| l.size.bytes())
4243
&& ty_ty_size < box_size_threshold
4344
// https://github.com/rust-lang/rust-clippy/issues/7114
44-
// this code also does not consider that Global can be explicitly
45-
// defined (yet) as in Vec<_, Global> so a very slight false negative edge case
4645
&& match (vec_alloc_ty, boxed_alloc_ty) {
4746
(None, None) => true,
47+
// this is in the event that we have something like
48+
// Vec<_, Global>, in which case is equivalent to
49+
// Vec<_>
50+
(None, Some(GenericArg::Type(inner))) | (Some(GenericArg::Type(inner)), None) => {
51+
if let TyKind::Path(path) = inner.kind
52+
&& let Some(did) = cx.qpath_res(&path, inner.hir_id).opt_def_id() {
53+
match_def_path(cx, did, &ALLOCATOR_GLOBAL)
54+
} else {
55+
true
56+
}
57+
},
4858
(Some(GenericArg::Type(l)), Some(GenericArg::Type(r))) =>
4959
hir_ty_to_ty(cx.tcx, l) == hir_ty_to_ty(cx.tcx, r),
5060
_ => false
@@ -57,7 +67,7 @@ pub(super) fn check(
5767
"`Vec<T>` is already on the heap, the boxing is unnecessary",
5868
"try",
5969
format!("Vec<{}>", snippet(cx, boxed_ty.span, "..")),
60-
Applicability::MachineApplicable,
70+
Applicability::Unspecified,
6171
);
6272
true
6373
} else {

clippy_utils/src/paths.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,4 @@ pub const OPTION_UNWRAP: [&str; 4] = ["core", "option", "Option", "unwrap"];
9999
pub const OPTION_EXPECT: [&str; 4] = ["core", "option", "Option", "expect"];
100100
#[expect(clippy::invalid_paths)] // not sure why it thinks this, it works so
101101
pub const BOOL_THEN: [&str; 4] = ["core", "bool", "<impl bool>", "then"];
102+
pub const ALLOCATOR_GLOBAL: [&str; 3] = ["alloc", "alloc", "Global"];

tests/ui/vec_box_sized.fixed

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,26 @@
11
#![allow(dead_code)]
22
#![feature(allocator_api)]
33

4+
use std::alloc::{Layout, AllocError, Allocator};
5+
use std::ptr::NonNull;
6+
47
struct SizedStruct(i32);
58
struct UnsizedStruct([i32]);
69
struct BigStruct([i32; 10000]);
710

11+
struct DummyAllocator;
12+
unsafe impl Allocator for DummyAllocator {
13+
fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
14+
todo!()
15+
}
16+
unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
17+
todo!()
18+
}
19+
}
20+
821
/// The following should trigger the lint
922
mod should_trigger {
10-
use super::SizedStruct;
23+
use super::{SizedStruct, DummyAllocator};
1124
const C: Vec<i32> = Vec::new();
1225
static S: Vec<i32> = Vec::new();
1326

@@ -17,13 +30,21 @@ mod should_trigger {
1730

1831
struct A(Vec<SizedStruct>);
1932
struct B(Vec<Vec<u32>>);
33+
34+
fn allocator_global_defined_vec() -> Vec<i32> {
35+
Vec::new()
36+
}
37+
fn allocator_global_defined_box() -> Vec<i32> {
38+
Vec::new()
39+
}
40+
fn allocator_match() -> Vec<i32> {
41+
Vec::new_in(DummyAllocator)
42+
}
2043
}
2144

2245
/// The following should not trigger the lint
2346
mod should_not_trigger {
24-
use super::{BigStruct, UnsizedStruct};
25-
use std::alloc::{Layout, AllocError, Allocator};
26-
use std::ptr::NonNull;
47+
use super::{BigStruct, UnsizedStruct, DummyAllocator};
2748

2849
struct C(Vec<Box<UnsizedStruct>>);
2950
struct D(Vec<Box<BigStruct>>);
@@ -37,18 +58,8 @@ mod should_not_trigger {
3758
inner: Vec<Box<T>>,
3859
}
3960

40-
struct DummyAllocator;
41-
unsafe impl Allocator for DummyAllocator {
42-
fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
43-
todo!()
44-
}
45-
unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
46-
todo!()
47-
}
48-
}
49-
5061
fn allocator_mismatch() -> Vec<Box<i32, DummyAllocator>> {
51-
vec![]
62+
Vec::new()
5263
}
5364
}
5465

tests/ui/vec_box_sized.rs

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,26 @@
11
#![allow(dead_code)]
22
#![feature(allocator_api)]
33

4+
use std::alloc::{Layout, AllocError, Allocator};
5+
use std::ptr::NonNull;
6+
47
struct SizedStruct(i32);
58
struct UnsizedStruct([i32]);
69
struct BigStruct([i32; 10000]);
710

11+
struct DummyAllocator;
12+
unsafe impl Allocator for DummyAllocator {
13+
fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
14+
todo!()
15+
}
16+
unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
17+
todo!()
18+
}
19+
}
20+
821
/// The following should trigger the lint
922
mod should_trigger {
10-
use super::SizedStruct;
23+
use super::{SizedStruct, DummyAllocator};
1124
const C: Vec<Box<i32>> = Vec::new();
1225
static S: Vec<Box<i32>> = Vec::new();
1326

@@ -17,13 +30,21 @@ mod should_trigger {
1730

1831
struct A(Vec<Box<SizedStruct>>);
1932
struct B(Vec<Vec<Box<(u32)>>>);
33+
34+
fn allocator_global_defined_vec() -> Vec<Box<i32>, std::alloc::Global> {
35+
Vec::new()
36+
}
37+
fn allocator_global_defined_box() -> Vec<Box<i32, std::alloc::Global>> {
38+
Vec::new()
39+
}
40+
fn allocator_match() -> Vec<Box<i32, DummyAllocator>, DummyAllocator> {
41+
Vec::new_in(DummyAllocator)
42+
}
2043
}
2144

2245
/// The following should not trigger the lint
2346
mod should_not_trigger {
24-
use super::{BigStruct, UnsizedStruct};
25-
use std::alloc::{Layout, AllocError, Allocator};
26-
use std::ptr::NonNull;
47+
use super::{BigStruct, UnsizedStruct, DummyAllocator};
2748

2849
struct C(Vec<Box<UnsizedStruct>>);
2950
struct D(Vec<Box<BigStruct>>);
@@ -37,18 +58,8 @@ mod should_not_trigger {
3758
inner: Vec<Box<T>>,
3859
}
3960

40-
struct DummyAllocator;
41-
unsafe impl Allocator for DummyAllocator {
42-
fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
43-
todo!()
44-
}
45-
unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
46-
todo!()
47-
}
48-
}
49-
5061
fn allocator_mismatch() -> Vec<Box<i32, DummyAllocator>> {
51-
vec![]
62+
Vec::new()
5263
}
5364
}
5465

tests/ui/vec_box_sized.stderr

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: `Vec<T>` is already on the heap, the boxing is unnecessary
2-
--> $DIR/vec_box_sized.rs:11:14
2+
--> $DIR/vec_box_sized.rs:24:14
33
|
44
LL | const C: Vec<Box<i32>> = Vec::new();
55
| ^^^^^^^^^^^^^ help: try: `Vec<i32>`
@@ -8,34 +8,52 @@ LL | const C: Vec<Box<i32>> = Vec::new();
88
= help: to override `-D warnings` add `#[allow(clippy::vec_box)]`
99

1010
error: `Vec<T>` is already on the heap, the boxing is unnecessary
11-
--> $DIR/vec_box_sized.rs:12:15
11+
--> $DIR/vec_box_sized.rs:25:15
1212
|
1313
LL | static S: Vec<Box<i32>> = Vec::new();
1414
| ^^^^^^^^^^^^^ help: try: `Vec<i32>`
1515

1616
error: `Vec<T>` is already on the heap, the boxing is unnecessary
17-
--> $DIR/vec_box_sized.rs:15:21
17+
--> $DIR/vec_box_sized.rs:28:21
1818
|
1919
LL | sized_type: Vec<Box<SizedStruct>>,
2020
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>`
2121

2222
error: `Vec<T>` is already on the heap, the boxing is unnecessary
23-
--> $DIR/vec_box_sized.rs:18:14
23+
--> $DIR/vec_box_sized.rs:31:14
2424
|
2525
LL | struct A(Vec<Box<SizedStruct>>);
2626
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>`
2727

2828
error: `Vec<T>` is already on the heap, the boxing is unnecessary
29-
--> $DIR/vec_box_sized.rs:19:18
29+
--> $DIR/vec_box_sized.rs:32:18
3030
|
3131
LL | struct B(Vec<Vec<Box<(u32)>>>);
3232
| ^^^^^^^^^^^^^^^ help: try: `Vec<u32>`
3333

3434
error: `Vec<T>` is already on the heap, the boxing is unnecessary
35-
--> $DIR/vec_box_sized.rs:63:23
35+
--> $DIR/vec_box_sized.rs:34:42
36+
|
37+
LL | fn allocator_global_defined_vec() -> Vec<Box<i32>, std::alloc::Global> {
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<i32>`
39+
40+
error: `Vec<T>` is already on the heap, the boxing is unnecessary
41+
--> $DIR/vec_box_sized.rs:37:42
42+
|
43+
LL | fn allocator_global_defined_box() -> Vec<Box<i32, std::alloc::Global>> {
44+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<i32>`
45+
46+
error: `Vec<T>` is already on the heap, the boxing is unnecessary
47+
--> $DIR/vec_box_sized.rs:40:29
48+
|
49+
LL | fn allocator_match() -> Vec<Box<i32, DummyAllocator>, DummyAllocator> {
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<i32>`
51+
52+
error: `Vec<T>` is already on the heap, the boxing is unnecessary
53+
--> $DIR/vec_box_sized.rs:74:23
3654
|
3755
LL | pub fn f() -> Vec<Box<S>> {
3856
| ^^^^^^^^^^^ help: try: `Vec<S>`
3957

40-
error: aborting due to 6 previous errors
58+
error: aborting due to 9 previous errors
4159

0 commit comments

Comments
 (0)