Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
oskgo opened this issue Jan 25, 2024 · 4 comments
Open

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

oskgo opened this issue Jan 25, 2024 · 4 comments
Labels
A-lifetimes Area: Lifetimes / regions A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@oskgo
Copy link
Contributor

oskgo commented Jan 25, 2024

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 #48998 (comment)

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

@oskgo oskgo added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 25, 2024
@compiler-errors
Copy link
Member

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.

@fmease fmease added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-lifetimes Area: Lifetimes / regions T-lang Relevant to the language team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed A-diagnostics Area: Messages for errors, warnings, and lints labels Jan 25, 2024
@oskgo
Copy link
Contributor Author

oskgo commented Jan 26, 2024

@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
Copy link
Member

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
Copy link
Contributor Author

oskgo commented Jan 27, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: Lifetimes / regions A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants