diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 239822f40856..0d71be61651d 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -488,11 +488,13 @@ fn has_where_lifetimes<'tcx>(cx: &LateContext<'tcx>, generics: &'tcx Generics<'_ false } +#[allow(clippy::struct_excessive_bools)] struct Usage { lifetime: Lifetime, in_where_predicate: bool, in_bounded_ty: bool, in_generics_arg: bool, + lifetime_elision_impossible: bool, } struct LifetimeChecker<'cx, 'tcx, F> { @@ -501,6 +503,7 @@ struct LifetimeChecker<'cx, 'tcx, F> { where_predicate_depth: usize, bounded_ty_depth: usize, generic_args_depth: usize, + lifetime_elision_impossible: bool, phantom: std::marker::PhantomData, } @@ -525,6 +528,7 @@ where where_predicate_depth: 0, bounded_ty_depth: 0, generic_args_depth: 0, + lifetime_elision_impossible: false, phantom: std::marker::PhantomData, } } @@ -566,6 +570,7 @@ where in_where_predicate: self.where_predicate_depth != 0, in_bounded_ty: self.bounded_ty_depth != 0, in_generics_arg: self.generic_args_depth != 0, + lifetime_elision_impossible: self.lifetime_elision_impossible, }); } } @@ -592,11 +597,44 @@ where self.generic_args_depth -= 1; } + fn visit_fn_decl(&mut self, fd: &'tcx FnDecl<'tcx>) -> Self::Result { + self.lifetime_elision_impossible = !is_candidate_for_elision(fd); + walk_fn_decl(self, fd); + self.lifetime_elision_impossible = false; + } + fn nested_visit_map(&mut self) -> Self::Map { self.cx.tcx.hir() } } +/// Check if `fd` supports function elision with an anonymous (or elided) lifetime, +/// and has a lifetime somewhere in its output type. +fn is_candidate_for_elision(fd: &FnDecl<'_>) -> bool { + struct V; + + impl Visitor<'_> for V { + type Result = ControlFlow; + + fn visit_lifetime(&mut self, lifetime: &Lifetime) -> Self::Result { + ControlFlow::Break(lifetime.is_elided() || lifetime.is_anonymous()) + } + } + + if fd.lifetime_elision_allowed + && let Return(ret_ty) = fd.output + && walk_ty(&mut V, ret_ty).is_break() + { + // The first encountered input lifetime will either be one on `self`, or will be the only lifetime. + fd.inputs + .iter() + .find_map(|ty| walk_ty(&mut V, ty).break_value()) + .unwrap() + } else { + false + } +} + fn report_extra_lifetimes<'tcx>(cx: &LateContext<'tcx>, func: &'tcx FnDecl<'_>, generics: &'tcx Generics<'_>) { let mut checker = LifetimeChecker::::new(cx, generics); @@ -662,6 +700,7 @@ fn report_elidable_impl_lifetimes<'tcx>( Usage { lifetime, in_where_predicate: false, + lifetime_elision_impossible: false, .. }, ] = usages.as_slice() diff --git a/tests/ui/needless_lifetimes.fixed b/tests/ui/needless_lifetimes.fixed index 8196d608abd2..77188254316a 100644 --- a/tests/ui/needless_lifetimes.fixed +++ b/tests/ui/needless_lifetimes.fixed @@ -576,4 +576,85 @@ mod issue13749bis { impl<'a, T: 'a> Generic {} } +mod issue13923 { + struct Py<'py> { + data: &'py str, + } + + enum Content<'t, 'py> { + Py(Py<'py>), + T1(&'t str), + T2(&'t str), + } + + enum ContentString<'t> { + T1(&'t str), + T2(&'t str), + } + + impl<'t, 'py> ContentString<'t> { + // `'py` cannot be elided + fn map_content1(self, f: impl FnOnce(&'t str) -> &'t str) -> Content<'t, 'py> { + match self { + Self::T1(content) => Content::T1(f(content)), + Self::T2(content) => Content::T2(f(content)), + } + } + } + + impl<'t> ContentString<'t> { + // `'py` can be elided because of `&self` + fn map_content2(&self, f: impl FnOnce(&'t str) -> &'t str) -> Content<'t, '_> { + match self { + Self::T1(content) => Content::T1(f(content)), + Self::T2(content) => Content::T2(f(content)), + } + } + } + + impl<'t> ContentString<'t> { + // `'py` can be elided because of `&'_ self` + fn map_content3(&'_ self, f: impl FnOnce(&'t str) -> &'t str) -> Content<'t, '_> { + match self { + Self::T1(content) => Content::T1(f(content)), + Self::T2(content) => Content::T2(f(content)), + } + } + } + + impl<'t, 'py> ContentString<'t> { + // `'py` should not be elided as the default lifetime, even if working, could be named as `'t` + fn map_content4(self, f: impl FnOnce(&'t str) -> &'t str, o: &'t str) -> Content<'t, 'py> { + match self { + Self::T1(content) => Content::T1(f(content)), + Self::T2(_) => Content::T2(o), + } + } + } + + impl<'t> ContentString<'t> { + // `'py` can be elided because of `&Self` + fn map_content5( + self: std::pin::Pin<&Self>, + f: impl FnOnce(&'t str) -> &'t str, + o: &'t str, + ) -> Content<'t, '_> { + match *self { + Self::T1(content) => Content::T1(f(content)), + Self::T2(_) => Content::T2(o), + } + } + } + + struct Cx<'a, 'b> { + a: &'a u32, + b: &'b u32, + } + + // `'c` cannot be elided because we have several input lifetimes + fn one_explicit<'b>(x: Cx<'_, 'b>) -> &'b u32 { + x.b + } +} + fn main() {} diff --git a/tests/ui/needless_lifetimes.rs b/tests/ui/needless_lifetimes.rs index b55dd99c46d0..c74121d0d7b8 100644 --- a/tests/ui/needless_lifetimes.rs +++ b/tests/ui/needless_lifetimes.rs @@ -576,4 +576,85 @@ mod issue13749bis { impl<'a, T: 'a> Generic {} } +mod issue13923 { + struct Py<'py> { + data: &'py str, + } + + enum Content<'t, 'py> { + Py(Py<'py>), + T1(&'t str), + T2(&'t str), + } + + enum ContentString<'t> { + T1(&'t str), + T2(&'t str), + } + + impl<'t, 'py> ContentString<'t> { + // `'py` cannot be elided + fn map_content1(self, f: impl FnOnce(&'t str) -> &'t str) -> Content<'t, 'py> { + match self { + Self::T1(content) => Content::T1(f(content)), + Self::T2(content) => Content::T2(f(content)), + } + } + } + + impl<'t, 'py> ContentString<'t> { + // `'py` can be elided because of `&self` + fn map_content2(&self, f: impl FnOnce(&'t str) -> &'t str) -> Content<'t, 'py> { + match self { + Self::T1(content) => Content::T1(f(content)), + Self::T2(content) => Content::T2(f(content)), + } + } + } + + impl<'t, 'py> ContentString<'t> { + // `'py` can be elided because of `&'_ self` + fn map_content3(&'_ self, f: impl FnOnce(&'t str) -> &'t str) -> Content<'t, 'py> { + match self { + Self::T1(content) => Content::T1(f(content)), + Self::T2(content) => Content::T2(f(content)), + } + } + } + + impl<'t, 'py> ContentString<'t> { + // `'py` should not be elided as the default lifetime, even if working, could be named as `'t` + fn map_content4(self, f: impl FnOnce(&'t str) -> &'t str, o: &'t str) -> Content<'t, 'py> { + match self { + Self::T1(content) => Content::T1(f(content)), + Self::T2(_) => Content::T2(o), + } + } + } + + impl<'t, 'py> ContentString<'t> { + // `'py` can be elided because of `&Self` + fn map_content5( + self: std::pin::Pin<&Self>, + f: impl FnOnce(&'t str) -> &'t str, + o: &'t str, + ) -> Content<'t, 'py> { + match *self { + Self::T1(content) => Content::T1(f(content)), + Self::T2(_) => Content::T2(o), + } + } + } + + struct Cx<'a, 'b> { + a: &'a u32, + b: &'b u32, + } + + // `'c` cannot be elided because we have several input lifetimes + fn one_explicit<'b>(x: Cx<'_, 'b>) -> &'b u32 { + &x.b + } +} + fn main() {} diff --git a/tests/ui/needless_lifetimes.stderr b/tests/ui/needless_lifetimes.stderr index e56c914cc86d..5a739201d3d0 100644 --- a/tests/ui/needless_lifetimes.stderr +++ b/tests/ui/needless_lifetimes.stderr @@ -576,5 +576,64 @@ LL - fn one_input<'a>(x: &'a u8) -> &'a u8 { LL + fn one_input(x: &u8) -> &u8 { | -error: aborting due to 48 previous errors +error: the following explicit lifetimes could be elided: 'py + --> tests/ui/needless_lifetimes.rs:605:14 + | +LL | impl<'t, 'py> ContentString<'t> { + | ^^^ +LL | // `'py` can be elided because of `&self` +LL | fn map_content2(&self, f: impl FnOnce(&'t str) -> &'t str) -> Content<'t, 'py> { + | ^^^ + | +help: elide the lifetimes + | +LL ~ impl<'t> ContentString<'t> { +LL | // `'py` can be elided because of `&self` +LL ~ fn map_content2(&self, f: impl FnOnce(&'t str) -> &'t str) -> Content<'t, '_> { + | + +error: the following explicit lifetimes could be elided: 'py + --> tests/ui/needless_lifetimes.rs:615:14 + | +LL | impl<'t, 'py> ContentString<'t> { + | ^^^ +LL | // `'py` can be elided because of `&'_ self` +LL | fn map_content3(&'_ self, f: impl FnOnce(&'t str) -> &'t str) -> Content<'t, 'py> { + | ^^^ + | +help: elide the lifetimes + | +LL ~ impl<'t> ContentString<'t> { +LL | // `'py` can be elided because of `&'_ self` +LL ~ fn map_content3(&'_ self, f: impl FnOnce(&'t str) -> &'t str) -> Content<'t, '_> { + | + +error: the following explicit lifetimes could be elided: 'py + --> tests/ui/needless_lifetimes.rs:635:14 + | +LL | impl<'t, 'py> ContentString<'t> { + | ^^^ +... +LL | ) -> Content<'t, 'py> { + | ^^^ + | +help: elide the lifetimes + | +LL ~ impl<'t> ContentString<'t> { +LL | // `'py` can be elided because of `&Self` +... +LL | o: &'t str, +LL ~ ) -> Content<'t, '_> { + | + +error: this expression creates a reference which is immediately dereferenced by the compiler + --> tests/ui/needless_lifetimes.rs:656:9 + | +LL | &x.b + | ^^^^ help: change this to: `x.b` + | + = note: `-D clippy::needless-borrow` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::needless_borrow)]` + +error: aborting due to 52 previous errors