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

needless_lifetimes removes lifetimes that are actually necessary #12908

Open
AlfredAn opened this issue Jun 8, 2024 · 3 comments
Open

needless_lifetimes removes lifetimes that are actually necessary #12908

AlfredAn opened this issue Jun 8, 2024 · 3 comments
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@AlfredAn
Copy link

AlfredAn commented Jun 8, 2024

Summary

The lifetimes seem to be unavoidable in the function I defined, but clippy still suggests removing them. If I run cargo clippy --fix, the resulting code fails to compile.

Lint Name

needless_lifetimes

Reproducer

I tried this code:

#![allow(dead_code)]

use std::collections::hash_map::{Entry, HashMap};

use petgraph::matrix_graph::{NodeIndex, UnMatrix};
use winnow::{
    ascii::line_ending,
    ascii::{alpha1, dec_uint},
    combinator::{opt, separated, seq, terminated},
    error::ParserError,
    PResult, Parser,
};

pub fn lines<'a, Output, Error>(
    parser: impl Parser<&'a str, Output, Error>,
) -> impl Parser<&'a str, Vec<Output>, Error>
where
    Error: ParserError<&'a str>,
{
    terminated(separated(.., parser, line_ending), opt(line_ending))
}

type G<'s> = UnMatrix<&'s str, u32>;

fn edge<'s>(input: &mut &'s str) -> PResult<(&'s str, &'s str, u32)> {
    seq!(
        alpha1,
        _: " to ",
        alpha1,
        _: " = ",
        dec_uint
    )
    .parse_next(input)
}

fn parse<'s>(input: &'s str) -> G<'s> {
    lines(edge)
        .map(|edges| {
            let mut g = G::default();

            let mut nodes = HashMap::<&'s str, NodeIndex<u16>>::default();
            let mut node = |g: &mut G<'s>, tag: &'s str| match nodes.entry(tag) {
                Entry::Occupied(e) => *e.get(),
                Entry::Vacant(e) => *e.insert(g.add_node(tag)),
            };

            for (a, b, weight) in edges {
                let a = node(&mut g, a);
                let b = node(&mut g, b);
                g.add_edge(a, b, weight);
            }

            g
        })
        .parse(input)
        .unwrap()
}

I saw this happen:

rygno@Alfred-Desktop:~/aoc/test_clippy$ cargo clippy --fix --allow-dirty
    Checking test_clippy v0.1.0 (/home/rygno/aoc/test_clippy)
warning: failed to automatically apply fixes suggested by rustc to crate `test_clippy`

after fixes were automatically applied the compiler reported errors within these files:

  * test_clippy/src/lib.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust-clippy/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
error[E0261]: use of undeclared lifetime name `'s`
  --> test_clippy/src/lib.rs:41:40
   |
36 | fn parse(input: &str) -> G<'_> {
   |         - help: consider introducing lifetime `'s` here: `<'s>`
...
41 |             let mut nodes = HashMap::<&'s str, NodeIndex<u16>>::default();
   |                                        ^^ undeclared lifetime

error[E0261]: use of undeclared lifetime name `'s`
  --> test_clippy/src/lib.rs:42:39
   |
36 | fn parse(input: &str) -> G<'_> {
   |         - help: consider introducing lifetime `'s` here: `<'s>`
...
42 |             let mut node = |g: &mut G<'s>, tag: &'s str| match nodes.entry(tag) {
   |                                       ^^ undeclared lifetime

error[E0261]: use of undeclared lifetime name `'s`
  --> test_clippy/src/lib.rs:42:50
   |
36 | fn parse(input: &str) -> G<'_> {
   |         - help: consider introducing lifetime `'s` here: `<'s>`
...
42 |             let mut node = |g: &mut G<'s>, tag: &'s str| match nodes.entry(tag) {
   |                                                  ^^ undeclared lifetime

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0261`.
Original diagnostics will follow.

warning: the following explicit lifetimes could be elided: 's
  --> test_clippy/src/lib.rs:36:10
   |
36 | fn parse<'s>(input: &'s str) -> G<'s> {
   |          ^^          ^^           ^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
   = note: `#[warn(clippy::needless_lifetimes)]` on by default
help: elide the lifetimes
   |
36 - fn parse<'s>(input: &'s str) -> G<'s> {
36 + fn parse(input: &str) -> G<'_> {
   |

warning: `test_clippy` (lib) generated 1 warning (run `cargo clippy --fix --lib -p test_clippy` to apply 1 suggestion)
warning: failed to automatically apply fixes suggested by rustc to crate `test_clippy`

after fixes were automatically applied the compiler reported errors within these files:

  * test_clippy/src/lib.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust-clippy/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
error[E0261]: use of undeclared lifetime name `'s`
  --> test_clippy/src/lib.rs:41:40
   |
36 | fn parse(input: &str) -> G<'_> {
   |         - help: consider introducing lifetime `'s` here: `<'s>`
...
41 |             let mut nodes = HashMap::<&'s str, NodeIndex<u16>>::default();
   |                                        ^^ undeclared lifetime

error[E0261]: use of undeclared lifetime name `'s`
  --> test_clippy/src/lib.rs:42:39
   |
36 | fn parse(input: &str) -> G<'_> {
   |         - help: consider introducing lifetime `'s` here: `<'s>`
...
42 |             let mut node = |g: &mut G<'s>, tag: &'s str| match nodes.entry(tag) {
   |                                       ^^ undeclared lifetime

error[E0261]: use of undeclared lifetime name `'s`
  --> test_clippy/src/lib.rs:42:50
   |
36 | fn parse(input: &str) -> G<'_> {
   |         - help: consider introducing lifetime `'s` here: `<'s>`
...
42 |             let mut node = |g: &mut G<'s>, tag: &'s str| match nodes.entry(tag) {
   |                                                  ^^ undeclared lifetime

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0261`.
Original diagnostics will follow.

warning: `test_clippy` (lib test) generated 1 warning (1 duplicate)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.12s

I expected to see this happen:
No suggestions, as the lifetime annotation is necessary here as far as I know.

Version

rustc 1.78.0 (9b00956e5 2024-04-29)
binary: rustc
commit-hash: 9b00956e56009bab2aa15d7bff10916599e3d6d6
commit-date: 2024-04-29
host: x86_64-unknown-linux-gnu
release: 1.78.0
LLVM version: 18.1.2

Additional Labels

@rustbot label +l-suggestion-causes-error

@AlfredAn AlfredAn added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jun 8, 2024
@y21
Copy link
Member

y21 commented Jun 11, 2024

Looks like the lint's logic for checking if a lifetime is referred to in the function body stops at closures (probably a visitor not configured to enter nested bodies).

Minimized repro:

fn f<'a>(s: &'a str) {
  || { let _: &'a str; };
}

@m-rph
Copy link
Contributor

m-rph commented Jun 11, 2024

Hey @y21 I have a bunch of other issues on needless_lifetimes in my backlog that I am currently working on, do you mind if I take this?

@m-rph m-rph self-assigned this Jun 11, 2024
@y21
Copy link
Member

y21 commented Jun 11, 2024

Feel free to take it, I wasn't planning to work on it (am currently fairly busy with other things). Just decided to minimize this since the original reproducer was not minimal and relied on external types/deps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants