aaron.ballman added inline comments.
================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5654-5656 def warn_addition_in_bitshift : Warning< "operator '%0' has lower precedence than '%1'; " + "'%1' will be evaluated first">, InGroup<ShiftOpParentheses>, DefaultIgnore; ---------------- MaskRay wrote: > rsmith wrote: > > Do you have evidence that this warning has a significant false-positive > > rate? In my experience it's very common for people to think of `<<` as > > being a multiplication-like operator and be surprised when it turns out to > > have lower precedence than addition. > warn_addition_in_bitshift has many false positives. Some results when > searching for `[-Wshift-op-parentheses]` and the most common diagnostic > `operator '<<' has lower precedence than '+'; '+' will be evaluated first`: > > https://gitship.com/srutscher/pdp-6-emulator/blob/f41b119eee2f409ea519b15f3a76cfecb70c03d8/emu/Makefile > https://www.openwall.com/lists/musl/2014/04/04/12 > https://salmonella-freebsd-x86-64.call-cc.org/chicken-4/clang/freebsd/x86-64/2018/06/21/salmonella-report/install/bvsp-spline.html > https://software.intel.com/en-us/forums/intel-threading-building-blocks/topic/431422 > https://github.com/rdkit/rdkit/issues/145 > ffmpeg > https://bugs.freebsd.org/bugzilla/show_bug.cgi?format=multiple&id=191743 > https://clang.debian.net/logs/2015-03-25/fairymax_4.8v-1_unstable_clang.log > Some of those look like true positives. For instance, the fix to https://github.com/rdkit/rdkit/issues/145 was https://github.com/rdkit/rdkit/commit/38ca41c8abc3f4429ae8df2ad79d3ff8b3fea0b6 which looks like the warning behaved as intended. https://software.intel.com/en-us/forums/intel-threading-building-blocks/topic/431422 doesn't really have anything to do with the diagnostic. FWIW, in most of the cases, I feel like parens would clarify the code (heck, they're already using parens in many of these cases). e.g., ``` fairymax.c:776:46: warning: operator '>>' has lower precedence than '+'; '+' will be evaluated first [-Wshift-op-parentheses] MovesLeft = -(GamePtr+(Side==WHITE)>>1); ~~~~~~~^~~~~~~~~~~~~~~~ dbvspis.c:606:20: warning: operator '<<' has lower precedence than '+'; '+' will be evaluated first [-Wshift-op-parentheses] i3 = i2 + (*np + 1 << 1); ~~~~^~~ ~~ ``` FWIW, I'm fine leaving this default on. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65192/new/ https://reviews.llvm.org/D65192 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
