On Mon, Oct 18, 2021 at 10:14 AM Nick Desaulniers <ndesaulniers@xxxxxxxxxx> wrote: > > Right, the patch that added the warning explicitly checks for side effects. Well, it's a bit questionable. The "side effects" are things like any pointer dereference, because it could fault, but if you know that isn't an issue, then clang basically ends up complaining about code that is perfectly fine. Maybe it was written that way on purpose, like the kvm code. Now, it's probably not worth keeping that "bitops of booleans" logic - if it is a noticeable optimization, it's generally something that the compiler should do for us, but basically clang is warning about perfectly valid code. And what I find absolutely disgusting is the suggested "fix" that clang gives you. If the warning said "maybe you meant to use a logical or (||)", then that would be one thing. But what clang suggests as the "fix" for the warning is just bad coding practice. If a warning fix involves making the code uglier, then the warning fix is wrong. This is not the first time we've had compilers suggesting garbage. Gcc used to suggest (perhaps still does) the "extra parenthesis" for "assignment used as a truth value" situation. Which is - once again - disgusting garbage. Writing code like if (a = b) .. is bad and error prone. But the suggestion to "fix" the warning with if ((a = b)) .. is just completely unacceptably stupid, and is just BAD CODE. The proper fix might be to write it like if ((a = b) != 0) ... which at least makes the truth value part explicit - in ways that a silly double parenthesis does not. Or, better yet, write it as a = b; if (a) .. instead, which is legible and fine. The clang suggestion to add a cast to 'int' to avoid the warning is the same kind of "write bad code" suggestion. Just don't do it. Linus