mirror of
https://github.com/rust-lang/rust.git
synced 2024-12-26 15:36:39 +00:00
Auto merge of #6135 - cauebs:master, r=ebroto
Add large_types_passed_by_value lint Creates a new lint that checks for large function parameters passed by value. ~Types that are Copy were ignored, because I understand they are explicitly marked as being cheap (or just acceptable) to copy. Arrays were excluded from that restriction because they are always Copy, independently of the size, if the elements are Copy.~ Only Copy types are considered, because if a type is not Copy it cannot be dereferenced, as pointed out by `@ebroto,` and that would require analyzing the whole function body to check if the argument is moved somewhere else. `self` and `mut` parameters are also skipped. I refactored `trivially_copy_pass_by_ref` and the new lint into a new `pass_by_ref_or_value` module, since both share most of their implementations, as was pointed out to me in #4499. ~Some questions that remain:~ ~1. Should `self` be disregarded for this lint?~ ~2. Should we special case types like Vec and String when suggesting changes? (to slices and &str)~ ~3. What should be the default size limit?~ ~4. Are there any special cases I'm missing?~ I ask the maintainers to add the `hacktoberfest-accepted` label if this PR is decent, even if there are some minor details to work out, but I do intend to stick around and finish the job. Fixes #4499 changelog: Add new lint [`large_types_passed_by_value`]
This commit is contained in:
commit
5c78d26e5e
@ -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
|
||||
|
@ -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),
|
||||
|
256
clippy_lints/src/pass_by_ref_or_value.rs
Normal file
256
clippy_lints/src/pass_by_ref_or_value.rs
Normal file
@ -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<u64>, 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<Span>) {
|
||||
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));
|
||||
}
|
||||
}
|
@ -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<u64>, 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<Span>) {
|
||||
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));
|
||||
}
|
||||
}
|
@ -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<u64>, 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
|
||||
|
@ -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",
|
||||
|
@ -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
|
||||
|
||||
|
66
tests/ui/large_types_passed_by_value.rs
Normal file
66
tests/ui/large_types_passed_by_value.rs
Normal file
@ -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() {}
|
52
tests/ui/large_types_passed_by_value.stderr
Normal file
52
tests/ui/large_types_passed_by_value.stderr
Normal file
@ -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
|
||||
|
Loading…
Reference in New Issue
Block a user