[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7428-7430
+def warn_bitwise_and_bool : Warning<
+  "bitwise and of boolean expressions; did you mean logical and?">,
+  InGroup<BoolOperationAnd>;
----------------
xbolva00 wrote:
> Quuxplusone wrote:
> > I suggest that the name and wording of this diagnostic should match `warn_logical_instead_of_bitwise`, currently `"use of logical '%0' with constant operand"`. So:
> > ```
> > def warn_bitwise_instead_of_logical : Warning<
> >   "use of bitwise '%0' with boolean operand">,
> > ```
> > This neatly sidesteps the problem of what to call the `&` operator: I was not thrilled with the phrase `bitwise and of`, but have no problem with `use of bitwise '&'`.
> I see the point but then I will not be able to provide -Wbool-operation-and flag to enable/disable this warning.
> 
> For example I know that Google prefers a new flag for every new warning so they dont have to disable eg. -Wbool-operation, but just this new warning while there are working on fixes for new warnings.
> 
> @hans what do you think?
> 
> 
I don't understand your comment. My guess is that you didn't notice that `-Wbitwise-instead-of-logical` would still be a different flag from `-Wlogical-instead-of-bitwise`. I'm not suggesting putting them under the //same// flag; just making the newly added flag a little more consistent and (heh) logical, based on what's already there.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108003/new/

https://reviews.llvm.org/D108003