diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 52145341400..56521d440aa 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -196,6 +196,7 @@ pub mod suspicious_trait_impl; pub mod swap; pub mod temporary_assignment; pub mod transmute; +pub mod trivially_copy_pass_by_ref; pub mod types; pub mod unicode; pub mod unsafe_removed_from_name; @@ -399,6 +400,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box large_enum_variant::LargeEnumVariant::new(conf.enum_variant_size_threshold)); reg.register_late_lint_pass(box explicit_write::Pass); reg.register_late_lint_pass(box needless_pass_by_value::NeedlessPassByValue); + reg.register_late_lint_pass(box trivially_copy_pass_by_ref::TriviallyCopyPassByRef); reg.register_early_lint_pass(box literal_representation::LiteralDigitGrouping); reg.register_early_lint_pass(box literal_representation::LiteralRepresentation::new( conf.literal_representation_threshold @@ -672,6 +674,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { transmute::TRANSMUTE_PTR_TO_REF, transmute::USELESS_TRANSMUTE, transmute::WRONG_TRANSMUTE, + trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF, types::ABSURD_EXTREME_COMPARISONS, types::BORROWED_BOX, types::BOX_VEC, @@ -916,6 +919,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { methods::SINGLE_CHAR_PATTERN, misc::CMP_OWNED, mutex_atomic::MUTEX_ATOMIC, + trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF, types::BOX_VEC, vec::USELESS_VEC, ]); diff --git a/clippy_lints/src/trivially_copy_pass_by_ref.rs b/clippy_lints/src/trivially_copy_pass_by_ref.rs new file mode 100644 index 00000000000..30b5f65cc8c --- /dev/null +++ b/clippy_lints/src/trivially_copy_pass_by_ref.rs @@ -0,0 +1,116 @@ +use rustc::hir::*; +use rustc::hir::map::*; +use rustc::hir::intravisit::FnKind; +use rustc::lint::*; +use rustc::ty::TypeVariants; +use rustc_target::spec::abi::Abi; +use rustc_target::abi::LayoutOf; +use syntax::ast::NodeId; +use syntax_pos::Span; +use crate::utils::{in_macro, is_copy, is_self, span_lint_and_sugg, snippet}; + +/// **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. +/// +/// **Example:** +/// ```rust +/// fn foo(v: &u32) { +/// assert_eq!(v, 42); +/// } +/// // should be +/// fn foo(v: u32) { +/// assert_eq!(v, 42); +/// } +/// ``` +declare_clippy_lint! { + pub TRIVIALLY_COPY_PASS_BY_REF, + perf, + "functions taking small copyable arguments by reference" +} + +pub struct TriviallyCopyPassByRef; + +impl LintPass for TriviallyCopyPassByRef { + fn get_lints(&self) -> LintArray { + lint_array![TRIVIALLY_COPY_PASS_BY_REF] + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TriviallyCopyPassByRef { + fn check_fn( + &mut self, + cx: &LateContext<'a, 'tcx>, + kind: FnKind<'tcx>, + decl: &'tcx FnDecl, + body: &'tcx Body, + span: Span, + node_id: NodeId, + ) { + if in_macro(span) { + return; + } + + match kind { + FnKind::ItemFn(.., abi, _, attrs) => { + if abi != Abi::Rust { + return; + } + for a in attrs { + if a.meta_item_list().is_some() && a.name() == "proc_macro_derive" { + return; + } + } + }, + FnKind::Method(..) => (), + _ => return, + } + + // Exclude non-inherent impls + if let Some(NodeItem(item)) = cx.tcx.hir.find(cx.tcx.hir.get_parent_node(node_id)) { + if matches!(item.node, ItemImpl(_, _, _, _, Some(_), _, _) | + ItemTrait(..)) + { + return; + } + } + + let fn_def_id = cx.tcx.hir.local_def_id(node_id); + + let fn_sig = cx.tcx.fn_sig(fn_def_id); + let fn_sig = cx.tcx.erase_late_bound_regions(&fn_sig); + + for ((input, &ty), arg) in decl.inputs.iter().zip(fn_sig.inputs()).zip(&body.arguments) { + // All spans generated from a proc-macro invocation are the same... + if span == input.span { + return; + } + + if_chain! { + if let TypeVariants::TyRef(_, ty, Mutability::MutImmutable) = ty.sty; + if is_copy(cx, ty); + if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes()); + if size < 16; + if let Ty_::TyRptr(_, MutTy { ty: ref decl_ty, .. }) = input.node; + then { + let value_type = if is_self(arg) { + "self".into() + } else { + snippet(cx, decl_ty.span, "_").into() + }; + span_lint_and_sugg( + cx, + TRIVIALLY_COPY_PASS_BY_REF, + input.span, + "this argument is passed by reference, but would be more efficient if passed by value", + "consider passing by value instead", + value_type); + } + } + } + } +} diff --git a/tests/ui/clone_on_copy_mut.rs b/tests/ui/clone_on_copy_mut.rs index 5bfa256623b..5b491573c3f 100644 --- a/tests/ui/clone_on_copy_mut.rs +++ b/tests/ui/clone_on_copy_mut.rs @@ -5,6 +5,7 @@ pub fn dec_read_dec(i: &mut i32) -> i32 { ret } +#[allow(trivially_copy_pass_by_ref)] pub fn minus_1(i: &i32) -> i32 { dec_read_dec(&mut i.clone()) } diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs index be84f44bfb1..6e0b6f8cacd 100644 --- a/tests/ui/eta.rs +++ b/tests/ui/eta.rs @@ -1,6 +1,6 @@ -#![allow(unknown_lints, unused, no_effect, redundant_closure_call, many_single_char_names, needless_pass_by_value, option_map_unit_fn)] +#![allow(unknown_lints, unused, no_effect, redundant_closure_call, many_single_char_names, needless_pass_by_value, option_map_unit_fn, trivially_copy_pass_by_ref)] #![warn(redundant_closure, needless_borrow)] fn main() { diff --git a/tests/ui/infinite_iter.rs b/tests/ui/infinite_iter.rs index 08596ff2016..2e2ccd9f1ae 100644 --- a/tests/ui/infinite_iter.rs +++ b/tests/ui/infinite_iter.rs @@ -1,7 +1,7 @@ #![feature(iterator_for_each)] use std::iter::repeat; - +#[allow(trivially_copy_pass_by_ref)] fn square_is_lower_64(x: &u32) -> bool { x * x < 64 } #[allow(maybe_infinite_iter)] diff --git a/tests/ui/infinite_loop.rs b/tests/ui/infinite_loop.rs index 353d34134eb..aa4f8b53f6c 100644 --- a/tests/ui/infinite_loop.rs +++ b/tests/ui/infinite_loop.rs @@ -1,4 +1,4 @@ - +#![allow(trivially_copy_pass_by_ref)] fn fn_val(i: i32) -> i32 { unimplemented!() } diff --git a/tests/ui/lifetimes.rs b/tests/ui/lifetimes.rs index 1f6aeaafcf1..d2de1cb8ed8 100644 --- a/tests/ui/lifetimes.rs +++ b/tests/ui/lifetimes.rs @@ -2,7 +2,7 @@ #![warn(needless_lifetimes, extra_unused_lifetimes)] -#![allow(dead_code, needless_pass_by_value)] +#![allow(dead_code, needless_pass_by_value, trivially_copy_pass_by_ref)] fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) { } diff --git a/tests/ui/mut_from_ref.rs b/tests/ui/mut_from_ref.rs index 9e757155260..3fc464083c4 100644 --- a/tests/ui/mut_from_ref.rs +++ b/tests/ui/mut_from_ref.rs @@ -1,6 +1,6 @@ -#![allow(unused)] +#![allow(unused, trivially_copy_pass_by_ref)] #![warn(mut_from_ref)] struct Foo; diff --git a/tests/ui/mut_reference.rs b/tests/ui/mut_reference.rs index ac40bf2a186..34185f6a9c2 100644 --- a/tests/ui/mut_reference.rs +++ b/tests/ui/mut_reference.rs @@ -1,7 +1,7 @@ -#![allow(unused_variables)] +#![allow(unused_variables, trivially_copy_pass_by_ref)] fn takes_an_immutable_reference(a: &i32) {} fn takes_a_mutable_reference(a: &mut i32) {} diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index 491194e83b1..b086f0214a9 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; - +#[allow(trivially_copy_pass_by_ref)] fn x(y: &i32) -> i32 { *y } diff --git a/tests/ui/trivially_copy_pass_by_ref.rs b/tests/ui/trivially_copy_pass_by_ref.rs new file mode 100644 index 00000000000..aba4aa5ea32 --- /dev/null +++ b/tests/ui/trivially_copy_pass_by_ref.rs @@ -0,0 +1,57 @@ +#![allow(many_single_char_names, blacklisted_name)] + +#[derive(Copy, Clone)] +struct Foo(u32); + +#[derive(Copy, Clone)] +struct Bar([u8; 24]); + +type Baz = u32; + +fn good(a: &mut u32, b: u32, c: &Bar) { +} + +fn bad(x: &u32, y: &Foo, z: &Baz) { +} + +impl Foo { + fn good(self, a: &mut u32, b: u32, c: &Bar) { + } + + fn good2(&mut self) { + } + + fn bad(&self, x: &u32, y: &Foo, z: &Baz) { + } + + fn bad2(x: &u32, y: &Foo, z: &Baz) { + } +} + +impl AsRef for Foo { + fn as_ref(&self) -> &u32 { + &self.0 + } +} + +impl Bar { + fn good(&self, a: &mut u32, b: u32, c: &Bar) { + } + + fn bad2(x: &u32, y: &Foo, z: &Baz) { + } +} + +fn main() { + let (mut foo, bar) = (Foo(0), Bar([0; 24])); + let (mut a, b, c, x, y, z) = (0, 0, Bar([0; 24]), 0, Foo(0), 0); + good(&mut a, b, &c); + bad(&x, &y, &z); + foo.good(&mut a, b, &c); + foo.good2(); + foo.bad(&x, &y, &z); + Foo::bad2(&x, &y, &z); + bar.good(&mut a, b, &c); + Bar::bad2(&x, &y, &z); + foo.as_ref(); +} diff --git a/tests/ui/trivially_copy_pass_by_ref.stderr b/tests/ui/trivially_copy_pass_by_ref.stderr new file mode 100644 index 00000000000..c6ab968a7c5 --- /dev/null +++ b/tests/ui/trivially_copy_pass_by_ref.stderr @@ -0,0 +1,82 @@ +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:14:11 + | +14 | fn bad(x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `u32` + | + = note: `-D trivially-copy-pass-by-ref` implied by `-D warnings` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:14:20 + | +14 | fn bad(x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `Foo` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:14:29 + | +14 | fn bad(x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `Baz` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:24:12 + | +24 | fn bad(&self, x: &u32, y: &Foo, z: &Baz) { + | ^^^^^ help: consider passing by value instead: `self` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:24:22 + | +24 | fn bad(&self, x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `u32` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:24:31 + | +24 | fn bad(&self, x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `Foo` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:24:40 + | +24 | fn bad(&self, x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `Baz` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:27:16 + | +27 | fn bad2(x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `u32` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:27:25 + | +27 | fn bad2(x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `Foo` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:27:34 + | +27 | fn bad2(x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `Baz` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:41:16 + | +41 | fn bad2(x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `u32` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:41:25 + | +41 | fn bad2(x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `Foo` + +error: this argument is passed by reference, but would be more efficient if passed by value + --> $DIR/trivially_copy_pass_by_ref.rs:41:34 + | +41 | fn bad2(x: &u32, y: &Foo, z: &Baz) { + | ^^^^ help: consider passing by value instead: `Baz` + +error: aborting due to 13 previous errors + diff --git a/tests/ui/unused_lt.rs b/tests/ui/unused_lt.rs index 198730d87f3..8b166a34d29 100644 --- a/tests/ui/unused_lt.rs +++ b/tests/ui/unused_lt.rs @@ -1,6 +1,6 @@ -#![allow(unused, dead_code, needless_lifetimes, needless_pass_by_value)] +#![allow(unused, dead_code, needless_lifetimes, needless_pass_by_value, trivially_copy_pass_by_ref)] #![warn(extra_unused_lifetimes)] fn empty() { diff --git a/tests/ui/wrong_self_convention.rs b/tests/ui/wrong_self_convention.rs index bef87e2bb01..07a93d6889b 100644 --- a/tests/ui/wrong_self_convention.rs +++ b/tests/ui/wrong_self_convention.rs @@ -3,7 +3,7 @@ #![warn(wrong_self_convention)] #![warn(wrong_pub_self_convention)] -#![allow(dead_code)] +#![allow(dead_code, trivially_copy_pass_by_ref)] fn main() {}