njames93 created this revision. njames93 added reviewers: alexfh, aaron.ballman. Herald added subscribers: carlosgalvezp, xazax.hun, mgorny. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.
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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D130801 Files: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp clang-tools-extra/clang-tidy/utils/CMakeLists.txt clang-tools-extra/clang-tidy/utils/VisitorTrace.cpp clang-tools-extra/clang-tidy/utils/VisitorTrace.h clang-tools-extra/unittests/clang-tidy/CMakeLists.txt clang-tools-extra/unittests/clang-tidy/VisitorTraceTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/VisitorTraceTest.cpp =================================================================== --- /dev/null +++ clang-tools-extra/unittests/clang-tidy/VisitorTraceTest.cpp @@ -0,0 +1,103 @@ +//===------- 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 Index: clang-tools-extra/unittests/clang-tidy/CMakeLists.txt =================================================================== --- clang-tools-extra/unittests/clang-tidy/CMakeLists.txt +++ clang-tools-extra/unittests/clang-tidy/CMakeLists.txt @@ -33,6 +33,7 @@ UsingInserterTest.cpp ReadabilityModuleTest.cpp TransformerClangTidyCheckTest.cpp + VisitorTraceTest.cpp ) clang_target_link_libraries(ClangTidyTests Index: clang-tools-extra/clang-tidy/utils/VisitorTrace.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/utils/VisitorTrace.h @@ -0,0 +1,69 @@ +//===---------- 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 Index: clang-tools-extra/clang-tidy/utils/VisitorTrace.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/utils/VisitorTrace.cpp @@ -0,0 +1,56 @@ +//===---------- 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"; +} Index: clang-tools-extra/clang-tidy/utils/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/utils/CMakeLists.txt +++ clang-tools-extra/clang-tidy/utils/CMakeLists.txt @@ -21,6 +21,7 @@ TransformerClangTidyCheck.cpp TypeTraits.cpp UsingInserter.cpp + VisitorTrace.cpp LINK_LIBS clangTidy Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp +++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "SimplifyBooleanExprCheck.h" +#include "../utils/VisitorTrace.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/Lex/Lexer.h" #include "llvm/Support/SaveAndRestore.h" @@ -291,7 +292,8 @@ return true; } - bool VisitBinaryOperator(const BinaryOperator *Op) const { + bool VisitBinaryOperator(const BinaryOperator *Op) { + auto Trace = Tracer.enter(Op); Check->reportBinOp(Context, Op); return true; } @@ -354,6 +356,7 @@ } 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; @@ -433,6 +436,7 @@ } bool VisitConditionalOperator(ConditionalOperator *Cond) { + auto Trace = Tracer.enter(Cond); /* * Condition ? true : false; -> Condition * Condition ? false : true; -> !Condition; @@ -449,6 +453,7 @@ } bool VisitCompoundStmt(CompoundStmt *CS) { + auto Trace = Tracer.enter(CS); if (CS->size() < 2) return true; bool CurIf = false, PrevIf = false; @@ -547,6 +552,7 @@ } 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(); @@ -577,6 +583,7 @@ SimplifyBooleanExprCheck *Check; SmallVector<Stmt *, 32> StmtStack; ASTContext &Context; + VisitorTrace Tracer{*Check, Context}; }; SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
