[PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:77
@@ -74,3 +76,3 @@
 ///      implicit conversion of `i & 1` to `bool` and becomes
-///      `bool b = static_cast<bool>(i & 1);`.
+///      `bool b = i & 1 != 0;`.
 ///
----------------
LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > LegalizeAdulthood wrote:
> > > aaron.ballman wrote:
> > > > To me, this does not improve readability -- I now have to think much harder about operator precedence. Including parens would help significantly, IMO, so that it becomes `(i & 1) != 0`.
> > > To clarify: You'd like to see parens around the expression when it is a binary operator, correct?
> > > 
> > > When it is a variable, there's no need to add parentheses.
> > Correct; if it's just a DeclRefExpr or unary operator then parens aren't useful; but if it's a binary operator, I think parens may help clarify.
> Ironically, in another review we were talking about eliminating redundant parentheses globally and these parentheses added for binary operators would be considered redundant and removed.
In the other review, I was suggesting that parens in expressions involving binary operators of differing precedence should not be removed. ;-)

================
Comment at: docs/clang-tidy/checks/readability-simplify-boolean-expr.rst:59
@@ -56,3 +58,3 @@
 
   4. The conditional return ``if (p) return true; return false;`` has an
      implicit conversion of a pointer to ``bool`` and becomes
----------------
LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > Update for member pointers.
> Do I really need to explicitly say member pointer as well?  It seems redundant.
> 
> I didn't update it because a pointer to a member is a pointer.  When used as an implicit conversion to `bool`, the syntax is no different for a plain pointer than for a member pointer.  If the syntax were different, I could see your point.
> 
A pointer to a member is not a pointer according to the language standard, and that's why it may not be a bad idea to call it out explicitly as being supported. From [basic.compound]p3, "Except for pointers to static members, text referring to “pointers” does not apply to pointers to members." I do take your point about the syntax of the implicit conversion being the same, but it would be nice to be explicit about what we support. I would just change it to say "...implicit conversion of a pointer (or pointer to member) to ``bool``..."


http://reviews.llvm.org/D16308