Skip to content

Commit

Permalink
Revert "[ASTMatchers] Output currently processing match and nodes on …
Browse files Browse the repository at this point in the history 
…crash"

This reverts commit d89f9e9.
  • Loading branch information
@njames93
njames93 committed Mar 21, 2022
1 parent 45d9aab commit cff34cc
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 168 deletions.
3 changes: 0 additions & 3 deletions clang-tools-extra/docs/ReleaseNotes.rst
View file
Expand Up @@ -96,9 +96,6 @@ The improvements are...
Improvements to clang-tidy
--------------------------

- Added trace code to help narrow down any checks and the relevant source code
that result in crashes.

New checks
^^^^^^^^^^

Expand Down
88 changes: 6 additions & 82 deletions clang/lib/ASTMatchers/ASTMatchFinder.cpp
View file
Expand Up @@ -21,7 +21,6 @@
#include "clang/AST/RecursiveASTVisitor.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/PrettyStackTrace.h"
#include "llvm/Support/Timer.h"
#include <deque>
#include <memory>
Expand Down Expand Up @@ -761,67 +760,11 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
D);
}

class TraceReporter : llvm::PrettyStackTraceEntry {
public:
TraceReporter(const MatchASTVisitor &MV) : MV(MV) {}
void print(raw_ostream &OS) const override {
if (!MV.CurMatched) {
OS << "ASTMatcher: Not currently matching\n";
return;
}
assert(MV.ActiveASTContext &&
"ActiveASTContext should be set if there is a matched callback");

OS << "ASTMatcher: Processing '" << MV.CurMatched->getID() << "'\n";
const BoundNodes::IDToNodeMap &Map = MV.CurBoundNodes->getMap();
if (Map.empty()) {
OS << "No bound nodes\n";
return;
}
OS << "--- Bound Nodes Begin ---\n";
for (const auto &Item : Map) {
OS << " " << Item.first << " - { ";
if (const auto *D = Item.second.get<Decl>()) {
OS << D->getDeclKindName() << "Decl ";
if (const auto *ND = dyn_cast<NamedDecl>(D)) {
ND->printQualifiedName(OS);
OS << " : ";
} else
OS << ": ";
D->getSourceRange().print(OS,
MV.ActiveASTContext->getSourceManager());
} else if (const auto *S = Item.second.get<Stmt>()) {
OS << S->getStmtClassName() << " : ";
S->getSourceRange().print(OS,
MV.ActiveASTContext->getSourceManager());
} else if (const auto *T = Item.second.get<Type>()) {
OS << T->getTypeClassName() << "Type : ";
QualType(T, 0).print(OS, MV.ActiveASTContext->getPrintingPolicy());
} else if (const auto *QT = Item.second.get<QualType>()) {
OS << "QualType : ";
QT->print(OS, MV.ActiveASTContext->getPrintingPolicy());
} else {
OS << Item.second.getNodeKind().asStringRef() << " : ";
Item.second.getSourceRange().print(
OS, MV.ActiveASTContext->getSourceManager());
}
OS << " }\n";
}
OS << "--- Bound Nodes End ---\n";
}

private:
const MatchASTVisitor &MV;
};

private:
bool TraversingASTNodeNotSpelledInSource = false;
bool TraversingASTNodeNotAsIs = false;
bool TraversingASTChildrenNotSpelledInSource = false;

const MatchCallback *CurMatched = nullptr;
const BoundNodes *CurBoundNodes = nullptr;

struct ASTNodeNotSpelledInSourceScope {
ASTNodeNotSpelledInSourceScope(MatchASTVisitor *V, bool B)
: MV(V), MB(V->TraversingASTNodeNotSpelledInSource) {
Expand Down Expand Up @@ -888,7 +831,7 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
Timer.setBucket(&TimeByBucket[MP.second->getID()]);
BoundNodesTreeBuilder Builder;
if (MP.first.matches(Node, this, &Builder)) {
MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
MatchVisitor Visitor(ActiveASTContext, MP.second);
Builder.visitMatches(&Visitor);
}
}
Expand Down Expand Up @@ -920,7 +863,7 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
}

if (MP.first.matches(DynNode, this, &Builder)) {
MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
MatchVisitor Visitor(ActiveASTContext, MP.second);
Builder.visitMatches(&Visitor);
}
}
Expand Down Expand Up @@ -1106,36 +1049,18 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
// Implements a BoundNodesTree::Visitor that calls a MatchCallback with
// the aggregated bound nodes for each match.
class MatchVisitor : public BoundNodesTreeBuilder::Visitor {
struct CurBoundScope {
CurBoundScope(MatchASTVisitor &MV, const BoundNodes &BN) : MV(MV) {
assert(MV.CurMatched && !MV.CurBoundNodes);
MV.CurBoundNodes = &BN;
}

~CurBoundScope() { MV.CurBoundNodes = nullptr; }

private:
MatchASTVisitor &MV;
};

public:
MatchVisitor(MatchASTVisitor &MV, ASTContext *Context,
MatchFinder::MatchCallback *Callback)
: MV(MV), Context(Context), Callback(Callback) {
assert(!MV.CurMatched && !MV.CurBoundNodes);
MV.CurMatched = Callback;
}

~MatchVisitor() { MV.CurMatched = nullptr; }
MatchVisitor(ASTContext* Context,
MatchFinder::MatchCallback* Callback)
: Context(Context),
Callback(Callback) {}

void visitMatch(const BoundNodes& BoundNodesView) override {
TraversalKindScope RAII(*Context, Callback->getCheckTraversalKind());
CurBoundScope RAII2(MV, BoundNodesView);
Callback->run(MatchFinder::MatchResult(BoundNodesView, Context));
}

private:
MatchASTVisitor &MV;
ASTContext* Context;
MatchFinder::MatchCallback* Callback;
};
Expand Down Expand Up @@ -1545,7 +1470,6 @@ void MatchFinder::match(const clang::DynTypedNode &Node, ASTContext &Context) {

void MatchFinder::matchAST(ASTContext &Context) {
internal::MatchASTVisitor Visitor(&Matchers, Options);
internal::MatchASTVisitor::TraceReporter StackTrace(Visitor);
Visitor.set_active_ast_context(&Context);
Visitor.onStartOfTranslationUnit();
Visitor.TraverseAST(Context);
Expand Down
83 changes: 0 additions & 83 deletions clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
View file
Expand Up @@ -12,10 +12,8 @@
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Config/config.h"
#include "llvm/Support/Host.h"
#include "llvm/Testing/Support/SupportHelpers.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

namespace clang {
Expand All @@ -36,87 +34,6 @@ TEST(HasNameDeathTest, DiesOnEmptyPattern) {
EXPECT_TRUE(notMatches("class X {};", HasEmptyName));
}, "");
}

#if ENABLE_BACKTRACES

template <typename MatcherT>
static void crashTestNodeDump(MatcherT Matcher,
ArrayRef<StringRef> MatchedNodes,
StringRef Code) {
llvm::EnablePrettyStackTrace();
MatchFinder Finder;

struct CrashCallback : public MatchFinder::MatchCallback {
void run(const MatchFinder::MatchResult &Result) override {
LLVM_BUILTIN_TRAP;
}
llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
return TK_IgnoreUnlessSpelledInSource;
}
StringRef getID() const override { return "CrashTester"; }
} Callback;
Finder.addMatcher(std::move(Matcher), &Callback);
if (MatchedNodes.empty()) {
ASSERT_DEATH(tooling::runToolOnCode(
newFrontendActionFactory(&Finder)->create(), Code),
testing::HasSubstr(
"ASTMatcher: Processing 'CrashTester'\nNo bound nodes"));
} else {
std::vector<testing::PolymorphicMatcher<
testing::internal::HasSubstrMatcher<std::string>>>
Matchers;
Matchers.reserve(MatchedNodes.size());
for (auto Node : MatchedNodes) {
Matchers.push_back(testing::HasSubstr(Node.str()));
}
auto CrashMatcher = testing::AllOf(
testing::HasSubstr(
"ASTMatcher: Processing 'CrashTester'\n--- Bound Nodes Begin ---"),
testing::HasSubstr("--- Bound Nodes End ---"),
testing::AllOfArray(Matchers));

ASSERT_DEATH(tooling::runToolOnCode(
newFrontendActionFactory(&Finder)->create(), Code),
CrashMatcher);
}
}
TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
crashTestNodeDump(forStmt(), {}, "void foo() { for(;;); }");
crashTestNodeDump(
forStmt(hasLoopInit(declStmt(hasSingleDecl(
varDecl(hasType(qualType().bind("QT")),
hasType(type().bind("T")),
hasInitializer(
integerLiteral().bind("IL")))
.bind("VD")))
.bind("DS")))
.bind("FS"),
{"FS - { ForStmt : <input.cc:3:5, line:4:5> }",
"DS - { DeclStmt : <input.cc:3:10, col:19> }",
"IL - { IntegerLiteral : <input.cc:3:18> }", "QT - { QualType : int }",
"T - { BuiltinType : int }",
"VD - { VarDecl I : <input.cc:3:10, col:18> }"},
R"cpp(
void foo() {
for (int I = 0; I < 5; ++I) {
}
}
)cpp");
crashTestNodeDump(
cxxRecordDecl(hasMethod(cxxMethodDecl(hasName("operator+")).bind("Op+")))
.bind("Unnamed"),
{"Unnamed - { CXXRecordDecl (anonymous) : <input.cc:1:1, col:36> }",
"Op+ - { CXXMethodDecl (anonymous struct)::operator+ : <input.cc:1:10, "
"col:29> }"},
"struct { int operator+(int) const; } Unnamed;");
crashTestNodeDump(
cxxRecordDecl(hasMethod(cxxConstructorDecl(isDefaulted()).bind("Ctor")),
hasMethod(cxxDestructorDecl(isDefaulted()).bind("Dtor"))),
{"Ctor - { CXXConstructorDecl Foo::Foo : <input.cc:1:14, col:28> }",
"Dtor - { CXXDestructorDecl Foo::~Foo : <input.cc:1:31, col:46> }"},
"struct Foo { Foo() = default; ~Foo() = default; };");
}
#endif // ENABLE_BACKTRACES
#endif

TEST(ConstructVariadic, MismatchedTypes_Regression) {
Expand Down

0 comments on commit cff34cc

Please sign in to comment.