summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp229
-rw-r--r--clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h71
-rw-r--r--clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst11
-rw-r--r--clang-tools-extra/test/clang-tidy/readability-simplify-bool-expr.cpp45
4 files changed, 185 insertions, 171 deletions
diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index db09aaab985..243a5eb8feb 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -85,10 +85,11 @@ bool needsParensAfterUnaryNegation(const Expr *E) {
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 BinaryOperator *BinOp) {
}
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 NeedsStaticCast) {
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,27 @@ 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();
+ Twine ExprText = isa<BinaryOperator>(E) ? ("(" + getText(Result, *E) + ")")
+ : getText(Result, *E);
+ return (ExprText + " " + (Negated ? "!=" : "==") + " " + Constant).str();
+}
+
+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 +206,20 @@ std::string replacementExpression(const MatchFinder::MatchResult &Result,
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 +235,21 @@ std::string replacementExpression(const MatchFinder::MatchResult &Result,
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 +257,18 @@ std::string replacementExpression(const MatchFinder::MatchResult &Result,
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 +299,26 @@ const CXXBoolLiteralExpr *stmtReturnsBool(const IfStmt *IfRet, bool Negated) {
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 +362,13 @@ void SimplifyBooleanExprCheck::matchBoolCompOpExpr(MatchFinder *Finder,
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 +377,24 @@ void SimplifyBooleanExprCheck::matchExprCompOpBool(MatchFinder *Finder,
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 +410,19 @@ void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder,
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 +440,16 @@ void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
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 +460,8 @@ void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder,
unless(hasElse(stmt())))),
hasAnySubstatement(
returnStmt(has(cxxBoolLiteral(equals(!Value))))
- .bind(CompoundReturnId)))).bind(Id),
+ .bind(CompoundReturnId))))
+ .bind(Id),
this);
}
@@ -444,69 +510,46 @@ void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
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 +625,7 @@ void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(
// 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);
diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
index c74e6c45c5a..4a8076582a4 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -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);
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
index 4c3a1c8f8ce..c3c111961fb 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
@@ -35,11 +35,14 @@ The resulting expression ``e`` is modified as follows:
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;``
diff --git a/clang-tools-extra/test/clang-tidy/readability-simplify-bool-expr.cpp b/clang-tools-extra/test/clang-tidy/readability-simplify-bool-expr.cpp
index e3276103ded..8ae78741849 100644
--- a/clang-tools-extra/test/clang-tidy/readability-simplify-bool-expr.cpp
+++ b/clang-tools-extra/test/clang-tidy/readability-simplify-bool-expr.cpp
@@ -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;{{$}}
OpenPOWER on IntegriCloud