mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #11780 - Jacherr:vec-allocator-nolint, r=xFrednet

Browse Source 
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
This commit is contained in:
bors 2023-11-09 23:33:46 +00:00
commit 6be0f7414d
5 changed files with 110 additions and 101 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
clippy_lints/src/types/vec_box.rs
View File

@ -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

1
clippy_utils/src/paths.rs
View File

@ -99,3 +99,4 @@
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"];

57
tests/ui/vec_box_sized.fixed
View File

@ -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() {}

37
tests/ui/vec_box_sized.rs
View File

@ -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 @@ struct StructWithVecBox {
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 @@ struct TraitVec<T: ?Sized> {
// 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 {

32
tests/ui/vec_box_sized.stderr
View File

@ -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