[clang-tidy] Add trace code for ASTVisitors

[njames93]

Summary

This was some code I wrote to help debug crashes when refactoring the SimplifyBooleanExpr check.
I feel like this could help fix other potential crashes in checks that make use of ASTVisitors.
It would also bring the crashing debug info more in line with what we have for matcher based checks.

clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp

 //===-- SimplifyBooleanExprCheck.cpp - 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 "SimplifyBooleanExprCheck.h"
+#include "../utils/VisitorTrace.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/Support/SaveAndRestore.h"
 
 #include <string>
 #include <utility>
 
 using namespace clang::ast_matchers;
public:
   bool dataTraverseStmtPost(Stmt *S) {
     if (S && !shouldIgnore(S)) {
       assert(StmtStack.back() == S);
       StmtStack.pop_back();
     }
     return true;
   }
 
-  bool VisitBinaryOperator(const BinaryOperator *Op) const {
+  bool VisitBinaryOperator(const BinaryOperator *Op) {
+    auto Trace = Tracer.enter(Op);
     Check->reportBinOp(Context, Op);
     return true;
   }
 
   // Extracts a bool if an expression is (true|false|!true|!false);
   static Optional<bool> getAsBoolLiteral(const Expr *E, bool FilterMacro) {
     if (const auto *Bool = dyn_cast<CXXBoolLiteralExpr>(E)) {
       if (FilterMacro && Bool->getBeginLoc().isMacroID())
static auto checkSingleStatement(Stmt *S, Functor F) -> decltype(F(S)) {
     return F(S);
   }
 
   Stmt *parent() const {
     return StmtStack.size() < 2 ? nullptr : StmtStack[StmtStack.size() - 2];
   }
 
   bool VisitIfStmt(IfStmt *If) {
+    auto Trace = Tracer.enter(If);
     // Skip any if's that have a condition var or an init statement.
     if (If->hasInitStorage() || If->hasVarStorage())
       return true;
     /*
      * if (true) ThenStmt(); -> ThenStmt();
      * if (false) ThenStmt(); -> <Empty>;
      * if (false) ThenStmt(); else ElseStmt() -> ElseStmt();
      */
if (If->getElse()) {
           }
         }
       }
     }
     return true;
   }
 
   bool VisitConditionalOperator(ConditionalOperator *Cond) {
+    auto Trace = Tracer.enter(Cond);
     /*
      * Condition ? true : false; -> Condition
      * Condition ? false : true; -> !Condition;
      */
     if (Optional<bool> Then =
             getAsBoolLiteral(Cond->getTrueExpr()->IgnoreImplicit(), false)) {
       if (Optional<bool> Else =
               getAsBoolLiteral(Cond->getFalseExpr()->IgnoreImplicit(), false)) {
         if (*Then != *Else)
           Check->replaceWithCondition(Context, Cond, *Else);
       }
     }
     return true;
   }
 
   bool VisitCompoundStmt(CompoundStmt *CS) {
+    auto Trace = Tracer.enter(CS);
     if (CS->size() < 2)
       return true;
     bool CurIf = false, PrevIf = false;
     for (auto First = CS->body_begin(), Second = std::next(First),
               End = CS->body_end();
          Second != End; ++Second, ++First) {
       PrevIf = CurIf;
       CurIf = isa<IfStmt>(*First);
case BO_LOr:
       }
       return false;
     default:
       return false;
     }
   }
 
   bool TraverseUnaryOperator(UnaryOperator *Op) {
+    auto Trace = Tracer.enter(Op);
     if (!Check->SimplifyDeMorgan || Op->getOpcode() != UO_LNot)
       return Base::TraverseUnaryOperator(Op);
     Expr *SubImp = Op->getSubExpr()->IgnoreImplicit();
     auto *Parens = dyn_cast<ParenExpr>(SubImp);
     auto *BinaryOp =
         Parens
             ? dyn_cast<BinaryOperator>(Parens->getSubExpr()->IgnoreImplicit())
             : dyn_cast<BinaryOperator>(SubImp);
bool TraverseUnaryOperator(UnaryOperator *Op) {
     return Base::TraverseUnaryOperator(Op);
   }
 
 private:
   bool IsProcessing = false;
   SimplifyBooleanExprCheck *Check;
   SmallVector<Stmt *, 32> StmtStack;
   ASTContext &Context;
+  VisitorTrace Tracer{*Check, Context};
 };
 
 SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,
                                                    ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       ChainedConditionalReturn(Options.get("ChainedConditionalReturn", false)),
       ChainedConditionalAssignment(
           Options.get("ChainedConditionalAssignment", false)),

clang-tools-extra/clang-tidy/utils/CMakeLists.txt

add_clang_library(clangTidyUtils
   IncludeSorter.cpp
   LexerUtils.cpp
   NamespaceAliaser.cpp
   OptionsUtils.cpp
   RenamerClangTidyCheck.cpp
   TransformerClangTidyCheck.cpp
   TypeTraits.cpp
   UsingInserter.cpp
+  VisitorTrace.cpp
 
   LINK_LIBS
   clangTidy
 
   DEPENDS
   omp_gen
   )
 

clang-tools-extra/clang-tidy/utils/VisitorTrace.h

+//===---------- VisitorTrace.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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_VISITORTRACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_VISITORTRACE_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/PrettyStackTrace.h"
+
+namespace clang {
+namespace tidy {
+
+class VisitorTrace : public llvm::PrettyStackTraceEntry {
+  // These 3 types would cover pretty much all uses.
+  using PtrTy = llvm::PointerUnion<const Decl *, const Stmt *, const Type *>;
+  // Most uses in tidy would only ever need to store one item in here.
+  using StackTy = llvm::SmallVector<PtrTy, 2>;
+
+public:
+  VisitorTrace(llvm::StringLiteral ID, const ASTContext &Ctx)
+      : ID(ID), Ctx(Ctx) {}
+  VisitorTrace(ClangTidyCheck &Check, const ASTContext &Ctx)
+      // FIXME: This cast makes no sense, why does ClangTidyCheck hide the
+      // virtual getID call?
+      : ID(static_cast<ast_matchers::MatchFinder::MatchCallback &>(Check)
+               .getID()),
+        Ctx(Ctx) {}
+
+  struct LLVM_NODISCARD PushStateRAII {
+    friend class VisitorTrace;
+
+  private:
+    PushStateRAII(StackTy &Stack, PtrTy Ty) : Stack(Stack) {
+      assert(!Ty.isNull());
+      Stack.push_back(Ty);
+    }
+
+  public:
+    ~PushStateRAII() { Stack.pop_back(); }
+
+    StackTy &Stack;
+  };
+
+  PushStateRAII enter(const Decl *D) { return {Stack, D}; }
+
+  PushStateRAII enter(const Stmt *S) { return {Stack, S}; }
+
+  PushStateRAII enter(const Type *T) { return {Stack, T}; }
+
+  void print(llvm::raw_ostream &OS) const override;
+
+private:
+  llvm::StringRef ID;
+  const ASTContext &Ctx;
+  StackTy Stack;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif

clang-tools-extra/clang-tidy/utils/VisitorTrace.cpp

+//===---------- VisitorTrace.cpp - 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 "VisitorTrace.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang;
+using namespace clang::tidy;
+
+static void printStackEntry(llvm::raw_ostream &OS, const Decl *D,
+                            const ASTContext &Ctx) {
+  OS << D->getDeclKindName() << "Decl ";
+  if (const auto *ND = llvm::dyn_cast<NamedDecl>(D)) {
+    ND->printQualifiedName(OS);
+    OS << " - ";
+  } else
+    OS << "- ";
+  D->getSourceRange().print(OS, Ctx.getSourceManager());
+}
+
+static void printStackEntry(llvm::raw_ostream &OS, const Stmt *S,
+                            const ASTContext &Ctx) {
+  OS << S->getStmtClassName() << " - ";
+  S->getSourceRange().print(OS, Ctx.getSourceManager());
+}
+
+static void printStackEntry(llvm::raw_ostream &OS, const Type *T,
+                            const ASTContext &Ctx) {
+  OS << T->getTypeClassName() << "Type - ";
+  QualType(T, 0).print(OS, Ctx.getPrintingPolicy());
+}
+
+void VisitorTrace::print(llvm::raw_ostream &OS) const {
+  OS << "Running AST visitor for '" << ID << "'\n";
+  if (Stack.empty()) {
+    OS << "No children visited\n";
+    return;
+  }
+  unsigned Idx = 0;
+  OS << "Visiting:";
+  for (const PtrTy &Item : llvm::reverse(Stack)) {
+    OS << "\n  #" << ++Idx << ": ";
+    if (const auto *D = Item.dyn_cast<const Decl *>())
+      printStackEntry(OS, D, Ctx);
+    else if (const auto *S = Item.dyn_cast<const Stmt *>())
+      printStackEntry(OS, S, Ctx);
+    else
+      printStackEntry(OS, Item.get<const Type *>(), Ctx);
+  }
+  OS << "\n";
+}

clang-tools-extra/unittests/clang-tidy/CMakeLists.txt

add_extra_unittest(ClangTidyTests
   ModernizeModuleTest.cpp
   NamespaceAliaserTest.cpp
   ObjCModuleTest.cpp
   OptionsProviderTest.cpp
   OverlappingReplacementsTest.cpp
   UsingInserterTest.cpp
   ReadabilityModuleTest.cpp
   TransformerClangTidyCheckTest.cpp
+  VisitorTraceTest.cpp
   )
 
 clang_target_link_libraries(ClangTidyTests
   PRIVATE
   clangAST
   clangASTMatchers
   clangBasic
   clangFrontend

clang-tools-extra/unittests/clang-tidy/VisitorTraceTest.cpp

+//===------- VisitorTraceTest.cpp - 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 "utils/VisitorTrace.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+#include <memory>
+#include <utility>
+
+namespace clang {
+namespace tidy {
+
+constexpr llvm::StringLiteral Bind = "Bound";
+
+using namespace clang::ast_matchers;
+
+namespace {
+struct ASTAndTracer {
+  std::unique_ptr<ASTUnit> AST;
+  VisitorTrace Tracer;
+
+  ASTAndTracer(std::unique_ptr<ASTUnit> AST)
+      : AST(std::move(AST)), Tracer("Test", this->AST->getASTContext()) {}
+
+  ASTAndTracer(StringRef Code)
+      : ASTAndTracer(tooling::buildASTFromCodeWithArgs(Code, {"-std=c++11"})) {}
+
+  template <typename NodeT>
+  const NodeT *getNode(internal::BindableMatcher<NodeT> Matcher) {
+    return selectFirst<NodeT>(Bind,
+                              match(Matcher.bind(Bind), AST->getASTContext()));
+  }
+
+  template <typename NodeT, typename MatcherT>
+  const NodeT *getNode(StringRef ID, internal::Matcher<MatcherT> Matcher) {
+    return selectFirst<NodeT>(ID, match(Matcher, AST->getASTContext()));
+  }
+
+  std::string dumpVisitor() {
+    std::string S;
+    llvm::raw_string_ostream OS(S);
+    Tracer.print(OS);
+    return S;
+  }
+};
+} // namespace
+
+TEST(VisitorTrace, TestOutput) {
+  ASTAndTracer Item("void Foo(){}");
+
+  StringRef EmptyState = R"(Running AST visitor for 'Test'
+No children visited
+)";
+
+  StringRef State1 = R"(Running AST visitor for 'Test'
+Visiting:
+  #1: FunctionDecl Foo - <input.cc:1:1, col:12>
+)";
+
+  StringRef State2 = R"(Running AST visitor for 'Test'
+Visiting:
+  #1: BuiltinType - void
+  #2: FunctionDecl Foo - <input.cc:1:1, col:12>
+)";
+
+  StringRef State3 = R"(Running AST visitor for 'Test'
+Visiting:
+  #1: CompoundStmt - <input.cc:1:11, col:12>
+  #2: BuiltinType - void
+  #3: FunctionDecl Foo - <input.cc:1:1, col:12>
+)";
+
+  EXPECT_EQ(Item.dumpVisitor(), EmptyState);
+  {
+    auto State = Item.Tracer.enter(Item.getNode(functionDecl(hasName("Foo"))));
+    EXPECT_EQ(Item.dumpVisitor(), State1);
+    {
+      auto State = Item.Tracer.enter(
+          Item.getNode<Type>(Bind, functionDecl(returns(type().bind(Bind)))));
+      EXPECT_EQ(Item.dumpVisitor(), State2);
+      {
+        auto State = Item.Tracer.enter(Item.getNode(compoundStmt()));
+        EXPECT_EQ(Item.dumpVisitor(), State3);
+      }
+      EXPECT_EQ(Item.dumpVisitor(), State2);
+    }
+    EXPECT_EQ(Item.dumpVisitor(), State1);
+  }
+  EXPECT_EQ(Item.dumpVisitor(), EmptyState);
+}
+
+} // namespace tidy
+} // namespace clang