Skip to content

Add size-parameter to unecessary_box_returns #10651

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ Please use that command to update the file and do not edit it by hand.
| [suppress-restriction-lint-in-const](#suppress-restriction-lint-in-const) | `false` |
| [missing-docs-in-crate-items](#missing-docs-in-crate-items) | `false` |
| [future-size-threshold](#future-size-threshold) | `16384` |
| [unnecessary-box-size](#unnecessary-box-size) | `128` |

### arithmetic-side-effects-allowed
Suppress checking of the passed type names in all types of operations.
Expand Down Expand Up @@ -561,4 +562,12 @@ The maximum byte size a `Future` can have, before it triggers the `clippy::large
* [large_futures](https://rust-lang.github.io/rust-clippy/master/index.html#large_futures)


### unnecessary-box-size
The byte size a `T` in `Box<T>` can have, below which it triggers the `clippy::unnecessary_box` lint

**Default Value:** `128` (`u64`)

* [unnecessary_box_returns](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns)



2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -950,9 +950,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(allow_attributes::AllowAttribute));
store.register_late_pass(move |_| Box::new(manual_main_separator_str::ManualMainSeparatorStr::new(msrv())));
store.register_late_pass(|_| Box::new(unnecessary_struct_initialization::UnnecessaryStruct));
let unnecessary_box_size = conf.unnecessary_box_size;
store.register_late_pass(move |_| {
Box::new(unnecessary_box_returns::UnnecessaryBoxReturns::new(
avoid_breaking_exported_api,
unnecessary_box_size,
))
});
store.register_late_pass(|_| Box::new(lines_filter_map_ok::LinesFilterMapOk));
Expand Down
15 changes: 11 additions & 4 deletions clippy_lints/src/unnecessary_box_returns.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{diagnostics::span_lint_and_then, ty::approx_ty_size};
use rustc_errors::Applicability;
use rustc_hir::{def_id::LocalDefId, FnDecl, FnRetTy, ImplItemKind, Item, ItemKind, Node, TraitItem, TraitItemKind};
use rustc_lint::{LateContext, LateLintPass};
Expand All @@ -10,6 +10,9 @@ declare_clippy_lint! {
///
/// Checks for a return type containing a `Box<T>` where `T` implements `Sized`
///
/// The lint ignores `Box<T>` where `T` is larger than `unnecessary_box_size`,
/// as returning a large `T` directly may be detrimental to performance.
///
/// ### Why is this bad?
///
/// It's better to just return `T` in these cases. The caller may not need
Expand All @@ -36,14 +39,16 @@ declare_clippy_lint! {

pub struct UnnecessaryBoxReturns {
avoid_breaking_exported_api: bool,
maximum_size: u64,
}

impl_lint_pass!(UnnecessaryBoxReturns => [UNNECESSARY_BOX_RETURNS]);

impl UnnecessaryBoxReturns {
pub fn new(avoid_breaking_exported_api: bool) -> Self {
pub fn new(avoid_breaking_exported_api: bool, maximum_size: u64) -> Self {
Self {
avoid_breaking_exported_api,
maximum_size,
}
}

Expand Down Expand Up @@ -71,8 +76,10 @@ impl UnnecessaryBoxReturns {

let boxed_ty = return_ty.boxed_ty();

// it's sometimes useful to return Box<T> if T is unsized, so don't lint those
if boxed_ty.is_sized(cx.tcx, cx.param_env) {
// It's sometimes useful to return Box<T> if T is unsized, so don't lint those.
// Also, don't lint if we know that T is very large, in which case returning
// a Box<T> may be beneficial.
if boxed_ty.is_sized(cx.tcx, cx.param_env) && approx_ty_size(cx, boxed_ty) <= self.maximum_size {
span_lint_and_then(
cx,
UNNECESSARY_BOX_RETURNS,
Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,10 @@ define_Conf! {
///
/// The maximum byte size a `Future` can have, before it triggers the `clippy::large_futures` lint
(future_size_threshold: u64 = 16 * 1024),
/// Lint: UNNECESSARY_BOX_RETURNS.
///
/// The byte size a `T` in `Box<T>` can have, below which it triggers the `clippy::unnecessary_box` lint
(unnecessary_box_size: u64 = 128),
}

/// Search for the configuration file.
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown fie
too-many-lines-threshold
trivial-copy-size-limit
type-complexity-threshold
unnecessary-box-size
unreadable-literal-lint-fractions
upper-case-acronyms-aggressive
vec-box-size-threshold
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/unnecessary_box_returns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@ fn string() -> String {
String::from("Hello, world")
}

struct Huge([u8; 500]);
struct HasHuge(Box<Huge>);

impl HasHuge {
// don't lint: The size of `Huge` is very large
fn into_huge(self) -> Box<Huge> {
self.0
}
}

fn main() {
// don't lint: this is a closure
let a = || -> Box<usize> { Box::new(5) };
Expand Down