[clang-tools-extra] r260681 - Reapply r260096.

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 12 07:09:05 PST 2016


Author: aaronballman
Date: Fri Feb 12 09:09:05 2016
New Revision: 260681

URL: http://llvm.org/viewvc/llvm-project?rev=260681&view=rev
Log:
Reapply r260096.

Expand the simplify boolean expression check to handle implicit conversion of integral types to bool and improve the handling of implicit conversion of member pointers to bool.

Implicit conversion of member pointers are replaced with explicit comparisons to nullptr.

Implicit conversions of integral types are replaced with explicit comparisons to 0.

Patch by Richard Thomson.

Modified:
    clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
    clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h
    clang-tools-extra/trunk/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
    clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp

Modified: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp?rev=260681&r1=260680&r2=260681&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp Fri Feb 12 09:09:05 2016
@@ -85,10 +85,11 @@ bool needsParensAfterUnaryNegation(const
   E = E->IgnoreImpCasts();
   if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))
     return true;
-  if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E)) {
+
+  if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E))
     return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call &&
            Op->getOperator() != OO_Subscript;
-  }
+
   return false;
 }
 
@@ -107,12 +108,8 @@ StringRef negatedOperator(const BinaryOp
 }
 
 std::pair<OverloadedOperatorKind, StringRef> OperatorNames[] = {
-    {OO_EqualEqual, "=="},
-    {OO_ExclaimEqual, "!="},
-    {OO_Less, "<"},
-    {OO_GreaterEqual, ">="},
-    {OO_Greater, ">"},
-    {OO_LessEqual, "<="}};
+    {OO_EqualEqual, "=="},   {OO_ExclaimEqual, "!="}, {OO_Less, "<"},
+    {OO_GreaterEqual, ">="}, {OO_Greater, ">"},       {OO_LessEqual, "<="}};
 
 StringRef getOperatorName(OverloadedOperatorKind OpKind) {
   for (auto Name : OperatorNames) {
@@ -148,7 +145,15 @@ std::string asBool(StringRef text, bool
 
 bool needsNullPtrComparison(const Expr *E) {
   if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E))
-    return ImpCast->getCastKind() == CK_PointerToBoolean;
+    return ImpCast->getCastKind() == CK_PointerToBoolean ||
+           ImpCast->getCastKind() == CK_MemberPointerToBoolean;
+
+  return false;
+}
+
+bool needsZeroComparison(const Expr *E) {
+  if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E))
+    return ImpCast->getCastKind() == CK_IntegralToBoolean;
 
   return false;
 }
@@ -172,6 +177,29 @@ bool needsStaticCast(const Expr *E) {
   return !E->getType()->isBooleanType();
 }
 
+std::string compareExpressionToConstant(const MatchFinder::MatchResult &Result,
+                                        const Expr *E, bool Negated,
+                                        const char *Constant) {
+  E = E->IgnoreImpCasts();
+  const std::string ExprText =
+      (isa<BinaryOperator>(E) ? ("(" + getText(Result, *E) + ")")
+                              : getText(Result, *E))
+          .str();
+  return ExprText + " " + (Negated ? "!=" : "==") + " " + Constant;
+}
+
+std::string compareExpressionToNullPtr(const MatchFinder::MatchResult &Result,
+                                       const Expr *E, bool Negated) {
+  const char *NullPtr =
+      Result.Context->getLangOpts().CPlusPlus11 ? "nullptr" : "NULL";
+  return compareExpressionToConstant(Result, E, Negated, NullPtr);
+}
+
+std::string compareExpressionToZero(const MatchFinder::MatchResult &Result,
+                                    const Expr *E, bool Negated) {
+  return compareExpressionToConstant(Result, E, Negated, "0");
+}
+
 std::string replacementExpression(const MatchFinder::MatchResult &Result,
                                   bool Negated, const Expr *E) {
   E = E->ignoreParenBaseCasts();
@@ -180,14 +208,20 @@ std::string replacementExpression(const
     if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) {
       if (UnOp->getOpcode() == UO_LNot) {
         if (needsNullPtrComparison(UnOp->getSubExpr()))
-          return (getText(Result, *UnOp->getSubExpr()) + " != nullptr").str();
+          return compareExpressionToNullPtr(Result, UnOp->getSubExpr(), true);
+
+        if (needsZeroComparison(UnOp->getSubExpr()))
+          return compareExpressionToZero(Result, UnOp->getSubExpr(), true);
 
         return replacementExpression(Result, false, UnOp->getSubExpr());
       }
     }
 
     if (needsNullPtrComparison(E))
-      return (getText(Result, *E) + " == nullptr").str();
+      return compareExpressionToNullPtr(Result, E, false);
+
+    if (needsZeroComparison(E))
+      return compareExpressionToZero(Result, E, false);
 
     StringRef NegatedOperator;
     const Expr *LHS = nullptr;
@@ -203,18 +237,21 @@ std::string replacementExpression(const
         RHS = OpExpr->getArg(1);
       }
     }
-    if (!NegatedOperator.empty() && LHS && RHS) {
+    if (!NegatedOperator.empty() && LHS && RHS)
       return (asBool((getText(Result, *LHS) + " " + NegatedOperator + " " +
-                      getText(Result, *RHS)).str(),
+                      getText(Result, *RHS))
+                         .str(),
                      NeedsStaticCast));
-    }
 
     StringRef Text = getText(Result, *E);
     if (!NeedsStaticCast && needsParensAfterUnaryNegation(E))
       return ("!(" + Text + ")").str();
 
     if (needsNullPtrComparison(E))
-      return (getText(Result, *E) + " == nullptr").str();
+      return compareExpressionToNullPtr(Result, E, false);
+
+    if (needsZeroComparison(E))
+      return compareExpressionToZero(Result, E, false);
 
     return ("!" + asBool(Text, NeedsStaticCast));
   }
@@ -222,12 +259,18 @@ std::string replacementExpression(const
   if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) {
     if (UnOp->getOpcode() == UO_LNot) {
       if (needsNullPtrComparison(UnOp->getSubExpr()))
-        return (getText(Result, *UnOp->getSubExpr()) + " == nullptr").str();
+        return compareExpressionToNullPtr(Result, UnOp->getSubExpr(), false);
+
+      if (needsZeroComparison(UnOp->getSubExpr()))
+        return compareExpressionToZero(Result, UnOp->getSubExpr(), false);
     }
   }
 
   if (needsNullPtrComparison(E))
-    return (getText(Result, *E) + " != nullptr").str();
+    return compareExpressionToNullPtr(Result, E, true);
+
+  if (needsZeroComparison(E))
+    return compareExpressionToZero(Result, E, true);
 
   return asBool(getText(Result, *E), NeedsStaticCast);
 }
@@ -258,6 +301,26 @@ const CXXBoolLiteralExpr *stmtReturnsBoo
   return nullptr;
 }
 
+bool containsDiscardedTokens(const MatchFinder::MatchResult &Result,
+                             CharSourceRange CharRange) {
+  std::string ReplacementText =
+      Lexer::getSourceText(CharRange, *Result.SourceManager,
+                           Result.Context->getLangOpts())
+          .str();
+  Lexer Lex(CharRange.getBegin(), Result.Context->getLangOpts(),
+            ReplacementText.data(), ReplacementText.data(),
+            ReplacementText.data() + ReplacementText.size());
+  Lex.SetCommentRetentionState(true);
+
+  Token Tok;
+  while (!Lex.LexFromRawLexer(Tok)) {
+    if (Tok.is(tok::TokenKind::comment) || Tok.is(tok::TokenKind::hash))
+      return true;
+  }
+
+  return false;
+}
+
 } // namespace
 
 SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,
@@ -301,12 +364,13 @@ void SimplifyBooleanExprCheck::matchBool
                                                    StringRef OperatorName,
                                                    StringRef BooleanId) {
   Finder->addMatcher(
-      binaryOperator(isExpansionInMainFile(), hasOperatorName(OperatorName),
-                     hasLHS(allOf(expr().bind(LHSId),
-                                  ignoringImpCasts(cxxBoolLiteral(equals(Value))
-                                                       .bind(BooleanId)))),
-                     hasRHS(expr().bind(RHSId)),
-                     unless(hasRHS(hasDescendant(cxxBoolLiteral())))),
+      binaryOperator(
+          isExpansionInMainFile(), hasOperatorName(OperatorName),
+          hasLHS(allOf(
+              expr().bind(LHSId),
+              ignoringImpCasts(cxxBoolLiteral(equals(Value)).bind(BooleanId)))),
+          hasRHS(expr().bind(RHSId)),
+          unless(hasRHS(hasDescendant(cxxBoolLiteral())))),
       this);
 }
 
@@ -315,22 +379,24 @@ void SimplifyBooleanExprCheck::matchExpr
                                                    StringRef OperatorName,
                                                    StringRef BooleanId) {
   Finder->addMatcher(
-      binaryOperator(isExpansionInMainFile(), hasOperatorName(OperatorName),
-                     unless(hasLHS(hasDescendant(cxxBoolLiteral()))),
-                     hasLHS(expr().bind(LHSId)),
-                     hasRHS(allOf(expr().bind(RHSId),
-                                  ignoringImpCasts(cxxBoolLiteral(equals(Value))
-                                                       .bind(BooleanId))))),
+      binaryOperator(
+          isExpansionInMainFile(), hasOperatorName(OperatorName),
+          unless(hasLHS(hasDescendant(cxxBoolLiteral()))),
+          hasLHS(expr().bind(LHSId)),
+          hasRHS(allOf(expr().bind(RHSId),
+                       ignoringImpCasts(
+                           cxxBoolLiteral(equals(Value)).bind(BooleanId))))),
       this);
 }
 
 void SimplifyBooleanExprCheck::matchBoolCondition(MatchFinder *Finder,
                                                   bool Value,
                                                   StringRef BooleanId) {
-  Finder->addMatcher(ifStmt(isExpansionInMainFile(),
-                            hasCondition(cxxBoolLiteral(equals(Value))
-                                             .bind(BooleanId))).bind(IfStmtId),
-                     this);
+  Finder->addMatcher(
+      ifStmt(isExpansionInMainFile(),
+             hasCondition(cxxBoolLiteral(equals(Value)).bind(BooleanId)))
+          .bind(IfStmtId),
+      this);
 }
 
 void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder,
@@ -346,18 +412,19 @@ void SimplifyBooleanExprCheck::matchTern
 
 void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder,
                                                   bool Value, StringRef Id) {
-  if (ChainedConditionalReturn) {
+  if (ChainedConditionalReturn)
     Finder->addMatcher(ifStmt(isExpansionInMainFile(),
                               hasThen(returnsBool(Value, ThenLiteralId)),
-                              hasElse(returnsBool(!Value))).bind(Id),
+                              hasElse(returnsBool(!Value)))
+                           .bind(Id),
                        this);
-  } else {
+  else
     Finder->addMatcher(ifStmt(isExpansionInMainFile(),
                               unless(hasParent(ifStmt())),
                               hasThen(returnsBool(Value, ThenLiteralId)),
-                              hasElse(returnsBool(!Value))).bind(Id),
+                              hasElse(returnsBool(!Value)))
+                           .bind(Id),
                        this);
-  }
 }
 
 void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
@@ -375,16 +442,16 @@ void SimplifyBooleanExprCheck::matchIfAs
       hasRHS(cxxBoolLiteral(equals(!Value))));
   auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
                                              hasAnySubstatement(SimpleElse)));
-  if (ChainedConditionalAssignment) {
+  if (ChainedConditionalAssignment)
     Finder->addMatcher(
         ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id),
         this);
-  } else {
+  else
     Finder->addMatcher(ifStmt(isExpansionInMainFile(),
                               unless(hasParent(ifStmt())), hasThen(Then),
-                              hasElse(Else)).bind(Id),
+                              hasElse(Else))
+                           .bind(Id),
                        this);
-  }
 }
 
 void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder,
@@ -395,7 +462,8 @@ void SimplifyBooleanExprCheck::matchComp
                                                    unless(hasElse(stmt())))),
                          hasAnySubstatement(
                              returnStmt(has(cxxBoolLiteral(equals(!Value))))
-                                 .bind(CompoundReturnId)))).bind(Id),
+                                 .bind(CompoundReturnId))))
+          .bind(Id),
       this);
 }
 
@@ -444,69 +512,46 @@ void SimplifyBooleanExprCheck::registerM
 
 void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {
   if (const CXXBoolLiteralExpr *LeftRemoved =
-          getBoolLiteral(Result, RightExpressionId)) {
+          getBoolLiteral(Result, RightExpressionId))
     replaceWithExpression(Result, LeftRemoved, false);
-  } else if (const CXXBoolLiteralExpr *RightRemoved =
-                 getBoolLiteral(Result, LeftExpressionId)) {
+  else if (const CXXBoolLiteralExpr *RightRemoved =
+               getBoolLiteral(Result, LeftExpressionId))
     replaceWithExpression(Result, RightRemoved, true);
-  } else if (const CXXBoolLiteralExpr *NegatedLeftRemoved =
-                 getBoolLiteral(Result, NegatedRightExpressionId)) {
+  else if (const CXXBoolLiteralExpr *NegatedLeftRemoved =
+               getBoolLiteral(Result, NegatedRightExpressionId))
     replaceWithExpression(Result, NegatedLeftRemoved, false, true);
-  } else if (const CXXBoolLiteralExpr *NegatedRightRemoved =
-                 getBoolLiteral(Result, NegatedLeftExpressionId)) {
+  else if (const CXXBoolLiteralExpr *NegatedRightRemoved =
+               getBoolLiteral(Result, NegatedLeftExpressionId))
     replaceWithExpression(Result, NegatedRightRemoved, true, true);
-  } else if (const CXXBoolLiteralExpr *TrueConditionRemoved =
-                 getBoolLiteral(Result, ConditionThenStmtId)) {
+  else if (const CXXBoolLiteralExpr *TrueConditionRemoved =
+               getBoolLiteral(Result, ConditionThenStmtId))
     replaceWithThenStatement(Result, TrueConditionRemoved);
-  } else if (const CXXBoolLiteralExpr *FalseConditionRemoved =
-                 getBoolLiteral(Result, ConditionElseStmtId)) {
+  else if (const CXXBoolLiteralExpr *FalseConditionRemoved =
+               getBoolLiteral(Result, ConditionElseStmtId))
     replaceWithElseStatement(Result, FalseConditionRemoved);
-  } else if (const auto *Ternary =
-                 Result.Nodes.getNodeAs<ConditionalOperator>(TernaryId)) {
+  else if (const auto *Ternary =
+               Result.Nodes.getNodeAs<ConditionalOperator>(TernaryId))
     replaceWithCondition(Result, Ternary);
-  } else if (const auto *TernaryNegated =
-                 Result.Nodes.getNodeAs<ConditionalOperator>(
-                     TernaryNegatedId)) {
+  else if (const auto *TernaryNegated =
+               Result.Nodes.getNodeAs<ConditionalOperator>(TernaryNegatedId))
     replaceWithCondition(Result, TernaryNegated, true);
-  } else if (const auto *If = Result.Nodes.getNodeAs<IfStmt>(IfReturnsBoolId)) {
+  else if (const auto *If = Result.Nodes.getNodeAs<IfStmt>(IfReturnsBoolId))
     replaceWithReturnCondition(Result, If);
-  } else if (const auto *IfNot =
-                 Result.Nodes.getNodeAs<IfStmt>(IfReturnsNotBoolId)) {
+  else if (const auto *IfNot =
+               Result.Nodes.getNodeAs<IfStmt>(IfReturnsNotBoolId))
     replaceWithReturnCondition(Result, IfNot, true);
-  } else if (const auto *IfAssign =
-                 Result.Nodes.getNodeAs<IfStmt>(IfAssignBoolId)) {
+  else if (const auto *IfAssign =
+               Result.Nodes.getNodeAs<IfStmt>(IfAssignBoolId))
     replaceWithAssignment(Result, IfAssign);
-  } else if (const auto *IfAssignNot =
-                 Result.Nodes.getNodeAs<IfStmt>(IfAssignNotBoolId)) {
+  else if (const auto *IfAssignNot =
+               Result.Nodes.getNodeAs<IfStmt>(IfAssignNotBoolId))
     replaceWithAssignment(Result, IfAssignNot, true);
-  } else if (const auto *Compound =
-                 Result.Nodes.getNodeAs<CompoundStmt>(CompoundBoolId)) {
+  else if (const auto *Compound =
+               Result.Nodes.getNodeAs<CompoundStmt>(CompoundBoolId))
     replaceCompoundReturnWithCondition(Result, Compound);
-  } else if (const auto *Compound =
-                 Result.Nodes.getNodeAs<CompoundStmt>(CompoundNotBoolId)) {
+  else if (const auto *Compound =
+               Result.Nodes.getNodeAs<CompoundStmt>(CompoundNotBoolId))
     replaceCompoundReturnWithCondition(Result, Compound, true);
-  }
-}
-
-bool containsDiscardedTokens(
-    const ast_matchers::MatchFinder::MatchResult &Result,
-    CharSourceRange CharRange) {
-  std::string ReplacementText =
-      Lexer::getSourceText(CharRange, *Result.SourceManager,
-                           Result.Context->getLangOpts())
-          .str();
-  Lexer Lex(CharRange.getBegin(), Result.Context->getLangOpts(),
-            ReplacementText.data(), ReplacementText.data(),
-            ReplacementText.data() + ReplacementText.size());
-  Lex.SetCommentRetentionState(true);
-  Token Tok;
-
-  while (!Lex.LexFromRawLexer(Tok)) {
-    if (Tok.is(tok::TokenKind::comment) || Tok.is(tok::TokenKind::hash))
-      return true;
-  }
-
-  return false;
 }
 
 void SimplifyBooleanExprCheck::issueDiag(
@@ -582,7 +627,7 @@ void SimplifyBooleanExprCheck::replaceCo
   // The body shouldn't be empty because the matcher ensures that it must
   // contain at least two statements:
   // 1) A `return` statement returning a boolean literal `false` or `true`
-  // 2) An `if` statement with no `else` clause that consists fo a single
+  // 2) An `if` statement with no `else` clause that consists of a single
   //    `return` statement returning the opposite boolean literal `true` or
   //    `false`.
   assert(Compound->size() >= 2);

Modified: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h?rev=260681&r1=260680&r2=260681&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h Fri Feb 12 09:09:05 2016
@@ -19,75 +19,8 @@ namespace readability {
 /// Looks for boolean expressions involving boolean constants and simplifies
 /// 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
 class SimplifyBooleanExprCheck : public ClangTidyCheck {
 public:
   SimplifyBooleanExprCheck(StringRef Name, ClangTidyContext *Context);

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst?rev=260681&r1=260680&r2=260681&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst Fri Feb 12 09:09:05 2016
@@ -35,11 +35,14 @@ The resulting expression ``e`` is modifi
   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``.
+  4. Implicit conversions of pointers, including pointers to members, to
+     ``bool`` are replaced with explicit comparisons to ``nullptr`` in C++11
+      or ``NULL`` in C++98/03.
   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``.
+  7. Implicit conversions of integral types to ``bool`` are replaced with
+     explicit comparisons to ``0``.
 
 Examples:
   1. The ternary assignment ``bool b = (i < 0) ? true : false;`` has redundant
@@ -60,11 +63,11 @@ Examples:
 
      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);``.
+     ``bool b = (i & 1) != 0;``.
 
   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);``
+     becomes ``return (i & 1) != 0;``
 
   6. Given ``struct X { explicit operator bool(); };``, and an instance ``x`` of
      ``struct X``, the conditional return ``if (x) return true; return false;``

Modified: clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp?rev=260681&r1=260680&r2=260681&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp Fri Feb 12 09:09:05 2016
@@ -690,7 +690,7 @@ bool if_implicit_bool_expr(int i) {
   }
 }
 // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
-// CHECK-FIXES: {{^}}  return static_cast<bool>(i & 1);{{$}}
+// CHECK-FIXES: {{^}}  return (i & 1) != 0;{{$}}
 
 bool negated_if_implicit_bool_expr(int i) {
   if (i - 1) {
@@ -700,7 +700,7 @@ bool negated_if_implicit_bool_expr(int i
   }
 }
 // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
-// CHECK-FIXES: {{^}}  return !static_cast<bool>(i - 1);{{$}}
+// CHECK-FIXES: {{^}}  return (i - 1) == 0;{{$}}
 
 bool implicit_int(int i) {
   if (i) {
@@ -710,7 +710,7 @@ bool implicit_int(int i) {
   }
 }
 // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
-// CHECK-FIXES: {{^}}  return static_cast<bool>(i);{{$}}
+// CHECK-FIXES: {{^}}  return i != 0;{{$}}
 
 bool explicit_bool(bool b) {
   if (b) {
@@ -757,7 +757,7 @@ bool bitwise_complement_conversion(int i
   }
 }
 // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
-// CHECK-FIXES: {{^}}  return static_cast<bool>(~i);{{$}}
+// CHECK-FIXES: {{^}}  return ~i != 0;{{$}}
 
 bool logical_or(bool a, bool b) {
   if (a || b) {
@@ -830,7 +830,7 @@ void ternary_integer_condition(int i) {
   bool b = i ? true : false;
 }
 // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{.*}} in ternary expression result
-// CHECK-FIXES: bool b = static_cast<bool>(i);{{$}}
+// CHECK-FIXES: bool b = i != 0;{{$}}
 
 bool non_null_pointer_condition(int *p1) {
   if (p1) {
@@ -895,3 +895,38 @@ bool preprocessor_in_the_middle(bool b)
 // CHECK-MESSAGES: :[[@LINE-6]]:12: warning: {{.*}} in conditional return
 // CHECK-FIXES: {{^}}  if (b) {
 // CHECK-FIXES: {{^}}#define SOMETHING_WICKED false
+
+bool integer_not_zero(int i) {
+  if (i) {
+    return false;
+  } else {
+    return true;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}  return i == 0;{{$}}
+
+class A {
+public:
+    int m;
+};
+
+bool member_pointer_nullptr(int A::*p) {
+  if (p) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return p != nullptr;{{$}}
+
+bool integer_member_implicit_cast(A *p) {
+  if (p->m) {
+    return true;
+  } else {
+    return false;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return p->m != 0;{{$}}




More information about the cfe-commits mailing list