[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks for this warning! I just finished turning it on in Chromium. Here's a short report on the things it fired on in case it's interesting to anyone. (I'm not suggesting any changes to the warning, I think it works very well as-is).

The warning fired in 21 different files. In 16 cases, using && instead of & and || instead of | was possible, but the change was inconsequential [1]. In 6 cases, & and | were used intentionally to prevent short-circuiting, but it wasn't always obvious that that was the case. I usually rewrote the code to make it more obvious [2]. In addition to these, there was of course the true positive that motivated this warning :)

Apparently we build a (small) subset of LLVM 10 as part of some dependency of chrome. The warning fired a few times there, so I turned it off for those files (in trunk LLVM it's fixed). I also found a quality-of-implementation bug with the warning which I reported as https://bugs.llvm.org/show_bug.cgi?id=52226

1:
https://chromium-review.googlesource.com/c/angle/angle/+/3212889/2/src/libANGLE/renderer/gl/VertexArrayGL.cpp
https://chromium-review.googlesource.com/c/angle/angle/+/3212889/2/src/tests/gl_tests/CopyTexImageTest.cpp
https://chromium-review.googlesource.com/c/chromium/src/+/3212300/2/content/browser/payments/payment_app_database.cc
https://chromium-review.googlesource.com/c/chromium/src/+/3212300/2/gin/v8_initializer.cc
https://chromium-review.googlesource.com/c/chromium/src/+/3212300/2/net/spdy/spdy_http_stream.cc
https://chromium-review.googlesource.com/c/chromium/src/+/3212300/2/services/network/url_loader.cc
https://chromium-review.googlesource.com/c/chromium/src/+/3212300/2/third_party/blink/renderer/core/intersection_observer/intersection_observation.cc
https://chromium-review.googlesource.com/c/chromium/src/+/3229610/2/chromecast/cast_core/url_rewrite_rules_adapter.cc
https://chromium-review.googlesource.com/c/v8/v8/+/3212891/2/src/objects/feedback-vector.cc
https://chromium-review.googlesource.com/c/v8/v8/+/3231077/2/src/codegen/arm64/assembler-arm64.cc
https://chromium.googlesource.com/external/github.com/google/distributed_point_functions/+/refs/heads/upstream/master%5E%21/#F0
https://dawn-review.googlesource.com/c/dawn/+/66200/2/src/tests/end2end/QueryTests.cpp
https://github.com/KhronosGroup/Vulkan-ValidationLayers/commit/51279399eaecf651d176544d8197f91f48637b2f
https://github.com/netwide-assembler/nasm/pull/17
https://skia-review.googlesource.com/c/skia/+/459456/3/src/core/SkColorFilter.cpp
https://webrtc-review.googlesource.com/c/src/+/234600/2/media/engine/simulcast_encoder_adapter.cc

2:
https://bugs.chromium.org/p/chromium/issues/detail?id=1261591 (still undecided)
https://chromium-review.googlesource.com/c/v8/v8/+/3212891/2/src/compiler/branch-elimination.cc
https://github.com/harfbuzz/harfbuzz/pull/3256
https://skia-review.googlesource.com/c/skia/+/459456/3/modules/skottie/src/animator/Vec2KeyframeAnimator.cpp
https://skia-review.googlesource.com/c/skia/+/459456/3/modules/skottie/src/animator/VectorKeyframeAnimator.cpp
https://skia-review.googlesource.com/c/skia/+/459456/3/src/core/SkPathEffect.cpp


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108003/new/

https://reviews.llvm.org/D108003