Single-use-lifetimes false positive?

help
1

The following code triggers the single_use_lifetimes lint:

struct Foo<'a>(&'a str);
impl<'a> Foo<'a> {
    // The below lint causes a compilation failure.
    #[deny(single_use_lifetimes)]
    fn new<'b: 'a>(x: &'b str) -> Self {
        Self(x)
    }
}

however the below does not:

impl<'a: 'b, 'b> From<&'a str> for Foo<'b> {
    // The below lint does not cause a compilation failure.
    #[deny(single_use_lifetimes)]
    fn from(value: &'a str) -> Self {
        Self(value)
    }
}

There are benefits to the From impl, so I'm glad the lint does not trigger for it. How is Foo::new different, or am I correct in thinking this is a false positive?

2

&'b str is automatically coerced into &'a str where 'b: 'a so the lifetime 'b is redundent, you can just say:

fn new(x: &'a str) -> Self {
    Self(x)
}
3

So because we can't talk about this in a "higher-level" context like we can with traits, there's no benefit?

The example provided by @steffahn that illustrates why there is benefit in the From impl utilizes the fact that traits can be "talked about" in a higher-level context, so I was curious if a similar thing could happen with an associated function like Foo::new. Thinking about it more, it seems like you would need to model this via a trait for a semantic distinction to arise.

4

It could be a breaking change to apply the suggestion, so I'd call it a false positive, or perhaps something that should be in more opinionated lint group (e.g. in Clippy or such).

The lint seems reasonable for new interfaces (and arguably private ones) though.

1 Like
5

My brain seems to be failing me. Your example2 seems to be a good reason even for new interfaces. example1 is "obvious", but I'm willing to live without the ability of explicitly passing in a lifetime argument. How can example2 be made to work without Foo::new being polymorphic?

6

this is indeed an edge case, you cannot pass Foo::new directly in such cases where explicit lifetimes are involed, the workaround is to wrap it in a closure:

- i.map(Foo::new)
+ i.map(|s| Foo::new(s))

this trick is commonly used in many cases, e.g. to utilize the "magic power" of the method call syntax, not only for the lifetime coersion, but other coersions too, such as Deref coersion;

let ss: Vec<String> = todo!();
let total_len: usize;
// error
total_len = ss.iter().map(str::len).sum(); //<-- this won't work
total_len = ss.iter().map(String::len).sum(); //<-- nor does this

// ok
total_len = ss.iter().map(|s| s.len()).sum(); // but this works!

// note, even this doesn't work, because the iterator of `ss.iter()`
// yields `&&str`, not `&str`
let ss: Vec<&str> = todo!();
let total_len = ss.iter().map(str::len).sum(); //<-- does NOT work
3 Likes
7

Indeed that makes it work. I almost always side on design that make life easier for calling code all things being equal, so I think I may keep my design even for new code.

8

I agree with everything @nerditation said. I'll point out something else though I guess:

    pub fn foo1<'b: 'a, 'a>(s: &'b str) -> Foo<'a> {
        Foo(s)
    }
    
    pub fn foo2(s: &str) -> Foo<'_> {
        Foo(s)
    }

Although these are both generic, it's a breaking change to go between them. We've seen how foo1 can "do more" in some sense, but so can foo2: it implements for<'all> Fn(&'all str) -> Foo<'all> and foo1 does not (because the lifetimes are part of an explicit bound in that case).

The implicit implementations of function items is subtle business...

2 Likes
9

Grr! Why can't you make my life easier?!? I'm kidding of course. I love all the wisdom you share. I'll have to ruminate on this a bit.

10

OK, so that example only applies to free functions. An associated function like Foo::new does not implement for<'a> Fn(&'a str) -> Foo<'a> since the lifetime has to be named in order to ensure the lifetime of the passed str lasts long enough. This leads me back to my original conclusion of keeping the original API even for new code. I'm always hesitant to rely on subtyping since it often comes up at the call site which makes me "fearful" that I'm unnecessarily making an API less general.

For free functions like the one you provided, I never felt the need to specify bounds like that; so perhaps subconsciously I knew that this concern only applied to things like associated functions or more accurately areas where there is already an explicit lifetime involved.

It seemed more natural to do this when traits are involved for some reason (e.g., I never rely on #[derive(PartialEq)] for types that have lifetimes), but I think it makes sense for things like this as well especially since it should have little-to-no impact on calling code even if the generated documentation is slightly more verbose/complex.