summaryrefslogtreecommitdiffstats
path: root/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
diff options
context:
space:
mode:
Diffstat (limited to 'clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp')
-rw-r--r--clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp229
1 files changed, 136 insertions, 93 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);
OpenPOWER on IntegriCloud