[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values
    Takuya Shimizu via Phabricator via cfe-commits 
    cfe-commits at lists.llvm.org
       
    Sat Jan  7 07:47:55 PST 2023
    
    
  
hazohelet added a comment.
I have yet to do thorough checks using this patched clang to build significant code bases.
It will likely take quite a bit of time as I am not used to editing build tool files.
Instead, I used `grep` to find potentially newly-warned codes.
The `grep` command is shown below. It is intended to match C/C++ codes with both `&&` and `||` within which 0, 1, or 2 matching parentheses exist in a single line.
`grep -r --include=\*.cpp --include=\*.cc  --include=\*.c -e "&&[^()]*||" -e "||[^()]*&&" -e "&&[^()]*([^()]*)[^()]*||" -e "||[^()]*([^()]*)[^()]*&&" -e "&&[^()]*([^()]*([^()]*)[^()]*)[^()]*||" -e "||[^()]*([^()]*([^()]*)[^()]*)[^()]*&&"`
I applied this `grep` command to the following popular GitHub repositories `NVIDIA/TensorRT`, `bulletphysics/bullet3`, `apple/foundationdb`, `grpc/grpc`, `microsoft/lightgbm`, `rui314/mold`, `oneapi-src/oneTBB`, `SerenityOS/serenity`, `tensorflow/tensorflow`.
I found the 7 examples of missing parentheses at `&&` within `||` in the matched strings (many of the matchings exist in preprocessor conditionals, which is out of the scope of this patch)
They are listed at the end of this comment.
The last case in tensorflow is an `assert(a || b && "reason for the assert")` known idiom and is handled correctly.
Because the newly-warned constants are compile-time constants not dependent on function arguments, the other 6 cases will also get warnings from the clang before this patch.
It suggests that this patch does not generate extensive new warnings in existent codebases.
oneTBB:
https://github.com/oneapi-src/oneTBB/blob/e6e493f96ec8b7e2e2b4d048ed49356eb54ec2a0/examples/common/gui/xvideo.cpp#L359
https://github.com/oneapi-src/oneTBB/blob/e6e493f96ec8b7e2e2b4d048ed49356eb54ec2a0/src/tbbmalloc/frontend.cpp#L1266
tensorflow:
https://github.com/tensorflow/tensorflow/blob/fb2c3b1e3140af73f949981d8428379cbb28228b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc#L4130
https://github.com/tensorflow/tensorflow/blob/fb2c3b1e3140af73f949981d8428379cbb28228b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc#L4074
https://github.com/tensorflow/tensorflow/blob/fb2c3b1e3140af73f949981d8428379cbb28228b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc#L9456
https://github.com/tensorflow/tensorflow/blob/fb2c3b1e3140af73f949981d8428379cbb28228b/tensorflow/compiler/jit/deadness_analysis.cc#L1423
https://github.com/tensorflow/tensorflow/blob/fb2c3b1e3140af73f949981d8428379cbb28228b/tensorflow/core/ir/tf_op_wrapper.cc#L25
CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140860/new/
https://reviews.llvm.org/D140860
    
    
More information about the cfe-commits
mailing list