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

Richard via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 1 09:09:20 PST 2016


LegalizeAdulthood added inline comments.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:21-22
@@ -20,71 +20,4 @@
 /// them to use the appropriate boolean expression directly.
 ///
-/// Examples:
-///
-/// ===========================================  ================
-/// Initial expression                           Result
-/// -------------------------------------------  ----------------
-/// `if (b == true)`                             `if (b)`
-/// `if (b == false)`                            `if (!b)`
-/// `if (b && true)`                             `if (b)`
-/// `if (b && false)`                            `if (false)`
-/// `if (b || true)`                             `if (true)`
-/// `if (b || false)`                            `if (b)`
-/// `e ? true : false`                           `e`
-/// `e ? false : true`                           `!e`
-/// `if (true) t(); else f();`                   `t();`
-/// `if (false) t(); else f();`                  `f();`
-/// `if (e) return true; else return false;`     `return e;`
-/// `if (e) return false; else return true;`     `return !e;`
-/// `if (e) b = true; else b = false;`           `b = e;`
-/// `if (e) b = false; else b = true;`           `b = !e;`
-/// `if (e) return true; return false;`          `return e;`
-/// `if (e) return false; return true;`          `return !e;`
-/// ===========================================  ================
-///
-/// The resulting expression `e` is modified as follows:
-///   1. Unnecessary parentheses around the expression are removed.
-///   2. Negated applications of `!` are eliminated.
-///   3. Negated applications of comparison operators are changed to use the
-///      opposite condition.
-///   4. Implicit conversions of pointer to `bool` are replaced with explicit
-///      comparisons to `nullptr`.
-///   5. Implicit casts to `bool` are replaced with explicit casts to `bool`.
-///   6. Object expressions with `explicit operator bool` conversion operators
-///      are replaced with explicit casts to `bool`.
-///
-/// Examples:
-///   1. The ternary assignment `bool b = (i < 0) ? true : false;` has redundant
-///      parentheses and becomes `bool b = i < 0;`.
-///
-///   2. The conditional return `if (!b) return false; return true;` has an
-///      implied double negation and becomes `return b;`.
-///
-///   3. The conditional return `if (i < 0) return false; return true;` becomes
-///      `return i >= 0;`.
-///
-///      The conditional return `if (i != 0) return false; return true;` becomes
-///      `return i == 0;`.
-///
-///   4. The conditional return `if (p) return true; return false;` has an
-///      implicit conversion of a pointer to `bool` and becomes
-///      `return p != nullptr;`.
-///
-///      The ternary assignment `bool b = (i & 1) ? true : false;` has an
-///      implicit conversion of `i & 1` to `bool` and becomes
-///      `bool b = static_cast<bool>(i & 1);`.
-///
-///   5. The conditional return `if (i & 1) return true; else return false;` has
-///      an implicit conversion of an integer quantity `i & 1` to `bool` and
-///      becomes `return static_cast<bool>(i & 1);`
-///
-///   6. Given `struct X { explicit operator bool(); };`, and an instance `x` of
-///      `struct X`, the conditional return `if (x) return true; return false;`
-///      becomes `return static_cast<bool>(x);`
-///
-/// When a conditional boolean return or assignment appears at the end of a
-/// chain of `if`, `else if` statements, the conditional statement is left
-/// unchanged unless the option `ChainedConditionalReturn` or
-/// `ChainedConditionalAssignment`, respectively, is specified as non-zero.
-/// The default value for both options is zero.
-///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-simplify-boolean-expr.html
----------------
aaron.ballman wrote:
> I think I can agree with that, but I also think it depends a lot on what the purpose to the check is. If it's "improve readability regarding parens" + options that control when to remove or add, that makes sense to me. Otherwise, I think segregating the checks into one that removes (plus options) and one that adds (plus options) makes sense -- even if they perhaps use the same underlying heuristic engine and are simply surfaced as separate checks.
This check isn't at all about the readability of parens, so it doesn't make sense for this check to be concerned with it IMO.


http://reviews.llvm.org/D16308





More information about the cfe-commits mailing list