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, 93 insertions, 136 deletions
diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index 243a5eb8feb..db09aaab985 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -85,11 +85,10 @@ 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;
}
@@ -108,8 +107,12 @@ 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) {
@@ -145,15 +148,7 @@ 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 ||
- 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 ImpCast->getCastKind() == CK_PointerToBoolean;
return false;
}
@@ -177,27 +172,6 @@ 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();
@@ -206,20 +180,14 @@ 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 compareExpressionToNullPtr(Result, UnOp->getSubExpr(), true);
-
- if (needsZeroComparison(UnOp->getSubExpr()))
- return compareExpressionToZero(Result, UnOp->getSubExpr(), true);
+ return (getText(Result, *UnOp->getSubExpr()) + " != nullptr").str();
return replacementExpression(Result, false, UnOp->getSubExpr());
}
}
if (needsNullPtrComparison(E))
- return compareExpressionToNullPtr(Result, E, false);
-
- if (needsZeroComparison(E))
- return compareExpressionToZero(Result, E, false);
+ return (getText(Result, *E) + " == nullptr").str();
StringRef NegatedOperator;
const Expr *LHS = nullptr;
@@ -235,21 +203,18 @@ 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 compareExpressionToNullPtr(Result, E, false);
-
- if (needsZeroComparison(E))
- return compareExpressionToZero(Result, E, false);
+ return (getText(Result, *E) + " == nullptr").str();
return ("!" + asBool(Text, NeedsStaticCast));
}
@@ -257,18 +222,12 @@ 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 compareExpressionToNullPtr(Result, UnOp->getSubExpr(), false);
-
- if (needsZeroComparison(UnOp->getSubExpr()))
- return compareExpressionToZero(Result, UnOp->getSubExpr(), false);
+ return (getText(Result, *UnOp->getSubExpr()) + " == nullptr").str();
}
}
if (needsNullPtrComparison(E))
- return compareExpressionToNullPtr(Result, E, true);
-
- if (needsZeroComparison(E))
- return compareExpressionToZero(Result, E, true);
+ return (getText(Result, *E) + " != nullptr").str();
return asBool(getText(Result, *E), NeedsStaticCast);
}
@@ -299,26 +258,6 @@ 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,
@@ -362,13 +301,12 @@ 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);
}
@@ -377,24 +315,22 @@ 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,
@@ -410,19 +346,18 @@ 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,
@@ -440,16 +375,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,
@@ -460,8 +395,7 @@ void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder,
unless(hasElse(stmt())))),
hasAnySubstatement(
returnStmt(has(cxxBoolLiteral(equals(!Value))))
- .bind(CompoundReturnId))))
- .bind(Id),
+ .bind(CompoundReturnId)))).bind(Id),
this);
}
@@ -510,46 +444,69 @@ 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(
@@ -625,7 +582,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 of a single
+ // 2) An `if` statement with no `else` clause that consists fo a single
// `return` statement returning the opposite boolean literal `true` or
// `false`.
assert(Compound->size() >= 2);
OpenPOWER on IntegriCloud