diff options
| author | bors <bors@rust-lang.org> | 2023-11-09 23:33:46 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-11-09 23:33:46 +0000 |
| commit | 6be0f7414d958edf45dc0dabae33b259731ad040 (patch) | |
| tree | 9d4dd63255d8ef82e76197e2d4d37108c98fbd80 | |
| parent | 34b7d1559f03f4a35316ed2a5261a87b591834bb (diff) | |
| parent | eabc64f56c4a907b942881b94815cbe2150e39db (diff) | |
| download | rust-6be0f7414d958edf45dc0dabae33b259731ad040.tar.gz rust-6be0f7414d958edf45dc0dabae33b259731ad040.zip | |
Auto merge of #11780 - Jacherr:vec-allocator-nolint, r=xFrednet
Disable `vec_box` when using different allocators
Fixes #7114
This PR disables the `vec_box` lint when the `Box` and `Vec` use different allocators (but not when they use the same - custom - allocator).
For example - `Vec<Box<i32, DummyAllocator>>` will disable the lint, and `Vec<Box<i32, DummyAllocator>, DummyAllocator>` will not disable the lint.
In addition, the applicability of this lint has been changed to `Unspecified` due to the automatic fixes potentially breaking code such as the following:
```rs
fn foo() -> Vec<Box<i32>> { // -> Vec<i32>
vec![Box::new(1)]
}
```
It should be noted that the `if_chain->let-chains` fix has also been applied to this lint, so the diff does contain many changes.
changelog: disable `vec_box` lint when using nonstandard allocators
| -rw-r--r-- | clippy_lints/src/types/vec_box.rs | 84 | ||||
| -rw-r--r-- | clippy_utils/src/paths.rs | 1 | ||||
| -rw-r--r-- | tests/ui/vec_box_sized.fixed | 57 | ||||
| -rw-r--r-- | tests/ui/vec_box_sized.rs | 37 | ||||
| -rw-r--r-- | tests/ui/vec_box_sized.stderr | 32 |
5 files changed, 110 insertions, 101 deletions
diff --git a/clippy_lints/src/types/vec_box.rs b/clippy_lints/src/types/vec_box.rs index decc183ad96..9d5066199bd 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; +use clippy_utils::paths::ALLOCATOR_GLOBAL; use clippy_utils::source::snippet; -use if_chain::if_chain; +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}; @@ -21,43 +21,57 @@ 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() + // extract allocator from the Vec for later + && let vec_alloc_ty = last.args.get(1) // 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<T>, 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<T>` 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() + // 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() + && 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 + && 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 { + false + } + }, + (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, + VEC_BOX, + hir_ty.span, + "`Vec<T>` is already on the heap, the boxing is unnecessary", + "try", + format!("Vec<{}>", snippet(cx, boxed_ty.span, "..")), + Applicability::Unspecified, + ); + true + } else { + false } } else { false diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 5bca554378e..859bffd6c9c 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", "<impl bool>", "then"]; +pub const ALLOCATOR_GLOBAL: [&str; 3] = ["alloc", "alloc", "Global"]; diff --git a/tests/ui/vec_box_sized.fixed b/tests/ui/vec_box_sized.fixed deleted file mode 100644 index 4363d2224af..00000000000 --- a/tests/ui/vec_box_sized.fixed +++ /dev/null @@ -1,57 +0,0 @@ -#![allow(dead_code)] - -struct SizedStruct(i32); -struct UnsizedStruct([i32]); -struct BigStruct([i32; 10000]); - -/// The following should trigger the lint -mod should_trigger { - use super::SizedStruct; - const C: Vec<i32> = Vec::new(); - static S: Vec<i32> = Vec::new(); - - struct StructWithVecBox { - sized_type: Vec<SizedStruct>, - } - - struct A(Vec<SizedStruct>); - struct B(Vec<Vec<u32>>); -} - -/// The following should not trigger the lint -mod should_not_trigger { - use super::{BigStruct, UnsizedStruct}; - - struct C(Vec<Box<UnsizedStruct>>); - struct D(Vec<Box<BigStruct>>); - - struct StructWithVecBoxButItsUnsized { - unsized_type: Vec<Box<UnsizedStruct>>, - } - - struct TraitVec<T: ?Sized> { - // Regression test for #3720. This was causing an ICE. - inner: Vec<Box<T>>, - } -} - -mod inner_mod { - mod inner { - pub struct S; - } - - mod inner2 { - use super::inner::S; - - pub fn f() -> Vec<S> { - vec![] - } - } -} - -// https://github.com/rust-lang/rust-clippy/issues/11417 -fn in_closure() { - let _ = |_: Vec<Box<dyn ToString>>| {}; -} - -fn main() {} diff --git a/tests/ui/vec_box_sized.rs b/tests/ui/vec_box_sized.rs index f4e27fe4bd5..49eaf8e062a 100644 --- a/tests/ui/vec_box_sized.rs +++ b/tests/ui/vec_box_sized.rs @@ -1,12 +1,28 @@ +//@no-rustfix + #![allow(dead_code)] +#![feature(allocator_api)] + +use std::alloc::{AllocError, Allocator, Layout}; +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<NonNull<[u8]>, AllocError> { + todo!() + } + unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) { + todo!() + } +} + /// The following should trigger the lint mod should_trigger { - use super::SizedStruct; + use super::{DummyAllocator, SizedStruct}; const C: Vec<Box<i32>> = Vec::new(); static S: Vec<Box<i32>> = Vec::new(); @@ -16,11 +32,21 @@ mod should_trigger { struct A(Vec<Box<SizedStruct>>); struct B(Vec<Vec<Box<(u32)>>>); + + fn allocator_global_defined_vec() -> Vec<Box<i32>, std::alloc::Global> { + Vec::new() + } + fn allocator_global_defined_box() -> Vec<Box<i32, std::alloc::Global>> { + Vec::new() + } + fn allocator_match() -> Vec<Box<i32, DummyAllocator>, DummyAllocator> { + Vec::new_in(DummyAllocator) + } } /// The following should not trigger the lint mod should_not_trigger { - use super::{BigStruct, UnsizedStruct}; + use super::{BigStruct, DummyAllocator, UnsizedStruct}; struct C(Vec<Box<UnsizedStruct>>); struct D(Vec<Box<BigStruct>>); @@ -33,6 +59,13 @@ mod should_not_trigger { // Regression test for #3720. This was causing an ICE. inner: Vec<Box<T>>, } + + fn allocator_mismatch() -> Vec<Box<i32, DummyAllocator>> { + Vec::new() + } + fn allocator_mismatch_2() -> Vec<Box<i32>, 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 9118f284bb9..d6479271fa6 100644 --- a/tests/ui/vec_box_sized.stderr +++ b/tests/ui/vec_box_sized.stderr @@ -1,5 +1,5 @@ error: `Vec<T>` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:10:14 + --> $DIR/vec_box_sized.rs:26:14 | LL | const C: Vec<Box<i32>> = Vec::new(); | ^^^^^^^^^^^^^ help: try: `Vec<i32>` @@ -8,34 +8,52 @@ LL | const C: Vec<Box<i32>> = Vec::new(); = help: to override `-D warnings` add `#[allow(clippy::vec_box)]` error: `Vec<T>` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:11:15 + --> $DIR/vec_box_sized.rs:27:15 | LL | static S: Vec<Box<i32>> = Vec::new(); | ^^^^^^^^^^^^^ help: try: `Vec<i32>` error: `Vec<T>` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:14:21 + --> $DIR/vec_box_sized.rs:30:21 | LL | sized_type: Vec<Box<SizedStruct>>, | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>` error: `Vec<T>` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:17:14 + --> $DIR/vec_box_sized.rs:33:14 | LL | struct A(Vec<Box<SizedStruct>>); | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>` error: `Vec<T>` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:18:18 + --> $DIR/vec_box_sized.rs:34:18 | LL | struct B(Vec<Vec<Box<(u32)>>>); | ^^^^^^^^^^^^^^^ help: try: `Vec<u32>` error: `Vec<T>` is already on the heap, the boxing is unnecessary - --> $DIR/vec_box_sized.rs:46:23 + --> $DIR/vec_box_sized.rs:36:42 + | +LL | fn allocator_global_defined_vec() -> Vec<Box<i32>, std::alloc::Global> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<i32>` + +error: `Vec<T>` is already on the heap, the boxing is unnecessary + --> $DIR/vec_box_sized.rs:39:42 + | +LL | fn allocator_global_defined_box() -> Vec<Box<i32, std::alloc::Global>> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<i32>` + +error: `Vec<T>` is already on the heap, the boxing is unnecessary + --> $DIR/vec_box_sized.rs:42:29 + | +LL | fn allocator_match() -> Vec<Box<i32, DummyAllocator>, DummyAllocator> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<i32>` + +error: `Vec<T>` is already on the heap, the boxing is unnecessary + --> $DIR/vec_box_sized.rs:79:23 | LL | pub fn f() -> Vec<Box<S>> { | ^^^^^^^^^^^ help: try: `Vec<S>` -error: aborting due to 6 previous errors +error: aborting due to 9 previous errors |
