Allow pass by reference if we return a reference

Currently this code will trigger `trivally_copy_pass_by_ref`:

```
struct OuterStruct {
    field: [u8; 8],
}

fn return_inner(outer: &OuterStruct) -> &[u8] {
    &outer.field
}
```

If we change the `outer` to be pass-by-value it will not live long
enough for us to return the reference. The above example is trivial but
I've hit this in real code that either returns a reference to either the
argument or in to `self`.

This suppresses the `trivally_copy_pass_by_ref` lint if we return a
reference and it has the same lifetime as the argument. This will likely
miss complex cases with multiple lifetimes bounded by each other but it
should cover the majority of cases with little effort.

diff --git a/clippy_lints/src/trivially_copy_pass_by_ref.rs b/clippy_lints/src/trivially_copy_pass_by_ref.rs
index b6fd8db51575b45182f3844e4d09e900f589afcd..e0eb464596b50cb81bc3cde313567ba140846d21 100644 (file)
--- a/clippy_lints/src/trivially_copy_pass_by_ref.rs
+++ b/clippy_lints/src/trivially_copy_pass_by_ref.rs
@@ -115,6 +115,14 @@ fn check_fn(
         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 {
@@ -122,7 +130,8 @@ fn check_fn(
             }
 
             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;

diff --git a/tests/ui/trivially_copy_pass_by_ref.rs b/tests/ui/trivially_copy_pass_by_ref.rs
index aba4aa5ea327bee3e51fcdbef32e2ca678c306a1..c6773add244305186f2b8657174633e260464ffe 100644 (file)
--- a/tests/ui/trivially_copy_pass_by_ref.rs
+++ b/tests/ui/trivially_copy_pass_by_ref.rs
@@ -11,6 +11,15 @@
 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();

diff --git a/tests/ui/trivially_copy_pass_by_ref.stderr b/tests/ui/trivially_copy_pass_by_ref.stderr
index c6ab968a7c5f97d29e58cc83afa35dd0d90635d6..db25cc5a02011b2a278ef7e580948c88a9dad3ab 100644 (file)
--- a/tests/ui/trivially_copy_pass_by_ref.stderr
+++ b/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