[PATCH] D65192: [Sema] Disable some enabled-by-default -Wparentheses diagnostics
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 1 06:11:44 PDT 2019
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;
> 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`:
> ffmpeg https://bugs.freebsd.org/bugzilla/show_bug.cgi?format=multiple&id=191743
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.
CHANGES SINCE LAST ACTION
More information about the cfe-commits