[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.cpp:192
@@ +191,3 @@
+                                       const Expr *E, bool Negated) {
+  return compareExpressionToConstant(Result, E, Negated, "nullptr");
+}
----------------
Can we add a test to ensure that we aren't suggesting nullptr for a C program? Perhaps we could simply substitute nullptr for >= C++11 and NULL for everything else?

================
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;`.
 ///
----------------
Perhaps a different idea regarding parens isn't to add/remove parens in various checks for readability, but instead have two readability checks that do different things (both disabled by default?): (1) remove spurious parens where the presence of the parens has no effect on the order of evaluation of the subexpressions, and (2) add parens where there is an operator precedence difference between the operators of two subexpressions. Both of these checks are at odds with one another (which is why I don't think it makes sense to enable them by default), but both certainly seem like they would be useful.

Thoughts on the general idea (not trying to sign either of us up for work to implement it)?

================
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
----------------
I guess I view it differently; this isn't being hyper-anal, it's precisely stating what we support so that the user doesn't have to guess. However, I'm also fine leaving it out because it isn't likely to cause confusion for too long -- a simple test gives the answer if anyone is wondering.


http://reviews.llvm.org/D16308