[clang-tools-extra] df5335a - [clang-tidy] readability-simplify-boolean-expr detects negated literals
Nathan James via cfe-commits
cfe-commits at lists.llvm.org
Sat Aug 22 05:57:48 PDT 2020
Author: Nathan James
Date: 2020-08-22T13:57:36+01:00
New Revision: df5335a36d3d70927d1834f31c705e31b4b3701f
URL: https://github.com/llvm/llvm-project/commit/df5335a36d3d70927d1834f31c705e31b4b3701f
DIFF: https://github.com/llvm/llvm-project/commit/df5335a36d3d70927d1834f31c705e31b4b3701f.diff
LOG: [clang-tidy] readability-simplify-boolean-expr detects negated literals
Adds support for detecting cases like `if (!true) ...`.
Addresses [[ https://bugs.llvm.org/show_bug.cgi?id=47166 | readability-simplify-boolean-expr not detected for negated boolean literals. ]]
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D86176
Added:
Modified:
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index 4c83499ff7ca..9dcb10b9d20c 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -58,16 +58,28 @@ const char SimplifyConditionDiagnostic[] =
const char SimplifyConditionalReturnDiagnostic[] =
"redundant boolean literal in conditional return statement";
-const CXXBoolLiteralExpr *getBoolLiteral(const MatchFinder::MatchResult &Result,
- StringRef Id) {
- const auto *Literal = Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(Id);
- return (Literal && Literal->getBeginLoc().isMacroID()) ? nullptr : Literal;
+const Expr *getBoolLiteral(const MatchFinder::MatchResult &Result,
+ StringRef Id) {
+ if (const Expr *Literal = Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(Id))
+ return Literal->getBeginLoc().isMacroID() ? nullptr : Literal;
+ if (const auto *Negated = Result.Nodes.getNodeAs<UnaryOperator>(Id)) {
+ if (Negated->getOpcode() == UO_LNot &&
+ isa<CXXBoolLiteralExpr>(Negated->getSubExpr()))
+ return Negated->getBeginLoc().isMacroID() ? nullptr : Negated;
+ }
+ return nullptr;
+}
+
+internal::BindableMatcher<Stmt> literalOrNegatedBool(bool Value) {
+ return expr(anyOf(cxxBoolLiteral(equals(Value)),
+ unaryOperator(hasUnaryOperand(ignoringParenImpCasts(
+ cxxBoolLiteral(equals(!Value)))),
+ hasOperatorName("!"))));
}
internal::Matcher<Stmt> returnsBool(bool Value, StringRef Id = "ignored") {
- auto SimpleReturnsBool =
- returnStmt(has(cxxBoolLiteral(equals(Value)).bind(Id)))
- .bind("returns-bool");
+ auto SimpleReturnsBool = returnStmt(has(literalOrNegatedBool(Value).bind(Id)))
+ .bind("returns-bool");
return anyOf(SimpleReturnsBool,
compoundStmt(statementCountIs(1), has(SimpleReturnsBool)));
}
@@ -269,16 +281,25 @@ std::string replacementExpression(const MatchFinder::MatchResult &Result,
return asBool(getText(Result, *E), NeedsStaticCast);
}
-const CXXBoolLiteralExpr *stmtReturnsBool(const ReturnStmt *Ret, bool Negated) {
+const Expr *stmtReturnsBool(const ReturnStmt *Ret, bool Negated) {
if (const auto *Bool = dyn_cast<CXXBoolLiteralExpr>(Ret->getRetValue())) {
if (Bool->getValue() == !Negated)
return Bool;
}
+ if (const auto *Unary = dyn_cast<UnaryOperator>(Ret->getRetValue())) {
+ if (Unary->getOpcode() == UO_LNot) {
+ if (const auto *Bool =
+ dyn_cast<CXXBoolLiteralExpr>(Unary->getSubExpr())) {
+ if (Bool->getValue() == Negated)
+ return Bool;
+ }
+ }
+ }
return nullptr;
}
-const CXXBoolLiteralExpr *stmtReturnsBool(const IfStmt *IfRet, bool Negated) {
+const Expr *stmtReturnsBool(const IfStmt *IfRet, bool Negated) {
if (IfRet->getElse() != nullptr)
return nullptr;
@@ -423,7 +444,7 @@ void SimplifyBooleanExprCheck::matchBoolCondition(MatchFinder *Finder,
StringRef BooleanId) {
Finder->addMatcher(
ifStmt(unless(isInTemplateInstantiation()),
- hasCondition(cxxBoolLiteral(equals(Value)).bind(BooleanId)))
+ hasCondition(literalOrNegatedBool(Value).bind(BooleanId)))
.bind(IfStmtId),
this);
}
@@ -433,8 +454,8 @@ void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder,
StringRef TernaryId) {
Finder->addMatcher(
conditionalOperator(unless(isInTemplateInstantiation()),
- hasTrueExpression(cxxBoolLiteral(equals(Value))),
- hasFalseExpression(cxxBoolLiteral(equals(!Value))))
+ hasTrueExpression(literalOrNegatedBool(Value)),
+ hasFalseExpression(literalOrNegatedBool(!Value)))
.bind(TernaryId),
this);
}
@@ -465,12 +486,12 @@ void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
auto SimpleThen =
binaryOperator(hasOperatorName("="), hasLHS(anyOf(VarAssign, MemAssign)),
hasLHS(expr().bind(IfAssignVariableId)),
- hasRHS(cxxBoolLiteral(equals(Value)).bind(IfAssignLocId)));
+ hasRHS(literalOrNegatedBool(Value).bind(IfAssignLocId)));
auto Then = anyOf(SimpleThen, compoundStmt(statementCountIs(1),
hasAnySubstatement(SimpleThen)));
auto SimpleElse =
binaryOperator(hasOperatorName("="), hasLHS(anyOf(VarRef, MemRef)),
- hasRHS(cxxBoolLiteral(equals(!Value))));
+ hasRHS(literalOrNegatedBool(!Value)));
auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
hasAnySubstatement(SimpleElse)));
if (ChainedConditionalAssignment)
@@ -495,7 +516,7 @@ void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder,
hasAnySubstatement(
ifStmt(hasThen(returnsBool(Value)), unless(hasElse(stmt())))),
hasAnySubstatement(returnStmt(has(ignoringParenImpCasts(
- cxxBoolLiteral(equals(!Value)))))
+ literalOrNegatedBool(!Value))))
.bind(CompoundReturnId)))
.bind(Id),
this);
@@ -529,10 +550,10 @@ void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {
if (Result.Nodes.getNodeAs<TranslationUnitDecl>("top"))
Visitor(this, Result).TraverseAST(*Result.Context);
- else if (const CXXBoolLiteralExpr *TrueConditionRemoved =
+ else if (const Expr *TrueConditionRemoved =
getBoolLiteral(Result, ConditionThenStmtId))
replaceWithThenStatement(Result, TrueConditionRemoved);
- else if (const CXXBoolLiteralExpr *FalseConditionRemoved =
+ else if (const Expr *FalseConditionRemoved =
getBoolLiteral(Result, ConditionElseStmtId))
replaceWithElseStatement(Result, FalseConditionRemoved);
else if (const auto *Ternary =
@@ -574,8 +595,7 @@ void SimplifyBooleanExprCheck::issueDiag(
}
void SimplifyBooleanExprCheck::replaceWithThenStatement(
- const MatchFinder::MatchResult &Result,
- const CXXBoolLiteralExpr *TrueConditionRemoved) {
+ const MatchFinder::MatchResult &Result, const Expr *TrueConditionRemoved) {
const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
issueDiag(Result, TrueConditionRemoved->getBeginLoc(),
SimplifyConditionDiagnostic, IfStatement->getSourceRange(),
@@ -583,8 +603,7 @@ void SimplifyBooleanExprCheck::replaceWithThenStatement(
}
void SimplifyBooleanExprCheck::replaceWithElseStatement(
- const MatchFinder::MatchResult &Result,
- const CXXBoolLiteralExpr *FalseConditionRemoved) {
+ const MatchFinder::MatchResult &Result, const Expr *FalseConditionRemoved) {
const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
const Stmt *ElseStatement = IfStatement->getElse();
issueDiag(Result, FalseConditionRemoved->getBeginLoc(),
@@ -631,7 +650,7 @@ void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(
for (++After; After != Compound->body_end() && *Current != Ret;
++Current, ++After) {
if (const auto *If = dyn_cast<IfStmt>(*Current)) {
- if (const CXXBoolLiteralExpr *Lit = stmtReturnsBool(If, Negated)) {
+ if (const Expr *Lit = stmtReturnsBool(If, Negated)) {
if (*After == Ret) {
if (!ChainedConditionalReturn && BeforeIf)
continue;
diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
index 064953619f0b..af9017d4d3ae 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -51,11 +51,11 @@ class SimplifyBooleanExprCheck : public ClangTidyCheck {
void
replaceWithThenStatement(const ast_matchers::MatchFinder::MatchResult &Result,
- const CXXBoolLiteralExpr *BoolLiteral);
+ const Expr *BoolLiteral);
void
replaceWithElseStatement(const ast_matchers::MatchFinder::MatchResult &Result,
- const CXXBoolLiteralExpr *FalseConditionRemoved);
+ const Expr *FalseConditionRemoved);
void
replaceWithCondition(const ast_matchers::MatchFinder::MatchResult &Result,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
index 3f49eec4c88a..a00b733ec1ea 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
@@ -98,6 +98,46 @@ void if_with_bool_literal_condition() {
// CHECK-FIXES-NEXT: {{^ i = 11;$}}
}
+void if_with_negated_bool_condition() {
+ int i = 10;
+ if (!true) {
+ i = 11;
+ } else {
+ i = 12;
+ }
+ i = 13;
+ // CHECK-MESSAGES: :[[@LINE-6]]:7: warning: {{.*}} in if statement condition
+ // CHECK-FIXES: {{^ int i = 10;$}}
+ // CHECK-FIXES-NEXT: {{^ {$}}
+ // CHECK-FIXES-NEXT: {{^ i = 12;$}}
+ // CHECK-FIXES-NEXT: {{^ }$}}
+ // CHECK-FIXES-NEXT: {{^ i = 13;$}}
+
+ i = 14;
+ if (!false) {
+ i = 15;
+ } else {
+ i = 16;
+ }
+ i = 17;
+ // CHECK-MESSAGES: :[[@LINE-6]]:7: warning: {{.*}} in if statement condition
+ // CHECK-FIXES: {{^ i = 14;$}}
+ // CHECK-FIXES-NEXT: {{^ {$}}
+ // CHECK-FIXES-NEXT: {{^ i = 15;$}}
+ // CHECK-FIXES-NEXT: {{^ }$}}
+ // CHECK-FIXES-NEXT: {{^ i = 17;$}}
+
+ i = 18;
+ if (!true) {
+ i = 19;
+ }
+ i = 20;
+ // CHECK-MESSAGES: :[[@LINE-4]]:7: warning: {{.*}} in if statement condition
+ // CHECK-FIXES: {{^ i = 18;$}}
+ // CHECK-FIXES-NEXT: {{^ $}}
+ // CHECK-FIXES-NEXT: {{^ i = 20;$}}
+}
+
void operator_equals() {
int i = 0;
bool b1 = (i > 2);
More information about the cfe-commits
mailing list