Rust-clippy: Incorrect needless_lifetimes warning for impl Trait based code

Created on 20 Jul 2018  路  8Comments  路  Source: rust-lang/rust-clippy

Given the following code:

#![crate_type = "lib"]
#![allow(unused)]
use std::collections::{HashMap, HashSet};

struct MyMap {
    inner: HashMap<u32, HashSet<String>>,
}

impl MyMap {
    fn iter<'a>(&'a self, key: u32) -> Option<impl Iterator<Item = impl AsRef<str> + 'a>> {
        self.inner.get(&key).map(|set| set.iter())
    }
}

fn main() {}

Playground

clippy reports the following:

 Checking playground v0.0.1 (file:///playground)
warning: explicit lifetimes given in parameter types where they could be elided
  --> src/main.rs:10:5
   |
10 | /     fn iter<'a>(&'a self, key: u32) -> Option<impl Iterator<Item = impl AsRef<str> + 'a>> {
11 | |         self.inner.get(&key).map(|set| set.iter())
12 | |     }
   | |_____^
   |
   = note: #[warn(needless_lifetimes)] on by default
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#needless_lifetimes

    Finished dev [unoptimized + debuginfo] target(s) in 1.40s

But if you take its suggestion and remove 'a from iter(), you get the following compiler error:

 Compiling playground v0.0.1 (file:///playground)
error[E0495]: cannot infer an appropriate lifetime for autoref due to conflicting requirements
  --> src/lib.rs:11:20
   |
11 |         self.inner.get(&key).map(|set| set.iter())
   |                    ^^^
   |
note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the method body at 10:5...
  --> src/lib.rs:10:5
   |
10 | /     fn iter(&self, key: u32) -> Option<impl Iterator<Item = impl AsRef<str>>> {
11 | |         self.inner.get(&key).map(|set| set.iter())
12 | |     }
   | |_____^
note: ...so that reference does not outlive borrowed content
  --> src/lib.rs:11:9
   |
11 |         self.inner.get(&key).map(|set| set.iter())
   |         ^^^^^^^^^^
   = note: but, the lifetime must be valid for the static lifetime...
note: ...so that return value is valid for the call
  --> src/lib.rs:10:40
   |
10 |     fn iter(&self, key: u32) -> Option<impl Iterator<Item = impl AsRef<str>>> {
   |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0495`.
error: Could not compile `playground`.

To learn more, run the command again with --verbose.

It's possible this is a dupe of an existing issue but a cursory search revealed a few old (and closed) impl Trait related bugs, and I didn't find anything current.

L-bug

Most helpful comment

There are two distinct cases in this issue.

case 1, the original one in the description:

impl MyMap {
    fn iter<'a>(&'a self, key: u32) -> Option<impl Iterator<Item = impl AsRef<str> + 'a>> {
        self.inner.get(&key).map(|set| set.iter())
    }
}

In this case, the lint is actually correct. Removing the lifetime notations produces an error, but the compiler error has improved:

...
help: to declare that the `impl Trait` captures data from argument `self`, you can add an explicit `'_` lifetime bound
   |
10 |     fn iter(& self, key: u32) -> Option<impl Iterator<Item = impl AsRef<str>> + '_> {
...

So the following version with anonymous lifetime compiles fine.

impl MyMap {
    fn iter(&self, key: u32) -> Option<impl Iterator<Item = impl AsRef<str> + '_>> {
        self.inner.get(&key).map(|set| set.iter())
    }
}

Since the lint is not making any wrong suggestion here, I would leave it as is. The compiler error will help the user to find the correct elided syntax.

case 2: the next examples commented in the issue boil down to this:

fn get_x<'a>(i32_pointer: &'a i32) -> impl Fn() -> &'a i32 {
    move || i32_pointer
}

Lifetime elision applies to three constructs: function items, function pointers, and closure trait bounds. This case has a nested elision candidate in impl Fn() -> &'a i32 (a closure trait bound). If 'a explicit lifetimes are removed, the compiler infers lifetimes for impl Fn() -> &i32 separately and will assign the 'static lifetime to the output reference &'static i32.

One solution to avoid FPs here is to bail out if a function pointer or a closure trait bound is found in the function signature. That might introduce a few FNs though.

All 8 comments

I think I have also encountered this issue, with a more minimal example:

struct Context {
    x: i32,
}

impl Context {
    fn get_x<'a>(&'a self) -> impl Fn() -> &'a i32 {
        move || &self.x
    }
}

Playground

With a little guidance, I'm happy to make a PR to fix the issue.

The fix needs to be made in the
https://github.com/rust-lang/rust-clippy/blob/45d24fd6bf45a50c1833a3ebc535c611fa0f785b/clippy_lints/src/lifetimes.rs#L150-L156
function.

This function assumes, that elision is possible, if
https://github.com/rust-lang/rust-clippy/blob/45d24fd6bf45a50c1833a3ebc535c611fa0f785b/clippy_lints/src/lifetimes.rs#L159
This is wrong in this case.

This lint is one of the oldest lints Clippy has (#140, way before my time), and was only touched once (#1672) since then. So I'm not really sure how exactly it works and can only point you to the right place :wink:

Same issue here while hacking on glsl. Minimal reproducible code I was able to get:

use nom::IResult;
use nom::error::VerboseError;
use std::ops::Fn;

pub type ParserResult<'a, O> = IResult<&'a str, O, VerboseError<&'a str>>;

pub fn keyword<'a>(_: &'a str) -> impl Fn(&'a str) -> ParserResult<'a, &'a str> {
  |_| unimplemented!()
}

With nom-5.

I'm taking a stab at this one.

There are two distinct cases in this issue.

case 1, the original one in the description:

impl MyMap {
    fn iter<'a>(&'a self, key: u32) -> Option<impl Iterator<Item = impl AsRef<str> + 'a>> {
        self.inner.get(&key).map(|set| set.iter())
    }
}

In this case, the lint is actually correct. Removing the lifetime notations produces an error, but the compiler error has improved:

...
help: to declare that the `impl Trait` captures data from argument `self`, you can add an explicit `'_` lifetime bound
   |
10 |     fn iter(& self, key: u32) -> Option<impl Iterator<Item = impl AsRef<str>> + '_> {
...

So the following version with anonymous lifetime compiles fine.

impl MyMap {
    fn iter(&self, key: u32) -> Option<impl Iterator<Item = impl AsRef<str> + '_>> {
        self.inner.get(&key).map(|set| set.iter())
    }
}

Since the lint is not making any wrong suggestion here, I would leave it as is. The compiler error will help the user to find the correct elided syntax.

case 2: the next examples commented in the issue boil down to this:

fn get_x<'a>(i32_pointer: &'a i32) -> impl Fn() -> &'a i32 {
    move || i32_pointer
}

Lifetime elision applies to three constructs: function items, function pointers, and closure trait bounds. This case has a nested elision candidate in impl Fn() -> &'a i32 (a closure trait bound). If 'a explicit lifetimes are removed, the compiler infers lifetimes for impl Fn() -> &i32 separately and will assign the 'static lifetime to the output reference &'static i32.

One solution to avoid FPs here is to bail out if a function pointer or a closure trait bound is found in the function signature. That might introduce a few FNs though.

I believe I'm getting this as well for:

pub async fn profile<'a>(&self, doc: &'a Value) -> Result<ProfileResult<'a>, ProfilerError>

Using '_ for any of these lifetimes just makes rustc yell at me.

@andrewbanchich I think your case looks more like #4746 as you can't elide the lifetime here because it's not related to self.

@ebroto That makes sense. Thanks!

Was this page helpful?
0 / 5 - 0 ratings