[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: alexfh, klimek, aaron.ballman.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
njames93 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Reimplement most of the matching logic using Visitors instead of matchers.

Benchmarks from running the check over SemaCodeComplete.cpp
Before 1.02s, After 0.87s


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125026

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h

Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -34,73 +34,40 @@
 private:
   class Visitor;
 
-  void reportBinOp(const ast_matchers::MatchFinder::MatchResult &Result,
-   const BinaryOperator *Op);
-
-  void matchBoolCondition(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef BooleanId);
-
-  void matchTernaryResult(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
-
-  void matchIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
+  void reportBinOp(const ASTContext &Context, const BinaryOperator *Op);
 
   void matchIfAssignsBool(ast_matchers::MatchFinder *Finder, bool Value,
   StringRef Id);
 
-  void matchCompoundIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
-
-  void matchCaseIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
+  void replaceWithThenStatement(const ASTContext &Context,
+const IfStmt *IfStatement,
+const Expr *BoolLiteral);
 
-  void matchDefaultIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
- StringRef Id);
+  void replaceWithElseStatement(const ASTContext &Context,
+const IfStmt *IfStatement,
+const Expr *BoolLiteral);
 
-  void matchLabelIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-   StringRef Id);
+  void replaceWithCondition(const ASTContext &Context,
+const ConditionalOperator *Ternary, bool Negated);
 
-  void
-  replaceWithThenStatement(const ast_matchers::MatchFinder::MatchResult &Result,
-   const Expr *BoolLiteral);
-
-  void
-  replaceWithElseStatement(const ast_matchers::MatchFinder::MatchResult &Result,
-   const Expr *BoolLiteral);
-
-  void
-  replaceWithCondition(const ast_matchers::MatchFinder::MatchResult &Result,
-   const ConditionalOperator *Ternary, bool Negated);
-
-  void replaceWithReturnCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, const IfStmt *If,
-  bool Negated);
+  void replaceWithReturnCondition(const ASTContext &Context, const IfStmt *If,
+  const Expr *BoolLiteral, bool Negated);
 
   void
   replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result,
 const IfStmt *If, bool Negated);
 
-  void replaceCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result,
-  const CompoundStmt *Compound, bool Negated);
-
-  void replaceCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated,
-  const IfStmt *If);
-
-  void replaceCaseCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
-
-  void replaceDefaultCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
+  void replaceCompoundReturnWithCondition(const ASTContext &Context,
+  const CompoundStmt *Compound,
+  const ReturnStmt *Ret, bool Negated);
 
-  void replaceLabelCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
+  void replaceCompoundReturnWithCondition(const ASTContext &Context,
+  const ReturnStmt *Ret, bool Negated,
+  const IfStmt *If);
 
-  void issueDiag(const ast_matchers::MatchFinder::MatchResult &Result,
- SourceLocation Loc, StringRef Description,
- SourceRange ReplacementRange, StringRef Replacement);
+  void issueDiag(const ASTContext &Result, SourceLocation Loc

[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 427418.
njames93 added a comment.

Remove redundant traversal of CompoundStmt in a replace method.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125026/new/

https://reviews.llvm.org/D125026

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h

Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -34,73 +34,36 @@
 private:
   class Visitor;
 
-  void reportBinOp(const ast_matchers::MatchFinder::MatchResult &Result,
-   const BinaryOperator *Op);
-
-  void matchBoolCondition(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef BooleanId);
-
-  void matchTernaryResult(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
-
-  void matchIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
+  void reportBinOp(const ASTContext &Context, const BinaryOperator *Op);
 
   void matchIfAssignsBool(ast_matchers::MatchFinder *Finder, bool Value,
   StringRef Id);
 
-  void matchCompoundIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
-
-  void matchCaseIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
-
-  void matchDefaultIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
- StringRef Id);
-
-  void matchLabelIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-   StringRef Id);
+  void replaceWithThenStatement(const ASTContext &Context,
+const IfStmt *IfStatement,
+const Expr *BoolLiteral);
 
-  void
-  replaceWithThenStatement(const ast_matchers::MatchFinder::MatchResult &Result,
-   const Expr *BoolLiteral);
-
-  void
-  replaceWithElseStatement(const ast_matchers::MatchFinder::MatchResult &Result,
-   const Expr *BoolLiteral);
+  void replaceWithElseStatement(const ASTContext &Context,
+const IfStmt *IfStatement,
+const Expr *BoolLiteral);
 
-  void
-  replaceWithCondition(const ast_matchers::MatchFinder::MatchResult &Result,
-   const ConditionalOperator *Ternary, bool Negated);
+  void replaceWithCondition(const ASTContext &Context,
+const ConditionalOperator *Ternary, bool Negated);
 
-  void replaceWithReturnCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, const IfStmt *If,
-  bool Negated);
+  void replaceWithReturnCondition(const ASTContext &Context, const IfStmt *If,
+  const Expr *BoolLiteral, bool Negated);
 
   void
   replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result,
 const IfStmt *If, bool Negated);
 
-  void replaceCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result,
-  const CompoundStmt *Compound, bool Negated);
-
-  void replaceCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated,
-  const IfStmt *If);
-
-  void replaceCaseCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
-
-  void replaceDefaultCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
-
-  void replaceLabelCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
+  void replaceCompoundReturnWithCondition(const ASTContext &Context,
+  const ReturnStmt *Ret, bool Negated,
+  const IfStmt *If);
 
-  void issueDiag(const ast_matchers::MatchFinder::MatchResult &Result,
- SourceLocation Loc, StringRef Description,
- SourceRange ReplacementRange, StringRef Replacement);
+  void issueDiag(const ASTContext &Result, SourceLocation Loc,
+ StringRef Description, SourceRange ReplacementRange,
+ StringRef Replacement);
 
   const bool ChainedConditionalReturn;
   const bool ChainedConditionalAssignment;
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBoolea

[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 427430.
njames93 added a comment.

Transform final matcher, Benchmark -> 0.82s.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125026/new/

https://reviews.llvm.org/D125026

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h

Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -10,6 +10,8 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SIMPLIFY_BOOLEAN_EXPR_H
 
 #include "../ClangTidyCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Stmt.h"
 
 namespace clang {
 namespace tidy {
@@ -34,73 +36,32 @@
 private:
   class Visitor;
 
-  void reportBinOp(const ast_matchers::MatchFinder::MatchResult &Result,
-   const BinaryOperator *Op);
+  void reportBinOp(const ASTContext &Context, const BinaryOperator *Op);
 
-  void matchBoolCondition(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef BooleanId);
+  void replaceWithThenStatement(const ASTContext &Context,
+const IfStmt *IfStatement,
+const Expr *BoolLiteral);
 
-  void matchTernaryResult(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
+  void replaceWithElseStatement(const ASTContext &Context,
+const IfStmt *IfStatement,
+const Expr *BoolLiteral);
 
-  void matchIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
+  void replaceWithCondition(const ASTContext &Context,
+const ConditionalOperator *Ternary, bool Negated);
 
-  void matchIfAssignsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
+  void replaceWithReturnCondition(const ASTContext &Context, const IfStmt *If,
+  const Expr *BoolLiteral, bool Negated);
 
-  void matchCompoundIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
+  void replaceWithAssignment(const ASTContext &Context, const IfStmt *If,
+ const Expr *Var, SourceLocation Loc, bool Negated);
 
-  void matchCaseIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
+  void replaceCompoundReturnWithCondition(const ASTContext &Context,
+  const ReturnStmt *Ret, bool Negated,
+  const IfStmt *If);
 
-  void matchDefaultIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
- StringRef Id);
-
-  void matchLabelIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-   StringRef Id);
-
-  void
-  replaceWithThenStatement(const ast_matchers::MatchFinder::MatchResult &Result,
-   const Expr *BoolLiteral);
-
-  void
-  replaceWithElseStatement(const ast_matchers::MatchFinder::MatchResult &Result,
-   const Expr *BoolLiteral);
-
-  void
-  replaceWithCondition(const ast_matchers::MatchFinder::MatchResult &Result,
-   const ConditionalOperator *Ternary, bool Negated);
-
-  void replaceWithReturnCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, const IfStmt *If,
-  bool Negated);
-
-  void
-  replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result,
-const IfStmt *If, bool Negated);
-
-  void replaceCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result,
-  const CompoundStmt *Compound, bool Negated);
-
-  void replaceCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated,
-  const IfStmt *If);
-
-  void replaceCaseCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
-
-  void replaceDefaultCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
-
-  void replaceLabelCompoundReturnWithCondition(
-  const ast_matchers::MatchFinder::MatchResult &Result, bool Negated);
-
-  void issueDiag(const ast_matchers::MatchFinder::MatchResult &Result,
- SourceLocation Loc, StringRef Description,
- SourceRange ReplacementRange, StringRef Replacement);
+  void issueDiag(const ASTContext &Result, SourceLocation Loc,
+ StringRef Description, SourceRange ReplacementRange,
+ StringRef Replac

[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 427436.
njames93 added a comment.

Remove SimplifyBoolExprMatchers header


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125026/new/

https://reviews.llvm.org/D125026

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprMatchers.h

Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprMatchers.h
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprMatchers.h
+++ /dev/null
@@ -1,68 +0,0 @@
-//===-- SimplifyBooleanExprMatchers.h - clang-tidy ===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/ASTMatchers/ASTMatchersInternal.h"
-#include "clang/ASTMatchers/ASTMatchersMacros.h"
-
-namespace clang {
-namespace ast_matchers {
-
-/// Matches the substatement associated with a case, default or label statement.
-///
-/// Given
-/// \code
-///   switch (1) { case 1: break; case 2: return; break; default: return; break;
-///   }
-///   foo: return;
-///   bar: break;
-/// \endcode
-///
-/// caseStmt(hasSubstatement(returnStmt()))
-///   matches "case 2: return;"
-/// defaultStmt(hasSubstatement(returnStmt()))
-///   matches "default: return;"
-/// labelStmt(hasSubstatement(breakStmt()))
-///   matches "bar: break;"
-AST_POLYMORPHIC_MATCHER_P(hasSubstatement,
-  AST_POLYMORPHIC_SUPPORTED_TYPES(CaseStmt, DefaultStmt,
-  LabelStmt),
-  internal::Matcher, InnerMatcher) {
-  return InnerMatcher.matches(*Node.getSubStmt(), Finder, Builder);
-}
-
-/// Matches two consecutive statements within a compound statement.
-///
-/// Given
-/// \code
-///   { if (x > 0) return true; return false; }
-/// \endcode
-/// compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt()))
-///   matches '{ if (x > 0) return true; return false; }'
-AST_POLYMORPHIC_MATCHER_P2(hasSubstatementSequence,
-   AST_POLYMORPHIC_SUPPORTED_TYPES(CompoundStmt,
-   StmtExpr),
-   internal::Matcher, InnerMatcher1,
-   internal::Matcher, InnerMatcher2) {
-  if (const CompoundStmt *CS = CompoundStmtMatcher::get(Node)) {
-auto It = matchesFirstInPointerRange(InnerMatcher1, CS->body_begin(),
- CS->body_end(), Finder, Builder);
-while (It != CS->body_end()) {
-  ++It;
-  if (It == CS->body_end())
-return false;
-  if (InnerMatcher2.matches(**It, Finder, Builder))
-return true;
-  It = matchesFirstInPointerRange(InnerMatcher1, It, CS->body_end(), Finder,
-  Builder);
-}
-  }
-  return false;
-}
-
-} // namespace ast_matchers
-} // namespace clang
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -34,73 +34,32 @@
 private:
   class Visitor;
 
-  void reportBinOp(const ast_matchers::MatchFinder::MatchResult &Result,
-   const BinaryOperator *Op);
+  void reportBinOp(const ASTContext &Context, const BinaryOperator *Op);
 
-  void matchBoolCondition(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef BooleanId);
+  void replaceWithThenStatement(const ASTContext &Context,
+const IfStmt *IfStatement,
+const Expr *BoolLiteral);
 
-  void matchTernaryResult(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
+  void replaceWithElseStatement(const ASTContext &Context,
+const IfStmt *IfStatement,
+const Expr *BoolLiteral);
 
-  void matchIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
+  void replaceWithCondition(const ASTContext &Context,
+const ConditionalOperator *Ternary, bool Negated);
 
-  void matchIfAssignsBool(ast_matchers::MatchFinder *Finder, bool Value,
-  StringRef Id);
+  void replaceWithReturnCondition(const ASTContext &Context, const IfStmt *If,
+  const Expr *BoolLiteral, bool Negated);
 
-  void matchCompoundIfRetu

[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 427529.
njames93 added a comment.

Remove unnecessary tests in ReadabilityTidyModule now that the 
SimplifyBooleanExprMatchers header has been removed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125026/new/

https://reviews.llvm.org/D125026

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprMatchers.h
  clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
@@ -2,8 +2,6 @@
 #include "ClangTidyTest.h"
 #include "readability/BracesAroundStatementsCheck.h"
 #include "readability/NamespaceCommentCheck.h"
-#include "readability/SimplifyBooleanExprMatchers.h"
-#include "clang/ASTMatchers/ASTMatchers.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -14,91 +12,6 @@
 using readability::NamespaceCommentCheck;
 using namespace ast_matchers;
 
-TEST_P(ASTMatchersTest, HasCaseSubstatement) {
-  EXPECT_TRUE(matches(
-  "void f() { switch (1) { case 1: return; break; default: break; } }",
-  traverse(TK_AsIs, caseStmt(hasSubstatement(returnStmt());
-}
-
-TEST_P(ASTMatchersTest, HasDefaultSubstatement) {
-  EXPECT_TRUE(matches(
-  "void f() { switch (1) { case 1: return; break; default: break; } }",
-  traverse(TK_AsIs, defaultStmt(hasSubstatement(breakStmt());
-}
-
-TEST_P(ASTMatchersTest, HasLabelSubstatement) {
-  EXPECT_TRUE(
-  matches("void f() { while (1) { bar: break; foo: return; } }",
-  traverse(TK_AsIs, labelStmt(hasSubstatement(breakStmt());
-}
-
-TEST_P(ASTMatchersTest, HasSubstatementSequenceSimple) {
-  const char *Text = "int f() { int x = 5; if (x < 0) return 1; return 0; }";
-  EXPECT_TRUE(matches(
-  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
-  EXPECT_FALSE(matches(
-  Text, compoundStmt(hasSubstatementSequence(ifStmt(), labelStmt();
-  EXPECT_FALSE(matches(
-  Text, compoundStmt(hasSubstatementSequence(returnStmt(), ifStmt();
-  EXPECT_FALSE(matches(
-  Text, compoundStmt(hasSubstatementSequence(switchStmt(), labelStmt();
-}
-
-TEST_P(ASTMatchersTest, HasSubstatementSequenceAlmost) {
-  const char *Text = R"code(
-int f() {
-  int x = 5;
-  if (x < 10)
-;
-  if (x < 0)
-return 1;
-  return 0;
-}
-)code";
-  EXPECT_TRUE(matches(
-  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
-  EXPECT_TRUE(
-  matches(Text, compoundStmt(hasSubstatementSequence(ifStmt(), ifStmt();
-}
-
-TEST_P(ASTMatchersTest, HasSubstatementSequenceComplex) {
-  const char *Text = R"code(
-int f() {
-  int x = 5;
-  if (x < 10)
-x -= 10;
-  if (x < 0)
-return 1;
-  return 0;
-}
-)code";
-  EXPECT_TRUE(matches(
-  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
-  EXPECT_FALSE(
-  matches(Text, compoundStmt(hasSubstatementSequence(ifStmt(), expr();
-}
-
-TEST_P(ASTMatchersTest, HasSubstatementSequenceExpression) {
-  const char *Text = R"code(
-int f() {
-  return ({ int x = 5;
-  int result;
-  if (x < 10)
-x -= 10;
-  if (x < 0)
-result = 1;
-  else
-result = 0;
-  result;
-});
-  }
-)code";
-  EXPECT_TRUE(
-  matches(Text, stmtExpr(hasSubstatementSequence(ifStmt(), expr();
-  EXPECT_FALSE(
-  matches(Text, stmtExpr(hasSubstatementSequence(ifStmt(), returnStmt();
-}
-
 // Copied from ASTMatchersTests
 static std::vector allTestClangConfigs() {
   std::vector all_configs;
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprMatchers.h
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprMatchers.h
+++ /dev/null
@@ -1,68 +0,0 @@
-//===-- SimplifyBooleanExprMatchers.h - clang-tidy ===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/ASTMatchers/ASTMatchersInternal.h"
-#include "clang/ASTMatchers/ASTMatchersMacros.h"
-
-namespace clang {
-namespace ast_matchers {
-
-/// Matches the substatement associated with a case, default or label statement.
-///
-/// Given
-/// \code
-///   switch (1) { case 1: break; case 2: return; break; default: return; break;
-///   }
-///   foo: return;
-///   bar: break;
-/// \endcode
-///
-/// caseStmt(hasSubstatement(returnStmt()))
-///   matches "

[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 428041.
njames93 added a comment.

Clean up some code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125026/new/

https://reviews.llvm.org/D125026

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprMatchers.h
  clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
@@ -2,8 +2,6 @@
 #include "ClangTidyTest.h"
 #include "readability/BracesAroundStatementsCheck.h"
 #include "readability/NamespaceCommentCheck.h"
-#include "readability/SimplifyBooleanExprMatchers.h"
-#include "clang/ASTMatchers/ASTMatchers.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -14,91 +12,6 @@
 using readability::NamespaceCommentCheck;
 using namespace ast_matchers;
 
-TEST_P(ASTMatchersTest, HasCaseSubstatement) {
-  EXPECT_TRUE(matches(
-  "void f() { switch (1) { case 1: return; break; default: break; } }",
-  traverse(TK_AsIs, caseStmt(hasSubstatement(returnStmt());
-}
-
-TEST_P(ASTMatchersTest, HasDefaultSubstatement) {
-  EXPECT_TRUE(matches(
-  "void f() { switch (1) { case 1: return; break; default: break; } }",
-  traverse(TK_AsIs, defaultStmt(hasSubstatement(breakStmt());
-}
-
-TEST_P(ASTMatchersTest, HasLabelSubstatement) {
-  EXPECT_TRUE(
-  matches("void f() { while (1) { bar: break; foo: return; } }",
-  traverse(TK_AsIs, labelStmt(hasSubstatement(breakStmt());
-}
-
-TEST_P(ASTMatchersTest, HasSubstatementSequenceSimple) {
-  const char *Text = "int f() { int x = 5; if (x < 0) return 1; return 0; }";
-  EXPECT_TRUE(matches(
-  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
-  EXPECT_FALSE(matches(
-  Text, compoundStmt(hasSubstatementSequence(ifStmt(), labelStmt();
-  EXPECT_FALSE(matches(
-  Text, compoundStmt(hasSubstatementSequence(returnStmt(), ifStmt();
-  EXPECT_FALSE(matches(
-  Text, compoundStmt(hasSubstatementSequence(switchStmt(), labelStmt();
-}
-
-TEST_P(ASTMatchersTest, HasSubstatementSequenceAlmost) {
-  const char *Text = R"code(
-int f() {
-  int x = 5;
-  if (x < 10)
-;
-  if (x < 0)
-return 1;
-  return 0;
-}
-)code";
-  EXPECT_TRUE(matches(
-  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
-  EXPECT_TRUE(
-  matches(Text, compoundStmt(hasSubstatementSequence(ifStmt(), ifStmt();
-}
-
-TEST_P(ASTMatchersTest, HasSubstatementSequenceComplex) {
-  const char *Text = R"code(
-int f() {
-  int x = 5;
-  if (x < 10)
-x -= 10;
-  if (x < 0)
-return 1;
-  return 0;
-}
-)code";
-  EXPECT_TRUE(matches(
-  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
-  EXPECT_FALSE(
-  matches(Text, compoundStmt(hasSubstatementSequence(ifStmt(), expr();
-}
-
-TEST_P(ASTMatchersTest, HasSubstatementSequenceExpression) {
-  const char *Text = R"code(
-int f() {
-  return ({ int x = 5;
-  int result;
-  if (x < 10)
-x -= 10;
-  if (x < 0)
-result = 1;
-  else
-result = 0;
-  result;
-});
-  }
-)code";
-  EXPECT_TRUE(
-  matches(Text, stmtExpr(hasSubstatementSequence(ifStmt(), expr();
-  EXPECT_FALSE(
-  matches(Text, stmtExpr(hasSubstatementSequence(ifStmt(), returnStmt();
-}
-
 // Copied from ASTMatchersTests
 static std::vector allTestClangConfigs() {
   std::vector all_configs;
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprMatchers.h
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprMatchers.h
+++ /dev/null
@@ -1,68 +0,0 @@
-//===-- SimplifyBooleanExprMatchers.h - clang-tidy ===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/ASTMatchers/ASTMatchersInternal.h"
-#include "clang/ASTMatchers/ASTMatchersMacros.h"
-
-namespace clang {
-namespace ast_matchers {
-
-/// Matches the substatement associated with a case, default or label statement.
-///
-/// Given
-/// \code
-///   switch (1) { case 1: break; case 2: return; break; default: return; break;
-///   }
-///   foo: return;
-///   bar: break;
-/// \endcode
-///
-/// caseStmt(hasSubstatement(returnStmt()))
-///   matches "case 2: return;"
-/// defaultStmt(hasSubstatement(returnStmt()))
-///   matches "default: return;