New Lint: Pass small trivially copyable objects by value

Fixes #1680

Hardcoded for 64-bit "trivial" size for now

Author: Nemo157

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,
     ]);

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

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

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() {

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)]

tests/ui/infinite_loop.rs

@@ -1,4 +1,4 @@
-
+#![allow(trivially_copy_pass_by_ref)]
 
 
 fn fn_val(i: i32) -> i32 { unimplemented!() }

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

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;

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

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
 }