Merge pull request #2951 from etaoins/allow-pass-by-ref-on-ref-return

Allow pass by reference if we return a reference

Author: flip1995

clippy_lints/src/trivially_copy_pass_by_ref.rs

@@ -31,6 +31,12 @@ use crate::utils::{in_macro, is_copy, is_self, span_lint_and_sugg, snippet};
 /// 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
 /// fn foo(v: &u32) {
@@ -115,14 +121,24 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TriviallyCopyPassByRef {
         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_lt = if let TypeVariants::TyRef(output_lt, _, _) = fn_sig.output().sty {
+            Some(output_lt)
+        } else {
+            None
+        };
+
         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 let TypeVariants::TyRef(input_lt, ty, Mutability::MutImmutable) = ty.sty;
+                if Some(input_lt) != output_lt;
                 if is_copy(cx, ty);
                 if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes());
                 if size <= self.limit;

tests/ui/trivially_copy_pass_by_ref.rs

@@ -11,6 +11,15 @@ type Baz = u32;
 fn good(a: &mut u32, b: u32, c: &Bar) {
 }
 
+fn good_return_implicit_lt_ref(foo: &Foo) -> &u32 {
+    &foo.0
+}
+
+#[allow(needless_lifetimes)]
+fn good_return_explicit_lt_ref<'a>(foo: &'a Foo) -> &'a u32 {
+    &foo.0
+}
+
 fn bad(x: &u32, y: &Foo, z: &Baz) {
 }
 
@@ -46,6 +55,8 @@ 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);
+    good_return_implicit_lt_ref(&y);
+    good_return_explicit_lt_ref(&y);
     bad(&x, &y, &z);
     foo.good(&mut a, b, &c);
     foo.good2();

tests/ui/trivially_copy_pass_by_ref.stderr

@@ -1,81 +1,81 @@
 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
+  --> $DIR/trivially_copy_pass_by_ref.rs:23:11
    |
-14 | fn bad(x: &u32, y: &Foo, z: &Baz) {
+23 | 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
+  --> $DIR/trivially_copy_pass_by_ref.rs:23:20
    |
-14 | fn bad(x: &u32, y: &Foo, z: &Baz) {
+23 | 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
+  --> $DIR/trivially_copy_pass_by_ref.rs:23:29
    |
-14 | fn bad(x: &u32, y: &Foo, z: &Baz) {
+23 | 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
+  --> $DIR/trivially_copy_pass_by_ref.rs:33:12
    |
-24 |     fn bad(&self, x: &u32, y: &Foo, z: &Baz) {
+33 |     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
+  --> $DIR/trivially_copy_pass_by_ref.rs:33:22
    |
-24 |     fn bad(&self, x: &u32, y: &Foo, z: &Baz) {
+33 |     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
+  --> $DIR/trivially_copy_pass_by_ref.rs:33:31
    |
-24 |     fn bad(&self, x: &u32, y: &Foo, z: &Baz) {
+33 |     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
+  --> $DIR/trivially_copy_pass_by_ref.rs:33:40
    |
-24 |     fn bad(&self, x: &u32, y: &Foo, z: &Baz) {
+33 |     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
+  --> $DIR/trivially_copy_pass_by_ref.rs:36:16
    |
-27 |     fn bad2(x: &u32, y: &Foo, z: &Baz) {
+36 |     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
+  --> $DIR/trivially_copy_pass_by_ref.rs:36:25
    |
-27 |     fn bad2(x: &u32, y: &Foo, z: &Baz) {
+36 |     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
+  --> $DIR/trivially_copy_pass_by_ref.rs:36:34
    |
-27 |     fn bad2(x: &u32, y: &Foo, z: &Baz) {
+36 |     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
+  --> $DIR/trivially_copy_pass_by_ref.rs:50:16
    |
-41 |     fn bad2(x: &u32, y: &Foo, z: &Baz) {
+50 |     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
+  --> $DIR/trivially_copy_pass_by_ref.rs:50:25
    |
-41 |     fn bad2(x: &u32, y: &Foo, z: &Baz) {
+50 |     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
+  --> $DIR/trivially_copy_pass_by_ref.rs:50:34
    |
-41 |     fn bad2(x: &u32, y: &Foo, z: &Baz) {
+50 |     fn bad2(x: &u32, y: &Foo, z: &Baz) {
    |                                  ^^^^ help: consider passing by value instead: `Baz`
 
 error: aborting due to 13 previous errors