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

Add size-parameter to unecessary_box_returns

Fixes #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 #4652 (comment), also used in #9373). The overall rational is given in #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
[#10651](#10651)
<!-- changelog_checked -->

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