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

vec_init_then_push incorrectly fires when the push() is in a loop body #7019

Closed
duci9y opened this issue Apr 2, 2021 · 7 comments
Closed
Labels
C-bug Category: Clippy is not doing the correct thing E-needs-mcve Call for participation: This issue needs a Minimal Complete and Verifiable Example good-first-issue These issues are a good way to get started with Clippy I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@duci9y
Copy link

duci9y commented Apr 2, 2021

Lint name: vec_init_then_push

I tried this code:

let mut v = Vec::new();
loop {
    match *s {
        ...
        SExp::Cons(ref a, ref b) => {
            v.push(a);
        }
        ...
    }
}

I expected to see this happen: The lint to not fire, since this is a valid use case for init then push.

Instead, this happened: The lint fired.

378 | /         let mut v = Vec::new();
379 | |         loop {
380 | |             match *s {
381 | |                 ...
382 | |                 SExp::Cons(ref a, ref b) => {
383 | |                     v.push(a);
    | |______________________________^ help: consider using the `vec![]` macro: `let mut v = vec![..];`

Meta

  • cargo clippy -V: clippy 0.1.51 (2fd73fab 2021-03-23)
  • rustc -Vv:
rustc 1.51.0 (2fd73fabe 2021-03-23)
binary: rustc
commit-hash: 2fd73fabe469357a12c2c974c140f67e7cdd76d0
commit-date: 2021-03-23
host: x86_64-apple-darwin
release: 1.51.0
LLVM version: 11.0.1
@duci9y duci9y 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 Apr 2, 2021
@camsteffen
Copy link
Contributor

This should be fixed by #6697. Can you provide a minimal reproducible example?

@duci9y
Copy link
Author

duci9y commented Apr 2, 2021

I spent about 15 minutes trying to get a repro going but couldn't trigger the false positive. I'll try again later or see what I can gather about the context.

@camsteffen camsteffen added the E-needs-mcve Call for participation: This issue needs a Minimal Complete and Verifiable Example label Apr 2, 2021
@TyPR124
Copy link

TyPR124 commented Apr 28, 2021

I came across this, and managed to simplify syntax down to this. I can reproduce locally clippy 0.1.51 (2fd73fa 2021-03-23), but not on play.rust-lang.org 2021-04-27 727d101 (as of writing, true for both this example and my real code).

#[allow(clippy::while_let_loop)]
#[allow(clippy::never_loop)]
#[allow(clippy::match_single_binding)]
fn foo() {
    let mut v = vec![];
    loop {
        match () {
            () => {
                v.push(());
                return;
            }
        }
    }
}

@camsteffen camsteffen added good-first-issue These issues are a good way to get started with Clippy and removed E-needs-mcve Call for participation: This issue needs a Minimal Complete and Verifiable Example labels May 3, 2021
@camsteffen
Copy link
Contributor

I can reproduce locally clippy 0.1.51 (2fd73fa 2021-03-23)

That version predates #6697. Your example no longer lints.

@camsteffen camsteffen added the E-needs-mcve Call for participation: This issue needs a Minimal Complete and Verifiable Example label May 5, 2021
@TyPR124
Copy link

TyPR124 commented May 7, 2021

That's why I mentioned play.rust-lang.org as well, as I assume that version included that fix. Is there something that prevents that example from being a MCVE?

@camsteffen
Copy link
Contributor

We need an example that is a false positive in the latest version, or else there is no issue here.

@TyPR124
Copy link

TyPR124 commented May 8, 2021

But the issue was opened for clippy 0.1.51 (2fd73fab 2021-03-23), the same version I was using.

or else there is no issue here.

Yes, my example leads me to believe this issue has been resolved.

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 E-needs-mcve Call for participation: This issue needs a Minimal Complete and Verifiable Example good-first-issue These issues are a good way to get started with Clippy I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

No branches or pull requests

3 participants