[clang-tools-extra] r234626 - [clang-tidy] Add readability-simplify-boolean-expr check to clang-tidy
Alexander Kornienko
alexfh at google.com
Fri Apr 10 12:26:43 PDT 2015
Author: alexfh
Date: Fri Apr 10 14:26:43 2015
New Revision: 234626
URL: http://llvm.org/viewvc/llvm-project?rev=234626&view=rev
Log:
[clang-tidy] Add readability-simplify-boolean-expr check to clang-tidy
This check looks for comparisons between boolean expressions and boolean
constants and simplifies them to just use the appropriate boolean expression
directly.
if (b == true) becomes if (b)
if (b == false) becomes if (!b)
if (b && true) becomes if (b)
if (b && false) becomes if (false)
if (b || true) becomes if (true)
if (b || false) becomes if (b)
e ? true : false becomes e
e ? false : true becomes !e
if (true) t(); else f(); becomes t();
if (false) t(); else f(); becomes f();
if (e) return true; else return false; becomes return (e);
if (e) return false; else return true; becomes return !(e);
if (e) b = true; else b = false; becomes b = e;
if (e) b = false; else b = true; becomes b = !(e);
http://reviews.llvm.org/D7648
Patch by Richard Thomson!
Added:
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h
clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
Modified: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt?rev=234626&r1=234625&r2=234626&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt Fri Apr 10 14:26:43 2015
@@ -11,6 +11,7 @@ add_clang_library(clangTidyReadabilityMo
RedundantStringCStrCheck.cpp
RedundantSmartptrGetCheck.cpp
ShrinkToFitCheck.cpp
+ SimplifyBooleanExprCheck.cpp
LINK_LIBS
clangAST
Modified: clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp?rev=234626&r1=234625&r2=234626&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp Fri Apr 10 14:26:43 2015
@@ -18,6 +18,7 @@
#include "RedundantSmartptrGetCheck.h"
#include "RedundantStringCStrCheck.h"
#include "ShrinkToFitCheck.h"
+#include "SimplifyBooleanExprCheck.h"
namespace clang {
namespace tidy {
@@ -40,7 +41,10 @@ public:
"readability-redundant-smartptr-get");
CheckFactories.registerCheck<RedundantStringCStrCheck>(
"readability-redundant-string-cstr");
- CheckFactories.registerCheck<ShrinkToFitCheck>("readability-shrink-to-fit");
+ CheckFactories.registerCheck<ShrinkToFitCheck>(
+ "readability-shrink-to-fit");
+ CheckFactories.registerCheck<SimplifyBooleanExprCheck>(
+ "readability-simplify-boolean-expr");
}
};
Added: 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=234626&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp Fri Apr 10 14:26:43 2015
@@ -0,0 +1,375 @@
+//===--- SimplifyBooleanExpr.cpp clang-tidy ---------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "SimplifyBooleanExprCheck.h"
+#include "clang/Lex/Lexer.h"
+
+#include <cassert>
+#include <string>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+
+StringRef getText(const MatchFinder::MatchResult &Result, SourceRange Range) {
+ return Lexer::getSourceText(CharSourceRange::getTokenRange(Range),
+ *Result.SourceManager,
+ Result.Context->getLangOpts());
+}
+
+template <typename T>
+StringRef getText(const MatchFinder::MatchResult &Result, T &Node) {
+ return getText(Result, Node.getSourceRange());
+}
+
+const char RightExpressionId[] = "bool-op-expr-yields-expr";
+const char LeftExpressionId[] = "expr-op-bool-yields-expr";
+const char NegatedRightExpressionId[] = "bool-op-expr-yields-not-expr";
+const char NegatedLeftExpressionId[] = "expr-op-bool-yields-not-expr";
+const char ConditionThenStmtId[] = "if-bool-yields-then";
+const char ConditionElseStmtId[] = "if-bool-yields-else";
+const char TernaryId[] = "ternary-bool-yields-condition";
+const char TernaryNegatedId[] = "ternary-bool-yields-not-condition";
+const char IfReturnsBoolId[] = "if-return";
+const char IfReturnsNotBoolId[] = "if-not-return";
+const char ThenLiteralId[] = "then-literal";
+const char IfAssignVariableId[] = "if-assign-lvalue";
+const char IfAssignLocId[] = "if-assign-loc";
+const char IfAssignBoolId[] = "if-assign";
+const char IfAssignNotBoolId[] = "if-assign-not";
+const char IfAssignObjId[] = "if-assign-obj";
+
+const char IfStmtId[] = "if";
+const char LHSId[] = "lhs-expr";
+const char RHSId[] = "rhs-expr";
+
+const char SimplifyOperatorDiagnostic[] =
+ "redundant boolean literal supplied to boolean operator";
+const char SimplifyConditionDiagnostic[] =
+ "redundant boolean literal in if statement condition";
+
+const CXXBoolLiteralExpr *getBoolLiteral(const MatchFinder::MatchResult &Result,
+ StringRef Id) {
+ const auto *Literal = Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(Id);
+ return (Literal &&
+ Result.SourceManager->isMacroBodyExpansion(Literal->getLocStart()))
+ ? nullptr
+ : Literal;
+}
+
+internal::Matcher<Stmt> ReturnsBool(bool Value, StringRef Id = "") {
+ auto SimpleReturnsBool = returnStmt(
+ has(boolLiteral(equals(Value)).bind(Id.empty() ? "ignored" : Id)));
+ return anyOf(SimpleReturnsBool,
+ compoundStmt(statementCountIs(1), has(SimpleReturnsBool)));
+}
+
+bool needsParensAfterUnaryNegation(const Expr *E) {
+ if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))
+ return true;
+ if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E))
+ return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call &&
+ Op->getOperator() != OO_Subscript;
+ return false;
+}
+
+std::pair<BinaryOperatorKind, BinaryOperatorKind> Opposites[] = {
+ std::make_pair(BO_LT, BO_GE), std::make_pair(BO_GT, BO_LE),
+ std::make_pair(BO_EQ, BO_NE)};
+
+StringRef negatedOperator(const BinaryOperator *BinOp) {
+ const BinaryOperatorKind Opcode = BinOp->getOpcode();
+ for (auto NegatableOp : Opposites) {
+ if (Opcode == NegatableOp.first)
+ return BinOp->getOpcodeStr(NegatableOp.second);
+ if (Opcode == NegatableOp.second)
+ return BinOp->getOpcodeStr(NegatableOp.first);
+ }
+ return StringRef();
+}
+
+std::string replacementExpression(const MatchFinder::MatchResult &Result,
+ bool Negated, const Expr *E) {
+ while (const auto *Parenthesized = dyn_cast<ParenExpr>(E)) {
+ E = Parenthesized->getSubExpr();
+ }
+ if (Negated) {
+ if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {
+ StringRef NegatedOperator = negatedOperator(BinOp);
+ if (!NegatedOperator.empty()) {
+ return (getText(Result, *BinOp->getLHS()) + " " + NegatedOperator +
+ " " + getText(Result, *BinOp->getRHS()))
+ .str();
+ }
+ }
+ }
+ StringRef Text = getText(Result, *E);
+ return (Negated ? (needsParensAfterUnaryNegation(E) ? "!(" + Text + ")"
+ : "!" + Text)
+ : Text)
+ .str();
+}
+
+} // namespace
+
+void SimplifyBooleanExprCheck::matchBoolBinOpExpr(MatchFinder *Finder,
+ bool Value,
+ StringRef OperatorName,
+ StringRef BooleanId) {
+ Finder->addMatcher(
+ binaryOperator(isExpansionInMainFile(), hasOperatorName(OperatorName),
+ hasLHS(allOf(expr().bind(LHSId),
+ boolLiteral(equals(Value)).bind(BooleanId))),
+ hasRHS(expr().bind(RHSId)),
+ unless(hasRHS(hasDescendant(boolLiteral())))),
+ this);
+}
+
+void SimplifyBooleanExprCheck::matchExprBinOpBool(MatchFinder *Finder,
+ bool Value,
+ StringRef OperatorName,
+ StringRef BooleanId) {
+ Finder->addMatcher(
+ binaryOperator(
+ isExpansionInMainFile(), hasOperatorName(OperatorName),
+ hasLHS(expr().bind(LHSId)),
+ unless(hasLHS(anyOf(boolLiteral(), hasDescendant(boolLiteral())))),
+ hasRHS(allOf(expr().bind(RHSId),
+ boolLiteral(equals(Value)).bind(BooleanId)))),
+ this);
+}
+
+void SimplifyBooleanExprCheck::matchBoolCompOpExpr(MatchFinder *Finder,
+ bool Value,
+ StringRef OperatorName,
+ StringRef BooleanId) {
+ Finder->addMatcher(
+ binaryOperator(isExpansionInMainFile(), hasOperatorName(OperatorName),
+ hasLHS(allOf(expr().bind(LHSId),
+ ignoringImpCasts(boolLiteral(equals(Value))
+ .bind(BooleanId)))),
+ hasRHS(expr().bind(RHSId)),
+ unless(hasRHS(hasDescendant(boolLiteral())))),
+ this);
+}
+
+void SimplifyBooleanExprCheck::matchExprCompOpBool(MatchFinder *Finder,
+ bool Value,
+ StringRef OperatorName,
+ StringRef BooleanId) {
+ Finder->addMatcher(
+ binaryOperator(isExpansionInMainFile(), hasOperatorName(OperatorName),
+ unless(hasLHS(hasDescendant(boolLiteral()))),
+ hasLHS(expr().bind(LHSId)),
+ hasRHS(allOf(expr().bind(RHSId),
+ ignoringImpCasts(boolLiteral(equals(Value))
+ .bind(BooleanId))))),
+ this);
+}
+
+void SimplifyBooleanExprCheck::matchBoolCondition(MatchFinder *Finder,
+ bool Value,
+ StringRef BooleanId) {
+ Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+ hasCondition(boolLiteral(equals(Value))
+ .bind(BooleanId))).bind(IfStmtId),
+ this);
+}
+
+void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder,
+ bool Value,
+ StringRef TernaryId) {
+ Finder->addMatcher(
+ conditionalOperator(isExpansionInMainFile(),
+ hasTrueExpression(boolLiteral(equals(Value))),
+ hasFalseExpression(boolLiteral(equals(!Value))))
+ .bind(TernaryId),
+ this);
+}
+
+void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder,
+ bool Value, StringRef Id) {
+ Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+ hasThen(ReturnsBool(Value, ThenLiteralId)),
+ hasElse(ReturnsBool(!Value))).bind(Id),
+ this);
+}
+
+void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
+ bool Value, StringRef Id) {
+ auto SimpleThen = binaryOperator(
+ hasOperatorName("="),
+ hasLHS(declRefExpr(hasDeclaration(decl().bind(IfAssignObjId)))),
+ hasLHS(expr().bind(IfAssignVariableId)),
+ hasRHS(boolLiteral(equals(Value)).bind(IfAssignLocId)));
+ auto Then = anyOf(SimpleThen, compoundStmt(statementCountIs(1),
+ hasAnySubstatement(SimpleThen)));
+ auto SimpleElse = binaryOperator(
+ hasOperatorName("="),
+ hasLHS(declRefExpr(hasDeclaration(equalsBoundNode(IfAssignObjId)))),
+ hasRHS(boolLiteral(equals(!Value))));
+ auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
+ hasAnySubstatement(SimpleElse)));
+ Finder->addMatcher(
+ ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id),
+ this);
+}
+
+void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
+ matchBoolBinOpExpr(Finder, true, "&&", RightExpressionId);
+ matchBoolBinOpExpr(Finder, false, "||", RightExpressionId);
+ matchExprBinOpBool(Finder, false, "&&", RightExpressionId);
+ matchExprBinOpBool(Finder, true, "||", RightExpressionId);
+ matchBoolCompOpExpr(Finder, true, "==", RightExpressionId);
+ matchBoolCompOpExpr(Finder, false, "!=", RightExpressionId);
+
+ matchExprBinOpBool(Finder, true, "&&", LeftExpressionId);
+ matchExprBinOpBool(Finder, false, "||", LeftExpressionId);
+ matchBoolBinOpExpr(Finder, false, "&&", LeftExpressionId);
+ matchBoolBinOpExpr(Finder, true, "||", LeftExpressionId);
+ matchExprCompOpBool(Finder, true, "==", LeftExpressionId);
+ matchExprCompOpBool(Finder, false, "!=", LeftExpressionId);
+
+ matchBoolCompOpExpr(Finder, false, "==", NegatedRightExpressionId);
+ matchBoolCompOpExpr(Finder, true, "!=", NegatedRightExpressionId);
+
+ matchExprCompOpBool(Finder, false, "==", NegatedLeftExpressionId);
+ matchExprCompOpBool(Finder, true, "!=", NegatedLeftExpressionId);
+
+ matchBoolCondition(Finder, true, ConditionThenStmtId);
+ matchBoolCondition(Finder, false, ConditionElseStmtId);
+
+ matchTernaryResult(Finder, true, TernaryId);
+ matchTernaryResult(Finder, false, TernaryNegatedId);
+
+ matchIfReturnsBool(Finder, true, IfReturnsBoolId);
+ matchIfReturnsBool(Finder, false, IfReturnsNotBoolId);
+
+ matchIfAssignsBool(Finder, true, IfAssignBoolId);
+ matchIfAssignsBool(Finder, false, IfAssignNotBoolId);
+}
+
+void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const CXXBoolLiteralExpr *LeftRemoved =
+ getBoolLiteral(Result, RightExpressionId)) {
+ replaceWithExpression(Result, LeftRemoved, false);
+ } else if (const CXXBoolLiteralExpr *RightRemoved =
+ getBoolLiteral(Result, LeftExpressionId)) {
+ replaceWithExpression(Result, RightRemoved, true);
+ } else if (const CXXBoolLiteralExpr *NegatedLeftRemoved =
+ getBoolLiteral(Result, NegatedRightExpressionId)) {
+ replaceWithExpression(Result, NegatedLeftRemoved, false, true);
+ } else if (const CXXBoolLiteralExpr *NegatedRightRemoved =
+ getBoolLiteral(Result, NegatedLeftExpressionId)) {
+ replaceWithExpression(Result, NegatedRightRemoved, true, true);
+ } else if (const CXXBoolLiteralExpr *TrueConditionRemoved =
+ getBoolLiteral(Result, ConditionThenStmtId)) {
+ replaceWithThenStatement(Result, TrueConditionRemoved);
+ } else if (const CXXBoolLiteralExpr *FalseConditionRemoved =
+ getBoolLiteral(Result, ConditionElseStmtId)) {
+ replaceWithElseStatement(Result, FalseConditionRemoved);
+ } else if (const auto *Ternary =
+ Result.Nodes.getNodeAs<ConditionalOperator>(TernaryId)) {
+ replaceWithCondition(Result, Ternary);
+ } else if (const auto *TernaryNegated =
+ Result.Nodes.getNodeAs<ConditionalOperator>(
+ TernaryNegatedId)) {
+ replaceWithCondition(Result, TernaryNegated, true);
+ } else if (const auto *If = Result.Nodes.getNodeAs<IfStmt>(IfReturnsBoolId)) {
+ replaceWithReturnCondition(Result, If);
+ } else if (const auto *IfNot =
+ Result.Nodes.getNodeAs<IfStmt>(IfReturnsNotBoolId)) {
+ replaceWithReturnCondition(Result, IfNot, true);
+ } else if (const auto *IfAssign =
+ Result.Nodes.getNodeAs<IfStmt>(IfAssignBoolId)) {
+ replaceWithAssignment(Result, IfAssign);
+ } else if (const auto *IfAssignNot =
+ Result.Nodes.getNodeAs<IfStmt>(IfAssignNotBoolId)) {
+ replaceWithAssignment(Result, IfAssignNot, true);
+ }
+}
+
+void SimplifyBooleanExprCheck::replaceWithExpression(
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ const CXXBoolLiteralExpr *BoolLiteral, bool UseLHS, bool Negated) {
+ const auto *LHS = Result.Nodes.getNodeAs<Expr>(LHSId);
+ const auto *RHS = Result.Nodes.getNodeAs<Expr>(RHSId);
+ std::string Replacement =
+ replacementExpression(Result, Negated, UseLHS ? LHS : RHS);
+ SourceLocation Start = LHS->getLocStart();
+ SourceLocation End = RHS->getLocEnd();
+ diag(BoolLiteral->getLocStart(), SimplifyOperatorDiagnostic)
+ << FixItHint::CreateReplacement(SourceRange(Start, End), Replacement);
+}
+
+void SimplifyBooleanExprCheck::replaceWithThenStatement(
+ const MatchFinder::MatchResult &Result,
+ const CXXBoolLiteralExpr *TrueConditionRemoved) {
+ const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
+ diag(TrueConditionRemoved->getLocStart(), SimplifyConditionDiagnostic)
+ << FixItHint::CreateReplacement(IfStatement->getSourceRange(),
+ getText(Result, *IfStatement->getThen()));
+}
+
+void SimplifyBooleanExprCheck::replaceWithElseStatement(
+ const MatchFinder::MatchResult &Result,
+ const CXXBoolLiteralExpr *FalseConditionRemoved) {
+ const auto *IfStatement = Result.Nodes.getNodeAs<IfStmt>(IfStmtId);
+ const Stmt *ElseStatement = IfStatement->getElse();
+ diag(FalseConditionRemoved->getLocStart(), SimplifyConditionDiagnostic)
+ << FixItHint::CreateReplacement(
+ IfStatement->getSourceRange(),
+ ElseStatement ? getText(Result, *ElseStatement) : "");
+}
+
+void SimplifyBooleanExprCheck::replaceWithCondition(
+ const MatchFinder::MatchResult &Result, const ConditionalOperator *Ternary,
+ bool Negated) {
+ std::string Replacement =
+ replacementExpression(Result, Negated, Ternary->getCond());
+ diag(Ternary->getTrueExpr()->getLocStart(),
+ "redundant boolean literal in ternary expression result")
+ << FixItHint::CreateReplacement(Ternary->getSourceRange(), Replacement);
+}
+
+void SimplifyBooleanExprCheck::replaceWithReturnCondition(
+ const MatchFinder::MatchResult &Result, const IfStmt *If, bool Negated) {
+ StringRef Terminator = isa<CompoundStmt>(If->getElse()) ? ";" : "";
+ std::string Condition = replacementExpression(Result, Negated, If->getCond());
+ std::string Replacement = ("return " + Condition + Terminator).str();
+ SourceLocation Start =
+ Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(ThenLiteralId)->getLocStart();
+ diag(Start, "redundant boolean literal in conditional return statement")
+ << FixItHint::CreateReplacement(If->getSourceRange(), Replacement);
+}
+
+void SimplifyBooleanExprCheck::replaceWithAssignment(
+ const MatchFinder::MatchResult &Result, const IfStmt *IfAssign,
+ bool Negated) {
+ SourceRange Range = IfAssign->getSourceRange();
+ StringRef VariableName =
+ getText(Result, *Result.Nodes.getNodeAs<Expr>(IfAssignVariableId));
+ StringRef Terminator = isa<CompoundStmt>(IfAssign->getElse()) ? ";" : "";
+ std::string Condition =
+ replacementExpression(Result, Negated, IfAssign->getCond());
+ std::string Replacement =
+ (VariableName + " = " + Condition + Terminator).str();
+ SourceLocation Location =
+ Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(IfAssignLocId)->getLocStart();
+ this->diag(Location, "redundant boolean literal in conditional assignment")
+ << FixItHint::CreateReplacement(Range, Replacement);
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Added: 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=234626&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h Fri Apr 10 14:26:43 2015
@@ -0,0 +1,101 @@
+//===--- SimplifyBooleanExpr.h clang-tidy -----------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFY_BOOLEAN_EXPR_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFY_BOOLEAN_EXPR_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// \brief Looks for boolean expressions involving boolean constants and
+// simplifies them to use the appropriate boolean expression directly.
+///
+/// Examples:
+/// `if (b == true)` becomes `if (b)`
+/// `if (b == false)` becomes `if (!b)`
+/// `if (b && true)` becomes `if (b)`
+/// `if (b && false)` becomes `if (false)`
+/// `if (b || true)` becomes `if (true)`
+/// `if (b || false)` becomes `if (b)`
+/// `e ? true : false` becomes `e`
+/// `e ? false : true` becomes `!e`
+/// `if (true) t(); else f();` becomes `t();`
+/// `if (false) t(); else f();` becomes `f();`
+/// `if (e) return true; else return false;` becomes `return (e);`
+/// `if (e) return false; else return true;` becomes `return !(e);`
+/// `if (e) b = true; else b = false;` becomes `b = e;`
+/// `if (e) b = false; else b = true;` becomes `b = !(e);`
+///
+class SimplifyBooleanExprCheck : public ClangTidyCheck {
+public:
+ SimplifyBooleanExprCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ void matchBoolBinOpExpr(ast_matchers::MatchFinder *Finder, bool Value,
+ StringRef OperatorName, StringRef BooleanId);
+
+ void matchExprBinOpBool(ast_matchers::MatchFinder *Finder, bool Value,
+ StringRef OperatorName, StringRef BooleanId);
+
+ void matchBoolCompOpExpr(ast_matchers::MatchFinder *Finder, bool Value,
+ StringRef OperatorName, StringRef BooleanId);
+
+ void matchExprCompOpBool(ast_matchers::MatchFinder *Finder, bool Value,
+ StringRef OperatorName, StringRef BooleanId);
+
+ void matchBoolCondition(ast_matchers::MatchFinder *Finder, bool Value,
+ StringRef BooleanId);
+
+ void matchTernaryResult(ast_matchers::MatchFinder *Finder, bool Value,
+ StringRef TernaryId);
+
+ void matchIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
+ StringRef Id);
+
+ void matchIfAssignsBool(ast_matchers::MatchFinder *Finder, bool Value,
+ StringRef Id);
+
+ void
+ replaceWithExpression(const ast_matchers::MatchFinder::MatchResult &Result,
+ const CXXBoolLiteralExpr *BoolLiteral, bool UseLHS,
+ bool Negated = false);
+
+ void
+ replaceWithThenStatement(const ast_matchers::MatchFinder::MatchResult &Result,
+ const CXXBoolLiteralExpr *BoolLiteral);
+
+ void
+ replaceWithElseStatement(const ast_matchers::MatchFinder::MatchResult &Result,
+ const CXXBoolLiteralExpr *FalseConditionRemoved);
+
+ void
+ replaceWithCondition(const ast_matchers::MatchFinder::MatchResult &Result,
+ const ConditionalOperator *Ternary,
+ bool Negated = false);
+
+ void replaceWithReturnCondition(
+ const ast_matchers::MatchFinder::MatchResult &Result, const IfStmt *If,
+ bool Negated = false);
+
+ void
+ replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result,
+ const IfStmt *If, bool Negated = false);
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFY_BOOLEAN_EXPR_H
Added: 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=234626&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp Fri Apr 10 14:26:43 2015
@@ -0,0 +1,543 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-simplify-boolean-expr %t
+// REQUIRES: shell
+
+bool a1 = false;
+
+//=-=-=-=-=-=-= operator ==
+bool aa = false == a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^bool aa = !a1;$}}
+bool ab = true == a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool ab = a1;$}}
+bool a2 = a1 == false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a2 = !a1;$}}
+bool a3 = a1 == true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a3 = a1;$}}
+
+//=-=-=-=-=-=-= operator !=
+bool n1 = a1 != false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool n1 = a1;$}}
+bool n2 = a1 != true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool n2 = !a1;$}}
+bool n3 = false != a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool n3 = a1;$}}
+bool n4 = true != a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool n4 = !a1;$}}
+
+//=-=-=-=-=-=-= operator ||
+bool a4 = a1 || false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a4 = a1;$}}
+bool a5 = a1 || true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a5 = true;$}}
+bool a6 = false || a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a6 = a1;$}}
+bool a7 = true || a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a7 = true;$}}
+
+//=-=-=-=-=-=-= operator &&
+bool a8 = a1 && false;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a8 = false;$}}
+bool a9 = a1 && true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool a9 = a1;$}}
+bool ac = false && a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool ac = false;$}}
+bool ad = true && a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}} to boolean operator
+// CHECK-FIXES: {{^bool ad = a1;$}}
+
+void if_with_bool_literal_condition() {
+ int i = 0;
+ if (false) {
+ i = 1;
+ } else {
+ i = 2;
+ }
+ i = 3;
+ // CHECK-MESSAGES: :[[@LINE-6]]:7: warning: {{.*}} in if statement condition
+ // CHECK-FIXES: {{^ int i = 0;$}}
+ // CHECK-FIXES-NEXT: {{^ {$}}
+ // CHECK-FIXES-NEXT: {{^ i = 2;$}}
+ // CHECK-FIXES-NEXT: {{^ }$}}
+ // CHECK-FIXES-NEXT: {{^ i = 3;$}}
+
+ i = 4;
+ if (true) {
+ i = 5;
+ } else {
+ i = 6;
+ }
+ i = 7;
+ // CHECK-MESSAGES: :[[@LINE-6]]:7: warning: {{.*}} in if statement condition
+ // CHECK-FIXES: {{^ i = 4;$}}
+ // CHECK-FIXES-NEXT: {{^ {$}}
+ // CHECK-FIXES-NEXT: {{^ i = 5;$}}
+ // CHECK-FIXES-NEXT: {{^ }$}}
+ // CHECK-FIXES-NEXT: {{^ i = 7;$}}
+
+ i = 8;
+ if (false) {
+ i = 9;
+ }
+ i = 11;
+ // CHECK-MESSAGES: :[[@LINE-4]]:7: warning: {{.*}} in if statement condition
+ // CHECK-FIXES: {{^ i = 8;$}}
+ // CHECK-FIXES-NEXT: {{^ $}}
+ // CHECK-FIXES-NEXT: {{^ i = 11;$}}
+}
+
+void operator_equals() {
+ int i = 0;
+ bool b1 = (i > 2);
+ if (b1 == true) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(b1\) {$}}
+ i = 5;
+ } else {
+ i = 6;
+ }
+ bool b2 = (i > 4);
+ if (b2 == false) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(!b2\) {$}}
+ i = 7;
+ } else {
+ i = 9;
+ }
+ bool b3 = (i > 6);
+ if (true == b3) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(b3\) {$}}
+ i = 10;
+ } else {
+ i = 11;
+ }
+ bool b4 = (i > 8);
+ if (false == b4) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(!b4\) {$}}
+ i = 12;
+ } else {
+ i = 13;
+ }
+}
+
+void operator_or() {
+ int i = 0;
+ bool b5 = (i > 10);
+ if (b5 || false) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(b5\) {$}}
+ i = 14;
+ } else {
+ i = 15;
+ }
+ bool b6 = (i > 10);
+ if (b6 || true) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(true\) {$}}
+ i = 16;
+ } else {
+ i = 17;
+ }
+ bool b7 = (i > 10);
+ if (false || b7) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(b7\) {$}}
+ i = 18;
+ } else {
+ i = 19;
+ }
+ bool b8 = (i > 10);
+ if (true || b8) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(true\) {$}}
+ i = 20;
+ } else {
+ i = 21;
+ }
+}
+
+void operator_and() {
+ int i = 0;
+ bool b9 = (i > 20);
+ if (b9 && false) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(false\) {$}}
+ i = 22;
+ } else {
+ i = 23;
+ }
+ bool ba = (i > 20);
+ if (ba && true) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(ba\) {$}}
+ i = 24;
+ } else {
+ i = 25;
+ }
+ bool bb = (i > 20);
+ if (false && bb) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(false\) {$}}
+ i = 26;
+ } else {
+ i = 27;
+ }
+ bool bc = (i > 20);
+ if (true && bc) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(bc\) {$}}
+ i = 28;
+ } else {
+ i = 29;
+ }
+}
+
+void ternary_operator() {
+ int i = 0;
+ bool bd = (i > 20) ? true : false;
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: {{.*}} in ternary expression result
+ // CHECK-FIXES: {{^ bool bd = i > 20;$}}
+
+ bool be = (i > 20) ? false : true;
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: {{.*}} in ternary expression result
+ // CHECK-FIXES: {{^ bool be = i <= 20;$}}
+
+ bool bf = ((i > 20)) ? false : true;
+ // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: {{.*}} in ternary expression result
+ // CHECK-FIXES: {{^ bool bf = i <= 20;$}}
+}
+
+void operator_not_equal() {
+ int i = 0;
+ bool bf = (i > 20);
+ if (false != bf) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(bf\) {$}}
+ i = 30;
+ } else {
+ i = 31;
+ }
+ bool bg = (i > 20);
+ if (true != bg) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(!bg\) {$}}
+ i = 32;
+ } else {
+ i = 33;
+ }
+ bool bh = (i > 20);
+ if (bh != false) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(bh\) {$}}
+ i = 34;
+ } else {
+ i = 35;
+ }
+ bool bi = (i > 20);
+ if (bi != true) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(!bi\) {$}}
+ i = 36;
+ } else {
+ i = 37;
+ }
+}
+
+void nested_booleans() {
+ if (false || (true || false)) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(false \|\| \(true\)\) {$}}
+ }
+ if (true && (true || false)) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(true && \(true\)\) {$}}
+ }
+ if (false || (true && false)) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(false \|\| \(false\)\) {$}}
+ }
+ if (true && (true && false)) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(true && \(false\)\) {$}}
+ }
+}
+
+static constexpr bool truthy() {
+ return true;
+}
+
+#define HAS_XYZ_FEATURE true
+
+void macros_and_constexprs(int i = 0) {
+ bool b = (i == 1);
+ if (b && truthy()) {
+ // leave this alone; if you want it simplified, then you should
+ // inline the constexpr function first.
+ i = 1;
+ }
+ i = 2;
+ if (b && HAS_XYZ_FEATURE) {
+ // leave this alone; if you want it simplified, then you should
+ // inline the macro first.
+ i = 3;
+ }
+ i = 4;
+}
+
+bool conditional_return_statements(int i) {
+ if (i == 0) return true; else return false;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:22: warning: {{.*}} in conditional return statement
+// CHECK-FIXES: {{^}} return i == 0;{{$}}
+// CHECK-FIXES-NEXT: {{^}$}}
+
+bool conditional_return_statements_then_expr(int i, int j) {
+ if (i == j) return (i == 0); else return false;
+}
+
+bool conditional_return_statements_else_expr(int i, int j) {
+ if (i == j) return true; else return (i == 0);
+}
+
+bool negated_conditional_return_statements(int i) {
+ if (i == 0) return false; else return true;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:22: warning: {{.*}} in conditional return statement
+// CHECK-FIXES: {{^}} return i != 0;{{$}}
+// CHECK-FIXES-NEXT: {{^}$}}
+
+bool conditional_compound_return_statements(int i) {
+ if (i == 1) {
+ return true;
+ } else {
+ return false;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return statement
+// CHECK-FIXES: {{^}}bool conditional_compound_return_statements(int i) {{{$}}
+// CHECK-FIXES-NEXT: {{^}} return i == 1;{{$}}
+// CHECK-FIXES-NEXT: {{^}$}}
+
+bool negated_conditional_compound_return_statements(int i) {
+ if (i == 1) {
+ return false;
+ } else {
+ return true;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return statement
+// CHECK-FIXES: {{^}}bool negated_conditional_compound_return_statements(int i) {{{$}}
+// CHECK-FIXES-NEXT: {{^}} return i != 1;{{$}}
+// CHECK-FIXES-NEXT: {{^}$}}
+
+bool conditional_return_statements_side_effects_then(int i) {
+ if (i == 2) {
+ macros_and_constexprs();
+ return true;
+ } else
+ return false;
+}
+
+bool negated_conditional_return_statements_side_effects_then(int i) {
+ if (i == 2) {
+ macros_and_constexprs();
+ return false;
+ } else
+ return true;
+}
+
+bool conditional_return_statements_side_effects_else(int i) {
+ if (i == 2)
+ return true;
+ else {
+ macros_and_constexprs();
+ return false;
+ }
+}
+
+bool negated_conditional_return_statements_side_effects_else(int i) {
+ if (i == 2)
+ return false;
+ else {
+ macros_and_constexprs();
+ return true;
+ }
+}
+
+void lambda_conditional_return_statements() {
+ auto lambda = [](int n) -> bool { if (n > 0) return true; else return false; };
+ // CHECK-MESSAGES: :[[@LINE-1]]:55: warning: {{.*}} in conditional return statement
+ // CHECK-FIXES: {{^}} auto lambda = [](int n) -> bool { return n > 0; };{{$}}
+
+ auto lambda2 = [](int n) -> bool {
+ if (n > 0) {
+ return true;
+ } else {
+ return false;
+ }
+ };
+ // CHECK-MESSAGES: :[[@LINE-5]]:16: warning: {{.*}} in conditional return statement
+ // CHECK-FIXES: {{^}} auto lambda2 = [](int n) -> bool {{{$}}
+ // CHECK-FIXES-NEXT: {{^}} return n > 0;{{$}}
+ // CHECK-FIXES-NEXT: {{^}} };{{$}}
+
+ auto lambda3 = [](int n) -> bool { if (n > 0) {macros_and_constexprs(); return true; } else return false; };
+
+ auto lambda4 = [](int n) -> bool {
+ if (n > 0)
+ return true;
+ else {
+ macros_and_constexprs();
+ return false;
+ }
+ };
+
+ auto lambda5 = [](int n) -> bool { if (n > 0) return false; else return true; };
+ // CHECK-MESSAGES: :[[@LINE-1]]:56: warning: {{.*}} in conditional return statement
+ // CHECK-FIXES: {{^}} auto lambda5 = [](int n) -> bool { return n <= 0; };{{$}}
+
+ auto lambda6 = [](int n) -> bool {
+ if (n > 0) {
+ return false;
+ } else {
+ return true;
+ }
+ };
+ // CHECK-MESSAGES: :[[@LINE-5]]:16: warning: {{.*}} in conditional return statement
+ // CHECK-FIXES: {{^}} auto lambda6 = [](int n) -> bool {{{$}}
+ // CHECK-FIXES-NEXT: {{^}} return n <= 0;{{$}}
+ // CHECK-FIXES-NEXT: {{^}} };{{$}}
+}
+
+void simple_conditional_assignment_statements(int i) {
+ bool b;
+ if (i > 10)
+ b = true;
+ else
+ b = false;
+ bool bb = false;
+ // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: {{.*}} in conditional assignment
+ // CHECK-FIXES: bool b;
+ // CHECK-FIXES: {{^ }}b = i > 10;{{$}}
+ // CHECK-FIXES: bool bb = false;
+
+ bool c;
+ if (i > 20)
+ c = false;
+ else
+ c = true;
+ bool c2 = false;
+ // CHECK-MESSAGES: :[[@LINE-4]]:9: warning: {{.*}} in conditional assignment
+ // CHECK-FIXES: bool c;
+ // CHECK-FIXES: {{^ }}c = i <= 20;{{$}}
+ // CHECK-FIXES: bool c2 = false;
+
+ // unchanged; different variables
+ bool b2;
+ if (i > 12)
+ b = true;
+ else
+ b2 = false;
+
+ // unchanged; no else statement
+ bool b3;
+ if (i > 15)
+ b3 = true;
+
+ // unchanged; not boolean assignment
+ int j;
+ if (i > 17)
+ j = 10;
+ else
+ j = 20;
+
+ // unchanged; different variables assigned
+ int k = 0;
+ bool b4 = false;
+ if (i > 10)
+ b4 = true;
+ else
+ k = 10;
+}
+
+void complex_conditional_assignment_statements(int i) {
+ bool d;
+ if (i > 30) {
+ d = true;
+ } else {
+ d = false;
+ }
+ d = false;
+ // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in conditional assignment
+ // CHECK-FIXES: bool d;
+ // CHECK-FIXES: {{^ }}d = i > 30;{{$}}
+ // CHECK-FIXES: d = false;
+
+ bool e;
+ if (i > 40) {
+ e = false;
+ } else {
+ e = true;
+ }
+ e = false;
+ // CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in conditional assignment
+ // CHECK-FIXES: bool e;
+ // CHECK-FIXES: {{^ }}e = i <= 40;{{$}}
+ // CHECK-FIXES: e = false;
+
+ // unchanged; no else statement
+ bool b3;
+ if (i > 15) {
+ b3 = true;
+ }
+
+ // unchanged; not a boolean assignment
+ int j;
+ if (i > 17) {
+ j = 10;
+ } else {
+ j = 20;
+ }
+
+ // unchanged; multiple statements
+ bool f;
+ if (j > 10) {
+ j = 10;
+ f = true;
+ } else {
+ j = 20;
+ f = false;
+ }
+
+ // unchanged; multiple statements
+ bool g;
+ if (j > 10)
+ f = true;
+ else {
+ j = 20;
+ f = false;
+ }
+
+ // unchanged; multiple statements
+ bool h;
+ if (j > 10) {
+ j = 10;
+ f = true;
+ } else
+ f = false;
+}
More information about the cfe-commits
mailing list