Skip to content

Commit

Permalink
Add size-parameter to unecessary_box_returns
Browse files Browse the repository at this point in the history
Fixes #10641
  • Loading branch information
lukaslueg committed Apr 16, 2023
1 parent 0049816 commit 819e02a
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 4 deletions.
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

0 comments on commit 819e02a

Please sign in to comment.