Skip to content

Commit 91e537b

Browse files
authored
Don't trigger unnecessary_box_returns when the size depends on generics (#17249)
Fixes #17202. `unnecessary_box_returns` fired on return types whose size depends on generic parameters, e.g.: ```rust fn strip<const N: usize, T>(value: Box<[SyncUnsafeCell<T>; N]>) -> Box<[T; N]> { /* ... */ } // ^^^^^^^^^^^ help: try: `[T; N]` ``` The size of `[T; N]` can't be determined here (it depends on `T` and the const parameter `N`), but `approx_ty_size` falls back to `0` for such arrays, so the `<= unnecessary_box_size` check always passed and the lint fired. Returning a `Box` is often deliberate in these cases to avoid copying a potentially large value. The lint now queries the type's layout directly and only fires when the size can actually be computed. Concrete types and generic types with a known layout (e.g. `Box<Vec<T>>`) are unaffected. changelog: [`unnecessary_box_returns`]: no longer fires when the boxed type's size depends on generic parameters (e.g. `Box<[T; N]>`)
2 parents 440f1a4 + 3054d6f commit 91e537b

3 files changed

Lines changed: 34 additions & 4 deletions

File tree

clippy_lints/src/unnecessary_box_returns.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
use clippy_config::Conf;
22
use clippy_utils::diagnostics::span_lint_and_then;
3-
use clippy_utils::ty::approx_ty_size;
43
use rustc_errors::Applicability;
54
use rustc_hir::def_id::LocalDefId;
65
use rustc_hir::{FnDecl, FnRetTy, ImplItemKind, Item, ItemKind, Node, TraitItem, TraitItemKind};
76
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_middle::ty::layout::LayoutOf;
88
use rustc_session::impl_lint_pass;
99
use rustc_span::Symbol;
1010

@@ -81,8 +81,13 @@ impl UnnecessaryBoxReturns {
8181

8282
// It's sometimes useful to return Box<T> if T is unsized, so don't lint those.
8383
// Also, don't lint if we know that T is very large, in which case returning
84-
// a Box<T> may be beneficial.
85-
if boxed_ty.is_sized(cx.tcx, cx.typing_env()) && approx_ty_size(cx, boxed_ty) <= self.maximum_size {
84+
// a Box<T> may be beneficial. When the size depends on generic parameters
85+
// (e.g. `[T; N]`) it cannot be determined here, so don't lint that either, as
86+
// the `Box` may be a deliberate choice to avoid copying a large value.
87+
if boxed_ty.is_sized(cx.tcx, cx.typing_env())
88+
&& let Ok(layout) = cx.layout_of(boxed_ty)
89+
&& layout.size.bytes() <= self.maximum_size
90+
{
8691
span_lint_and_then(
8792
cx,
8893
UNNECESSARY_BOX_RETURNS,

tests/ui/unnecessary_box_returns.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,23 @@ impl HasHuge {
7171
}
7272
}
7373

74+
// don't lint (issue #17202): the size of `[T; N]` depends on a const generic parameter
75+
fn const_generic_array<const N: usize, T>(value: Box<[T; N]>) -> Box<[T; N]> {
76+
value
77+
}
78+
79+
// don't lint (issue #17202): the size of `[T; 10]` depends on the generic element type
80+
fn generic_element_array<T>(value: Box<[T; 10]>) -> Box<[T; 10]> {
81+
value
82+
}
83+
84+
// lint: the size of `Vec<T>` is known regardless of the generic element type
85+
#[expect(clippy::box_collection)]
86+
fn generic_vec<T>(value: Box<Vec<T>>) -> Box<Vec<T>> {
87+
//~^ unnecessary_box_returns
88+
value
89+
}
90+
7491
fn main() {
7592
// don't lint: this is a closure
7693
let a = || -> Box<usize> { Box::new(5) };

tests/ui/unnecessary_box_returns.stderr

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,13 @@ LL | fn _bxed_foo() -> Box<Foo> {
3232
|
3333
= help: changing this also requires a change to the return expressions in this function
3434

35-
error: aborting due to 4 previous errors
35+
error: boxed return of the sized type `std::vec::Vec<T>`
36+
--> tests/ui/unnecessary_box_returns.rs:86:42
37+
|
38+
LL | fn generic_vec<T>(value: Box<Vec<T>>) -> Box<Vec<T>> {
39+
| ^^^^^^^^^^^ help: try: `std::vec::Vec<T>`
40+
|
41+
= help: changing this also requires a change to the return expressions in this function
42+
43+
error: aborting due to 5 previous errors
3644

0 commit comments

Comments
 (0)