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

blocks_in_if_conditions triggers in while loops in a too strigent way #7580

Closed
AlexTMjugador opened this issue Aug 17, 2021 · 2 comments
Closed
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

@AlexTMjugador
Copy link

AlexTMjugador commented Aug 17, 2021

Lint name: blocks_in_if_conditions

I tried this code:

https://github.com/ComunidadAylas/PackSquash/blob/b514fba469b38edf207120aaa3fb8a12476b72fe/src/squash_zip.rs#L277-L281

I expected to see this happen: no warnings emitted by Clippy.

Instead, this happened: Clippy emitted the warning. In my humble opinion, I think this warning is a false positive due to the following reasons:

  • The affected code is not an if condition, and the side effects of evaluating that expression are needed for the code to work as intended.
  • The suggested correction of assigning the result of the expression to a variable is, therefore, not appropriate.
  • Using closures or auxiliary functions is, in my opinion, not much more clearer in this case than using this block. Closures also sometimes impose some cognitive or maintenance burden because they borrow their environment in a certain way, and make the compiler do more work to optimize them away.
  • Restructuring the code so it does the method call before entering the while loop and before finishing its body is not much better either, because it leads to duplicated code.
  • Because it uses .await, putting the offending code in a closure requires async closures, which is an unstable feature, and it might not be desirable to use additional unstable features because of this lint.

Meta

  • cargo clippy -V: clippy 0.1.56 (0035d9d 2021-08-16)
  • rustc -Vv:
    rustc 1.56.0-nightly (0035d9dce 2021-08-16)
    binary: rustc
    commit-hash: 0035d9dcecee49d1f7349932bfa52c05a6f83641
    commit-date: 2021-08-16
    host: x86_64-unknown-linux-gnu
    release: 1.56.0-nightly
    LLVM version: 12.0.1
    

This warning did not occur in previous nightly versions of Clippy. I couldn't find any recent commit or issue that referenced this lint and may be the cause of it, either.

Clippy warning output screenshot

@AlexTMjugador AlexTMjugador 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 Aug 17, 2021
@camsteffen
Copy link
Contributor

Fixed by rust-lang/rust#88175

@TonalidadeHidrica
Copy link

rust-lang/rust#88175 has been merged, so shall we close this issue?

monoid added a commit to monoid/ch_cityhash102 that referenced this issue Nov 5, 2021
Acutally, we just allow globally clippy::many_single_char_names and
clippy::blocks_in_if_conditions.  The latter is to be enabled because
of clippy bug: rust-lang/rust-clippy#7580

The single char names stem from the original sources, and I prefer to
keep them as is.
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

No branches or pull requests

3 participants