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: > LegalizeAdulthood wrote: > > 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. > Agreed; trying to suss out what the appropriate design for this particular > check is. I think I've talked myself into "don't touch parens here." Currently in this patch, parens are added when the expression compared to `nullptr` or `0` is a binary operator.
I think that is the right thing to do here so we get: ``` bool b = (i & 1) == 0; ``` instead of ``` bool b = i & 1 == 0; ``` http://reviews.llvm.org/D16308 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits