dblaikie added a comment. In D140860#4033530 <https://reviews.llvm.org/D140860#4033530>, @hazohelet wrote:
> 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. FWIW a lot of build systems support setting CXXFLAGS/CFLAGS before invoking the build system/build generator (cmake, for instance) and respects those - so might be relatively easy to add a new warning flag to the build (& CXX/CC to set the compiler to point to your local build with the patch/changes) > 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 So, based on best guess at least from grepping - you don't find any new instances of the warning in these code bases? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140860/new/ https://reviews.llvm.org/D140860 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
