Auto merge of rust-lang#10651 - lukaslueg:issue10641, r=xFrednet

Add size-parameter to unecessary_box_returns

Fixes rust-lang#10641

This adds a configuration-knob to the `unecessary_box_returns`-lint which allows _not_ linting a `fn() -> Box<T>` if `T` is "large". The default byte size above which we no longer lint is 128 bytes (due to rust-lang/rust-clippy#4652 (comment), also used in rust-lang#9373). The overall rational is given in rust-lang#10641.

---

changelog: Enhancement: [`unnecessary_box_returns`]: Added new lint configuration `unnecessary-box-size` to set the maximum size of `T` in `Box<T>` to be linted
[rust-lang#10651](rust-lang/rust-clippy#10651)
<!-- changelog_checked -->

Author: bors

book/src/lint_configuration.md

@@ -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.
@@ -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)
+
+
 

clippy_lints/src/lib.rs

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

clippy_lints/src/unnecessary_box_returns.rs

@@ -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};
@@ -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
@@ -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,
         }
     }
 
@@ -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,

clippy_lints/src/utils/conf.rs

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

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

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

tests/ui/unnecessary_box_returns.rs

@@ -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) };