rsmith added a comment.
For the `&&` vs `||` and `&` vs `|` warnings, it seems extremely implausible to
me that they meet the "few or no false-positives" criterion for an
enabled-by-default warning. Even assuming that people don't know the relative
precedences of those operators, we'd expect a false-positive rate of at least
50%. So disabling those by default seems reasonable to me, and in line with
prior guidance for what makes a good on-by-default warning.
For the other two changes, I'm not yet convinced we should change the default,
but could be convinced by data about false positive rates.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5647-5650
def warn_overloaded_shift_in_comparison :Warning<
"overloaded operator %select{>>|<<}0 has higher precedence than "
"comparison operator">,
+ InGroup<OverloadedShiftOpParentheses>, DefaultIgnore;
----------------
I think this should remain enabled by default unless you have evidence of false
positives. In my experience, this catches bugs like
```
ostream << "got expected result: " << x == 0;
```
... and very little else.
That said, perhaps we could do better here, since this warning is typically
followed by an error: if we detect the special case of overload resolution
failure for an operator with an `<<` (or `>>`) operator expression on its
left-hand side, we could produce a much more targeted diagnostic for this case
and avoid the current situation of a warning followed by an error for the same
problem. If we did that, we could probably remove this warning entirely.
================
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;
----------------
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.
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
