Skip to content

branches_sharing_code reporting a false positive #7452

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

Closed
MGlolenstine opened this issue Jul 10, 2021 · 6 comments · Fixed by #9138
Closed

branches_sharing_code reporting a false positive #7452

MGlolenstine opened this issue Jul 10, 2021 · 6 comments · Fixed by #9138
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@MGlolenstine
Copy link

I tried this code:

fn test() {
    let v = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 0];
    let mut counter = 0;
    for i in v {
        if counter == 0 {
            counter += 1;
            println!("first");
        } else if counter == 1 {
            counter += 1;
            println!("second");
        } else {
            counter += 1;
            println!("other: {}", i);
        }
    }
}

I expected to see this happen: I expected the lint to pass without a hitch, as the counter is used in the if statement.

Instead, this happened:
I got a lint report, that I could change the if statements, as they all start with the same line of code, but doing that makes the code invalid.

warning: all if blocks contain the same code at the start
 --> src/lib.rs:5:9
  |
5 | /         if counter == 0 {
6 | |             counter += 1;
  | |_________________________^
  |
  = note: `#[warn(clippy::branches_sharing_code)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#branches_sharing_code
help: consider moving the start statements out like this
  |
5 |         counter += 1;
6 |         if counter == 0 {
  |

Meta

  • cargo clippy -V: clippy 0.1.53 (53cb7b0 2021-06-17)
  • rustc -Vv:
rustc 1.53.0 (53cb7b09b 2021-06-17)
binary: rustc
commit-hash: 53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b
commit-date: 2021-06-17
host: x86_64-unknown-linux-gnu
release: 1.53.0
LLVM version: 12.0.1
@MGlolenstine MGlolenstine 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 Jul 10, 2021
@MGlolenstine
Copy link
Author

Suggesting label change to I-suggestion-causes-error, as it's correctly detected the case, but the fix causes a problem.

@xFrednet
Copy link
Member

@rustbot label +I-suggestion-causes-error -I-false-positive

@rustbot rustbot added I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied and removed I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jul 12, 2021
@xFrednet
Copy link
Member

This kind of fits both labels, I'm going over a possible solution in my head and might claim this issue :)

bors added a commit that referenced this issue Jul 14, 2021
…p, r=camsteffen

FP fix and documentation for `branches_sharing_code` lint

Closes #7369

Related #7452 I'm still thinking about the best way to fix this. I could simply add another visitor to ensure that the moved expressions don't modify values being used in the condition, but I'm not totally happy with this due to the complexity. I therefore only documented it for now

changelog: [`branches_sharing_code`] fixed false positive where block expressions would sometimes be ignored.
@ghost ghost mentioned this issue Jul 15, 2021
11 tasks
@MGlolenstine
Copy link
Author

@xFrednet Can this issue be closed now that the #7462 has been merged?
Thanks for the great work!

@xFrednet
Copy link
Member

#7462 only documented this as a known issue. It's possible to actually fix this issue to not trigger the lint at all. I would keep this issue open until that is done, or it's decided that it's not worth it.

You're welcome, thank you for the report. 🙃

@youknowone
Copy link
Contributor

Similar false positive happens when the shared code is locking. Moving lock to outside of if statement may cause different behaviour.

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-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants