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

manual_flatten suggestin make code not compilable #6784

Closed
xNxExOx opened this issue Feb 24, 2021 · 3 comments · Fixed by #7566
Closed

manual_flatten suggestin make code not compilable #6784

xNxExOx opened this issue Feb 24, 2021 · 3 comments · Fixed by #7566
Labels
C-bug Category: Clippy is not doing the correct thing 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

@xNxExOx
Copy link

xNxExOx commented Feb 24, 2021

Lint name: manual_flatten

I tried this code:
Code is simplified so workers are never set, but to trigger the incorrect sugestion it is enough.
The full code starts same with all workers doing nothing, and later in the loop assign work to them, and here early in the loop let them finish the work if it is the right time.

fn main() {
    let mut time = 0;
    let mut workers : [Option<(u8, i32)>; 5] = [None, None, None, None, None];
    loop {
        for w in &mut workers {
            if let Some((_to, when)) = w {
                if time == *when {
                    // ...
                    *w = None;
                }
            }
        }
        // ...
        time += 1;
    }
}

I expected to see this happen: No suggestion probably.

Instead, this happened:

warning: unnecessary `if let` since only the `Some` variant of the iterator element is used
  --> src\main.rs:6:9
   |
6  |           for w in &mut workers {
   |           ^        ------------ help: try: `workers.iter_mut().flatten()`
   |  _________|
   | |
7  | |             if let Some((_to, when)) = w {
8  | |                 if time == *when {
9  | |                     *w = None;
10 | |                 }
11 | |             }
12 | |         }
   | |_________^
   |
   = note: `#[warn(clippy::manual_flatten)]` on by default
help: ...and remove the `if let` statement in the for loop
  --> src\main.rs:7:13
   |
7  | /             if let Some((_to, when)) = w {
8  | |                 if time == *when {
9  | |                     *w = None;
10 | |                 }
11 | |             }
   | |_____________^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten

warning: 1 warning emitted

In this simplified example line 9 *w = None; is visible.
Am I missing a way how to handle this line? I asked on Rust community server and no one find a way how to make this work, so I think it must be false positive.

Meta

  • cargo clippy -V: clippy 0.1.52 (fe1bf8e 2021-02-23)
  • rustc -Vv:
rustc 1.52.0-nightly (fe1bf8e05 2021-02-23)
binary: rustc
commit-hash: fe1bf8e05c39bdcc73fc09e246b7209444e389bc
commit-date: 2021-02-23
host: x86_64-pc-windows-msvc
release: 1.52.0-nightly
LLVM version: 11.0.1
@xNxExOx xNxExOx 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 Feb 24, 2021
@xNxExOx
Copy link
Author

xNxExOx commented Feb 24, 2021

manual_flatten interaction with nested matches could better #6776 is probably related, but that code will still compile, just the simplification of it is not as good as could be expected, by a bit misleading message. So I do not think they are same and should not be merged together. (just my opinion)

@camsteffen
Copy link
Contributor

I agree. This is a separate issue since we should not lint if w is used in any way. This should be an easy fix with LocalUsedVisitor.

@camsteffen camsteffen added the good-first-issue These issues are a good way to get started with Clippy label Feb 24, 2021
@chris-oo
Copy link

I have some code that I believe is hitting this exact issue. Here's another example that triggers another false positive with this same lint. It's stripped down, but essentially it seems like clippy is not detecting cases where the outer option variable is actually being used.

fn main() {
    let mut vec = vec![Some(10), None, Some(20)];
    
    for entry in vec.iter_mut() {
        if let Some(number) = entry {
            if *number < 15 {
                let number = entry.take().unwrap();
                print!("removed {}", number);
            }
        }
    }
}

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 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

Successfully merging a pull request may close this issue.

3 participants