From e8731a926c9a642ca1ddf5b52baf40e0a8873d53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Baasch=20de=20Souza?= Date: Thu, 8 Oct 2020 02:17:32 -0300 Subject: [PATCH] Add large_types_passed_by_value lint Refactor trivially_copy_pass_by_ref and the new lint into pass_by_ref_or_value module Update stderr of conf_unknown_key test Rename lint to large_types_passed_by_value Increase `pass_by_value_size_limit` default value to 256 Improve rules for `large_types_passed_by_value` Improve tests for `large_types_passed_by_value` Improve documentation for `large_types_passed_by_value` Make minor corrections to pass_by_ref_or_value.rs suggested by clippy itself Fix `large_types_passed_by_value` example and improve docs pass_by_ref_or_value: Tweak check for mut annotation in params large_types_passed_by_value: add tests for pub trait, trait impl and inline attributes --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 13 +- clippy_lints/src/pass_by_ref_or_value.rs | 256 ++++++++++++++++++ .../src/trivially_copy_pass_by_ref.rs | 183 ------------- clippy_lints/src/utils/conf.rs | 2 + src/lintlist/mod.rs | 9 +- .../toml_unknown_key/conf_unknown_key.stderr | 2 +- tests/ui/large_types_passed_by_value.rs | 66 +++++ tests/ui/large_types_passed_by_value.stderr | 52 ++++ 9 files changed, 394 insertions(+), 190 deletions(-) create mode 100644 clippy_lints/src/pass_by_ref_or_value.rs delete mode 100644 clippy_lints/src/trivially_copy_pass_by_ref.rs create mode 100644 tests/ui/large_types_passed_by_value.rs create mode 100644 tests/ui/large_types_passed_by_value.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index d82f970b8bf2..22f963981538 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1779,6 +1779,7 @@ Released 2018-09-13 [`large_digit_groups`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_digit_groups [`large_enum_variant`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant [`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays +[`large_types_passed_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_types_passed_by_value [`len_without_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty [`len_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_zero [`let_and_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d4d2f92a6a69..1a950a7c334c 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -278,6 +278,7 @@ mod overflow_check_conditional; mod panic_in_result_fn; mod panic_unimplemented; mod partialeq_ne_impl; +mod pass_by_ref_or_value; mod path_buf_push_overwrite; mod pattern_type_mismatch; mod precedence; @@ -311,7 +312,6 @@ mod to_string_in_display; mod trait_bounds; mod transmute; mod transmuting_null; -mod trivially_copy_pass_by_ref; mod try_err; mod types; mod unicode; @@ -776,6 +776,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &panic_unimplemented::UNIMPLEMENTED, &panic_unimplemented::UNREACHABLE, &partialeq_ne_impl::PARTIALEQ_NE_IMPL, + &pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE, + &pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF, &path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE, &pattern_type_mismatch::PATTERN_TYPE_MISMATCH, &precedence::PRECEDENCE, @@ -835,7 +837,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &transmute::USELESS_TRANSMUTE, &transmute::WRONG_TRANSMUTE, &transmuting_null::TRANSMUTING_NULL, - &trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF, &try_err::TRY_ERR, &types::ABSURD_EXTREME_COMPARISONS, &types::BORROWED_BOX, @@ -1009,11 +1010,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box large_enum_variant::LargeEnumVariant::new(enum_variant_size_threshold)); store.register_late_pass(|| box explicit_write::ExplicitWrite); store.register_late_pass(|| box needless_pass_by_value::NeedlessPassByValue); - let trivially_copy_pass_by_ref = trivially_copy_pass_by_ref::TriviallyCopyPassByRef::new( + let pass_by_ref_or_value = pass_by_ref_or_value::PassByRefOrValue::new( conf.trivial_copy_size_limit, + conf.pass_by_value_size_limit, &sess.target, ); - store.register_late_pass(move || box trivially_copy_pass_by_ref); + store.register_late_pass(move || box pass_by_ref_or_value); store.register_late_pass(|| box try_err::TryErr); store.register_late_pass(|| box use_self::UseSelf); store.register_late_pass(|| box bytecount::ByteCount); @@ -1237,13 +1239,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&needless_pass_by_value::NEEDLESS_PASS_BY_VALUE), LintId::of(&non_expressive_names::SIMILAR_NAMES), LintId::of(&option_if_let_else::OPTION_IF_LET_ELSE), + LintId::of(&pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE), + LintId::of(&pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF), LintId::of(&ranges::RANGE_MINUS_ONE), LintId::of(&ranges::RANGE_PLUS_ONE), LintId::of(&shadow::SHADOW_UNRELATED), LintId::of(&strings::STRING_ADD_ASSIGN), LintId::of(&trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS), LintId::of(&trait_bounds::TYPE_REPETITION_IN_BOUNDS), - LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF), LintId::of(&types::CAST_LOSSLESS), LintId::of(&types::CAST_POSSIBLE_TRUNCATION), LintId::of(&types::CAST_POSSIBLE_WRAP), diff --git a/clippy_lints/src/pass_by_ref_or_value.rs b/clippy_lints/src/pass_by_ref_or_value.rs new file mode 100644 index 000000000000..28816c3076dd --- /dev/null +++ b/clippy_lints/src/pass_by_ref_or_value.rs @@ -0,0 +1,256 @@ +use std::cmp; + +use crate::utils::{is_copy, is_self_ty, snippet, span_lint_and_sugg}; +use if_chain::if_chain; +use rustc_ast::attr; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{BindingAnnotation, Body, FnDecl, HirId, ItemKind, MutTy, Mutability, Node, PatKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::Span; +use rustc_target::abi::LayoutOf; +use rustc_target::spec::abi::Abi; +use rustc_target::spec::Target; + +declare_clippy_lint! { + /// **What it does:** Checks for functions taking arguments by reference, where + /// the argument type is `Copy` and small enough to be more efficient to always + /// pass by value. + /// + /// **Why is this bad?** In many calling conventions instances of structs will + /// be passed through registers if they fit into two or less general purpose + /// registers. + /// + /// **Known problems:** This lint is target register size dependent, it is + /// limited to 32-bit to try and reduce portability problems between 32 and + /// 64-bit, but if you are compiling for 8 or 16-bit targets then the limit + /// will be different. + /// + /// The configuration option `trivial_copy_size_limit` can be set to override + /// this limit for a project. + /// + /// This lint attempts to allow passing arguments by reference if a reference + /// to that argument is returned. This is implemented by comparing the lifetime + /// of the argument and return value for equality. However, this can cause + /// false positives in cases involving multiple lifetimes that are bounded by + /// each other. + /// + /// **Example:** + /// + /// ```rust + /// // Bad + /// fn foo(v: &u32) {} + /// ``` + /// + /// ```rust + /// // Better + /// fn foo(v: u32) {} + /// ``` + pub TRIVIALLY_COPY_PASS_BY_REF, + pedantic, + "functions taking small copyable arguments by reference" +} + +declare_clippy_lint! { + /// **What it does:** Checks for functions taking arguments by value, where + /// the argument type is `Copy` and large enough to be worth considering + /// passing by reference. Does not trigger if the function is being exported, + /// because that might induce API breakage, if the parameter is declared as mutable, + /// or if the argument is a `self`. + /// + /// **Why is this bad?** Arguments passed by value might result in an unnecessary + /// shallow copy, taking up more space in the stack and requiring a call to + /// `memcpy`, which which can be expensive. + /// + /// **Example:** + /// + /// ```rust + /// #[derive(Clone, Copy)] + /// struct TooLarge([u8; 2048]); + /// + /// // Bad + /// fn foo(v: TooLarge) {} + /// ``` + /// ```rust + /// #[derive(Clone, Copy)] + /// struct TooLarge([u8; 2048]); + /// + /// // Good + /// fn foo(v: &TooLarge) {} + /// ``` + pub LARGE_TYPES_PASSED_BY_VALUE, + pedantic, + "functions taking large arguments by value" +} + +#[derive(Copy, Clone)] +pub struct PassByRefOrValue { + ref_min_size: u64, + value_max_size: u64, +} + +impl<'tcx> PassByRefOrValue { + pub fn new(ref_min_size: Option, value_max_size: u64, target: &Target) -> Self { + let ref_min_size = ref_min_size.unwrap_or_else(|| { + let bit_width = u64::from(target.pointer_width); + // Cap the calculated bit width at 32-bits to reduce + // portability problems between 32 and 64-bit targets + let bit_width = cmp::min(bit_width, 32); + #[allow(clippy::integer_division)] + let byte_width = bit_width / 8; + // Use a limit of 2 times the register byte width + byte_width * 2 + }); + + Self { + ref_min_size, + value_max_size, + } + } + + fn check_poly_fn(&mut self, cx: &LateContext<'tcx>, hir_id: HirId, decl: &FnDecl<'_>, span: Option) { + let fn_def_id = cx.tcx.hir().local_def_id(hir_id); + + let fn_sig = cx.tcx.fn_sig(fn_def_id); + let fn_sig = cx.tcx.erase_late_bound_regions(&fn_sig); + + let fn_body = cx.enclosing_body.map(|id| cx.tcx.hir().body(id)); + + for (index, (input, &ty)) in decl.inputs.iter().zip(fn_sig.inputs()).enumerate() { + // All spans generated from a proc-macro invocation are the same... + match span { + Some(s) if s == input.span => return, + _ => (), + } + + match ty.kind() { + ty::Ref(input_lt, ty, Mutability::Not) => { + // Use lifetimes to determine if we're returning a reference to the + // argument. In that case we can't switch to pass-by-value as the + // argument will not live long enough. + let output_lts = match *fn_sig.output().kind() { + ty::Ref(output_lt, _, _) => vec![output_lt], + ty::Adt(_, substs) => substs.regions().collect(), + _ => vec![], + }; + + if_chain! { + if !output_lts.contains(&input_lt); + if is_copy(cx, ty); + if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes()); + if size <= self.ref_min_size; + if let hir::TyKind::Rptr(_, MutTy { ty: ref decl_ty, .. }) = input.kind; + then { + let value_type = if is_self_ty(decl_ty) { + "self".into() + } else { + snippet(cx, decl_ty.span, "_").into() + }; + span_lint_and_sugg( + cx, + TRIVIALLY_COPY_PASS_BY_REF, + input.span, + &format!("this argument ({} byte) is passed by reference, but would be more efficient if passed by value (limit: {} byte)", size, self.ref_min_size), + "consider passing by value instead", + value_type, + Applicability::Unspecified, + ); + } + } + }, + + ty::Adt(_, _) | ty::Array(_, _) | ty::Tuple(_) => { + // if function has a body and parameter is annotated with mut, ignore + if let Some(param) = fn_body.and_then(|body| body.params.get(index)) { + match param.pat.kind { + PatKind::Binding(BindingAnnotation::Unannotated, _, _, _) => {}, + _ => continue, + } + } + + if_chain! { + if !cx.access_levels.is_exported(hir_id); + if is_copy(cx, ty); + if !is_self_ty(input); + if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes()); + if size > self.value_max_size; + then { + span_lint_and_sugg( + cx, + LARGE_TYPES_PASSED_BY_VALUE, + input.span, + &format!("this argument ({} byte) is passed by value, but might be more efficient if passed by reference (limit: {} byte)", size, self.value_max_size), + "consider passing by reference instead", + format!("&{}", snippet(cx, input.span, "_")), + Applicability::MaybeIncorrect, + ); + } + } + }, + + _ => {}, + } + } + } +} + +impl_lint_pass!(PassByRefOrValue => [TRIVIALLY_COPY_PASS_BY_REF, LARGE_TYPES_PASSED_BY_VALUE]); + +impl<'tcx> LateLintPass<'tcx> for PassByRefOrValue { + fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) { + if item.span.from_expansion() { + return; + } + + if let hir::TraitItemKind::Fn(method_sig, _) = &item.kind { + self.check_poly_fn(cx, item.hir_id, &*method_sig.decl, None); + } + } + + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + decl: &'tcx FnDecl<'_>, + _body: &'tcx Body<'_>, + span: Span, + hir_id: HirId, + ) { + if span.from_expansion() { + return; + } + + match kind { + FnKind::ItemFn(.., header, _, attrs) => { + if header.abi != Abi::Rust { + return; + } + for a in attrs { + if let Some(meta_items) = a.meta_item_list() { + if a.has_name(sym!(proc_macro_derive)) + || (a.has_name(sym!(inline)) && attr::list_contains_name(&meta_items, sym!(always))) + { + return; + } + } + } + }, + FnKind::Method(..) => (), + FnKind::Closure(..) => return, + } + + // Exclude non-inherent impls + if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { + if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. } | + ItemKind::Trait(..)) + { + return; + } + } + + self.check_poly_fn(cx, hir_id, decl, Some(span)); + } +} diff --git a/clippy_lints/src/trivially_copy_pass_by_ref.rs b/clippy_lints/src/trivially_copy_pass_by_ref.rs deleted file mode 100644 index e90ea0fc200a..000000000000 --- a/clippy_lints/src/trivially_copy_pass_by_ref.rs +++ /dev/null @@ -1,183 +0,0 @@ -use std::cmp; - -use crate::utils::{is_copy, is_self_ty, snippet, span_lint_and_sugg}; -use if_chain::if_chain; -use rustc_ast::attr; -use rustc_errors::Applicability; -use rustc_hir as hir; -use rustc_hir::intravisit::FnKind; -use rustc_hir::{Body, FnDecl, HirId, ItemKind, MutTy, Mutability, Node}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty; -use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::Span; -use rustc_target::abi::LayoutOf; -use rustc_target::spec::abi::Abi; -use rustc_target::spec::Target; - -declare_clippy_lint! { - /// **What it does:** Checks for functions taking arguments by reference, where - /// the argument type is `Copy` and small enough to be more efficient to always - /// pass by value. - /// - /// **Why is this bad?** In many calling conventions instances of structs will - /// be passed through registers if they fit into two or less general purpose - /// registers. - /// - /// **Known problems:** This lint is target register size dependent, it is - /// limited to 32-bit to try and reduce portability problems between 32 and - /// 64-bit, but if you are compiling for 8 or 16-bit targets then the limit - /// will be different. - /// - /// The configuration option `trivial_copy_size_limit` can be set to override - /// this limit for a project. - /// - /// This lint attempts to allow passing arguments by reference if a reference - /// to that argument is returned. This is implemented by comparing the lifetime - /// of the argument and return value for equality. However, this can cause - /// false positives in cases involving multiple lifetimes that are bounded by - /// each other. - /// - /// **Example:** - /// - /// ```rust - /// // Bad - /// fn foo(v: &u32) {} - /// ``` - /// - /// ```rust - /// // Better - /// fn foo(v: u32) {} - /// ``` - pub TRIVIALLY_COPY_PASS_BY_REF, - pedantic, - "functions taking small copyable arguments by reference" -} - -#[derive(Copy, Clone)] -pub struct TriviallyCopyPassByRef { - limit: u64, -} - -impl<'tcx> TriviallyCopyPassByRef { - pub fn new(limit: Option, target: &Target) -> Self { - let limit = limit.unwrap_or_else(|| { - let bit_width = u64::from(target.pointer_width); - // Cap the calculated bit width at 32-bits to reduce - // portability problems between 32 and 64-bit targets - let bit_width = cmp::min(bit_width, 32); - #[allow(clippy::integer_division)] - let byte_width = bit_width / 8; - // Use a limit of 2 times the register byte width - byte_width * 2 - }); - Self { limit } - } - - fn check_poly_fn(&mut self, cx: &LateContext<'tcx>, hir_id: HirId, decl: &FnDecl<'_>, span: Option) { - let fn_def_id = cx.tcx.hir().local_def_id(hir_id); - - let fn_sig = cx.tcx.fn_sig(fn_def_id); - let fn_sig = cx.tcx.erase_late_bound_regions(&fn_sig); - - // Use lifetimes to determine if we're returning a reference to the - // argument. In that case we can't switch to pass-by-value as the - // argument will not live long enough. - let output_lts = match *fn_sig.output().kind() { - ty::Ref(output_lt, _, _) => vec![output_lt], - ty::Adt(_, substs) => substs.regions().collect(), - _ => vec![], - }; - - for (input, &ty) in decl.inputs.iter().zip(fn_sig.inputs()) { - // All spans generated from a proc-macro invocation are the same... - match span { - Some(s) if s == input.span => return, - _ => (), - } - - if_chain! { - if let ty::Ref(input_lt, ty, Mutability::Not) = ty.kind(); - if !output_lts.contains(&input_lt); - if is_copy(cx, ty); - if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes()); - if size <= self.limit; - if let hir::TyKind::Rptr(_, MutTy { ty: ref decl_ty, .. }) = input.kind; - then { - let value_type = if is_self_ty(decl_ty) { - "self".into() - } else { - snippet(cx, decl_ty.span, "_").into() - }; - span_lint_and_sugg( - cx, - TRIVIALLY_COPY_PASS_BY_REF, - input.span, - &format!("this argument ({} byte) is passed by reference, but would be more efficient if passed by value (limit: {} byte)", size, self.limit), - "consider passing by value instead", - value_type, - Applicability::Unspecified, - ); - } - } - } - } -} - -impl_lint_pass!(TriviallyCopyPassByRef => [TRIVIALLY_COPY_PASS_BY_REF]); - -impl<'tcx> LateLintPass<'tcx> for TriviallyCopyPassByRef { - fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) { - if item.span.from_expansion() { - return; - } - - if let hir::TraitItemKind::Fn(method_sig, _) = &item.kind { - self.check_poly_fn(cx, item.hir_id, &*method_sig.decl, None); - } - } - - fn check_fn( - &mut self, - cx: &LateContext<'tcx>, - kind: FnKind<'tcx>, - decl: &'tcx FnDecl<'_>, - _body: &'tcx Body<'_>, - span: Span, - hir_id: HirId, - ) { - if span.from_expansion() { - return; - } - - match kind { - FnKind::ItemFn(.., header, _, attrs) => { - if header.abi != Abi::Rust { - return; - } - for a in attrs { - if let Some(meta_items) = a.meta_item_list() { - if a.has_name(sym!(proc_macro_derive)) - || (a.has_name(sym!(inline)) && attr::list_contains_name(&meta_items, sym!(always))) - { - return; - } - } - } - }, - FnKind::Method(..) => (), - FnKind::Closure(..) => return, - } - - // Exclude non-inherent impls - if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. } | - ItemKind::Trait(..)) - { - return; - } - } - - self.check_poly_fn(cx, hir_id, decl, Some(span)); - } -} diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index dd2fd0bb445f..0ac8fff69f05 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -150,6 +150,8 @@ define_Conf! { (literal_representation_threshold, "literal_representation_threshold": u64, 16384), /// Lint: TRIVIALLY_COPY_PASS_BY_REF. The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by reference. (trivial_copy_size_limit, "trivial_copy_size_limit": Option, None), + /// Lint: LARGE_TYPE_PASS_BY_MOVE. The minimum size (in bytes) to consider a type for passing by reference instead of by value. + (pass_by_value_size_limit, "pass_by_value_size_limit": u64, 256), /// Lint: TOO_MANY_LINES. The maximum number of lines a function or method can have (too_many_lines_threshold, "too_many_lines_threshold": u64, 100), /// Lint: LARGE_STACK_ARRAYS, LARGE_CONST_ARRAYS. The maximum allowed size for arrays on the stack diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 6301d623a2b1..f3536f263397 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1061,6 +1061,13 @@ vec![ deprecation: None, module: "large_stack_arrays", }, + Lint { + name: "large_types_passed_by_value", + group: "pedantic", + desc: "functions taking large arguments by value", + deprecation: None, + module: "pass_by_ref_or_value", + }, Lint { name: "len_without_is_empty", group: "style", @@ -2389,7 +2396,7 @@ vec![ group: "pedantic", desc: "functions taking small copyable arguments by reference", deprecation: None, - module: "trivially_copy_pass_by_ref", + module: "pass_by_ref_or_value", }, Lint { name: "try_err", diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 103ec27e7d75..a58e7e918e2f 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -1,4 +1,4 @@ -error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `third-party` at line 5 column 1 +error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `third-party` at line 5 column 1 error: aborting due to previous error diff --git a/tests/ui/large_types_passed_by_value.rs b/tests/ui/large_types_passed_by_value.rs new file mode 100644 index 000000000000..e4a2e9df4d7b --- /dev/null +++ b/tests/ui/large_types_passed_by_value.rs @@ -0,0 +1,66 @@ +// normalize-stderr-test "\(\d+ byte\)" -> "(N byte)" +// normalize-stderr-test "\(limit: \d+ byte\)" -> "(limit: N byte)" + +#![warn(clippy::large_types_passed_by_value)] + +pub struct Large([u8; 2048]); + +#[derive(Clone, Copy)] +pub struct LargeAndCopy([u8; 2048]); + +pub struct Small([u8; 4]); + +#[derive(Clone, Copy)] +pub struct SmallAndCopy([u8; 4]); + +fn small(a: Small, b: SmallAndCopy) {} +fn not_copy(a: Large) {} +fn by_ref(a: &Large, b: &LargeAndCopy) {} +fn mutable(mut a: LargeAndCopy) {} +fn bad(a: LargeAndCopy) {} +pub fn bad_but_pub(a: LargeAndCopy) {} + +impl LargeAndCopy { + fn self_is_ok(self) {} + fn other_is_not_ok(self, other: LargeAndCopy) {} + fn unless_other_can_change(self, mut other: LargeAndCopy) {} + pub fn or_were_in_public(self, other: LargeAndCopy) {} +} + +trait LargeTypeDevourer { + fn devoure_array(&self, array: [u8; 6666]); + fn devoure_tuple(&self, tup: (LargeAndCopy, LargeAndCopy)); + fn devoure_array_and_tuple_wow(&self, array: [u8; 6666], tup: (LargeAndCopy, LargeAndCopy)); +} + +pub trait PubLargeTypeDevourer { + fn devoure_array_in_public(&self, array: [u8; 6666]); +} + +struct S {} +impl LargeTypeDevourer for S { + fn devoure_array(&self, array: [u8; 6666]) { + todo!(); + } + fn devoure_tuple(&self, tup: (LargeAndCopy, LargeAndCopy)) { + todo!(); + } + fn devoure_array_and_tuple_wow(&self, array: [u8; 6666], tup: (LargeAndCopy, LargeAndCopy)) { + todo!(); + } +} + +#[inline(always)] +fn foo_always(x: LargeAndCopy) { + todo!(); +} +#[inline(never)] +fn foo_never(x: LargeAndCopy) { + todo!(); +} +#[inline] +fn foo(x: LargeAndCopy) { + todo!(); +} + +fn main() {} diff --git a/tests/ui/large_types_passed_by_value.stderr b/tests/ui/large_types_passed_by_value.stderr new file mode 100644 index 000000000000..5f42dcfb9b52 --- /dev/null +++ b/tests/ui/large_types_passed_by_value.stderr @@ -0,0 +1,52 @@ +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:20:11 + | +LL | fn bad(a: LargeAndCopy) {} + | ^^^^^^^^^^^^ help: consider passing by reference instead: `&LargeAndCopy` + | + = note: `-D clippy::large-types-passed-by-value` implied by `-D warnings` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:25:37 + | +LL | fn other_is_not_ok(self, other: LargeAndCopy) {} + | ^^^^^^^^^^^^ help: consider passing by reference instead: `&LargeAndCopy` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:31:36 + | +LL | fn devoure_array(&self, array: [u8; 6666]); + | ^^^^^^^^^^ help: consider passing by reference instead: `&[u8; 6666]` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:32:34 + | +LL | fn devoure_tuple(&self, tup: (LargeAndCopy, LargeAndCopy)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider passing by reference instead: `&(LargeAndCopy, LargeAndCopy)` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:33:50 + | +LL | fn devoure_array_and_tuple_wow(&self, array: [u8; 6666], tup: (LargeAndCopy, LargeAndCopy)); + | ^^^^^^^^^^ help: consider passing by reference instead: `&[u8; 6666]` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:33:67 + | +LL | fn devoure_array_and_tuple_wow(&self, array: [u8; 6666], tup: (LargeAndCopy, LargeAndCopy)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider passing by reference instead: `&(LargeAndCopy, LargeAndCopy)` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:58:17 + | +LL | fn foo_never(x: LargeAndCopy) { + | ^^^^^^^^^^^^ help: consider passing by reference instead: `&LargeAndCopy` + +error: this argument (N byte) is passed by value, but might be more efficient if passed by reference (limit: N byte) + --> $DIR/large_types_passed_by_value.rs:62:11 + | +LL | fn foo(x: LargeAndCopy) { + | ^^^^^^^^^^^^ help: consider passing by reference instead: `&LargeAndCopy` + +error: aborting due to 8 previous errors +