rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.54k stars 12.74k forks source link

Add lint to detect lifetime parameters that could be replaced by `'static` #120344

Open oskgo opened 9 months ago

oskgo commented 9 months ago

Code

pub fn foo<'a>() -> &'a i32 {&0}

Current output

empty

Desired output

warning: lifetime parameter `'a` is unnecessary
 --> src/lib.rs:1:12
  |
1 | pub fn foo<'a>() -> &'a i32 {&0};
  |            ^^ unnecessary lifetime parameter
  |
  = help: consider removing `'a`, using `'static` instead

Rationale and extra context

In some cases lifetime parameters might as well be 'static because of subtyping. Using a 'static lifetime can simplify the signature by avoiding generics and help teach beginners that the only reason this works is because of static promotion. Adding something about static promotion to the warning could also make sense. I'm not too happy with my "desired output".

This is also related to the unused_lifetimes lint, but that doesn't catch this either and it's not exactly "unused".

Credit goes to @estebank for the suggestion in https://github.com/rust-lang/rust/issues/48998#issuecomment-372911479

Other cases

No response

Rust Version

rustc 1.75.0 (82e1608df 2023-12-21)
binary: rustc
commit-hash: 82e1608dfa6e0b5569232559e3d385fea5a93112
commit-date: 2023-12-21
host: x86_64-pc-windows-msvc
release: 1.75.0
LLVM version: 17.0.6

Anything else?

No response

compiler-errors commented 9 months ago

This could be implemented as an extension to the REDUNDANT_LIFETIMES lint (#118391), but I'd want to make sure it is exactly correct given the variance of the return types, and I'm a bit skeptical of its actual usefulness.

oskgo commented 9 months ago

@compiler-errors Contravariance is a concern because the following lifetime 'a is needed;

fn foo<'a>() -> fn(&'a i32) -> i32 {
    fn bar<'b>(x: &'b i32) -> i32 {
        0
    }
    bar
}

I don't think it's much of a problem though. It just amounts to checking if the lifetime is used contravariantly in the output.

Invariance is never going to be a problem because we can't return an invariant type with non-'static lifetime without deriving it from one of the inputs.

The main motivation is to educate about static promotion (see #48998). Even without that I don't see how it would be any less useful than unused_lifetimes or the other REDUNDANT_LIFETIMES. Can you elaborate on why you're more skeptical about this in particular?

compiler-errors commented 9 months ago

Invariance is never going to be a problem because we can't return an invariant type with non-'static lifetime without deriving it from one of the inputs.

That's not true. In the rust compiler, for example, we have a 'tcx lifetime which is invariant due to being constrained by a GAT. We have several functions with the signature:

fn ...<'tcx>() -> Invariant<'tcx> which would be incorrect if you replaced the lifetime with 'static.


Also, this only works when the data pointed-to actually outlives 'static. For example, turning &'a Option<T> into &'static Option<T> doesn't work when T: 'static doesn't hold. This would need to take into account the changes in implied bounds from making that free lifetime in the output a 'static.

oskgo commented 9 months ago

When I wrote this up initially I had some text about 'a needing to be unconstrained, but it seems to have been lost in a rewrite. It's fairly obvious that any meaningful constraint on 'a will prevent it from being replacable by 'static.

fn ...<'tcx>() -> Invariant<'tcx> which would be incorrect if you replaced the lifetime with 'static.

You're right. I completely blanked on the fact that even though the type has a non-'static lifetime the values do not have to because of enums. I guess a check for invariance would be needed as well.

fn foo<'a>() -> Result<&'a mut u8, u8> {
    Err(0)
}

For example, turning &'a Option<T> into &'static Option<T> doesn't work when T: 'static doesn't hold.

Something like this, right?

fn foo<T>(_t: T) -> Option<&'static Option<T>> {None}

fn bar(a: &u8) {foo(a);}

I'll be honest, I didn't realize implied bounds on lifetimes were a thing. This is more complex than I thought.

I'll stop making uninformed (or informed for that matter) comments about the implementation details now. I'm still curious why you think that the usefulness here is any lower than with unused_lifetimes and REDUNDANT_LIFETIMES though.