Do not propose to elide lifetimes if this causes an ambiguity
Some lifetimes in function return types are not bound to concrete content and can be set arbitrarily. Clippy should not propose to replace them by the default `'_` lifetime if such a lifetime cannot be determined unambigously.
This commit is contained in:
Samuel Tardieu
parent
034f3d224c
commit
c686ffd193
4 changed files with 261 additions and 1 deletions
|
|
@ -482,11 +482,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> {
|
||||
|
|
@ -495,6 +497,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<F>,
|
||||
}
|
||||
|
||||
|
|
@ -519,6 +522,7 @@ where
|
|||
where_predicate_depth: 0,
|
||||
bounded_ty_depth: 0,
|
||||
generic_args_depth: 0,
|
||||
lifetime_elision_impossible: false,
|
||||
phantom: std::marker::PhantomData,
|
||||
}
|
||||
}
|
||||
|
|
@ -560,6 +564,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,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -586,11 +591,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<bool>;
|
||||
|
||||
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::<hir_nested_filter::None>::new(cx, generics);
|
||||
|
||||
|
|
@ -656,6 +694,7 @@ fn report_elidable_impl_lifetimes<'tcx>(
|
|||
Usage {
|
||||
lifetime,
|
||||
in_where_predicate: false,
|
||||
lifetime_elision_impossible: false,
|
||||
..
|
||||
},
|
||||
] = usages.as_slice()
|
||||
|
|
|
|||
|
|
@ -576,4 +576,85 @@ mod issue13749bis {
|
|||
impl<'a, T: 'a> Generic<T> {}
|
||||
}
|
||||
|
||||
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() {}
|
||||
|
|
|
|||
|
|
@ -576,4 +576,85 @@ mod issue13749bis {
|
|||
impl<'a, T: 'a> Generic<T> {}
|
||||
}
|
||||
|
||||
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() {}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue