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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
