diff --git a/clippy_lints/src/trivially_copy_pass_by_ref.rs b/clippy_lints/src/trivially_copy_pass_by_ref.rs index b6fd8db5157..6a048b19213 100644 --- a/clippy_lints/src/trivially_copy_pass_by_ref.rs +++ b/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,6 +121,15 @@ 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 { @@ -122,7 +137,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TriviallyCopyPassByRef { } 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 aba4aa5ea32..c6773add244 100644 --- a/tests/ui/trivially_copy_pass_by_ref.rs +++ b/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(); diff --git a/tests/ui/trivially_copy_pass_by_ref.stderr b/tests/ui/trivially_copy_pass_by_ref.stderr index c6ab968a7c5..db25cc5a020 100644 --- 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