Skip to content

Commit cff34cc

Browse files
njames93njames93
committed
Revert "[ASTMatchers] Output currently processing match and nodes on crash"
This reverts commit d89f9e9.
1 parent 45d9aab commit cff34cc

File tree

3 files changed

+6
-168
lines changed

3 files changed

+6
-168
lines changed

‎clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,6 @@ The improvements are...
9696
Improvements to clang-tidy
9797
--------------------------
9898

99-
- Added trace code to help narrow down any checks and the relevant source code
100-
that result in crashes.
101-
10299
New checks
103100
^^^^^^^^^^
104101

‎clang/lib/ASTMatchers/ASTMatchFinder.cpp

Lines changed: 6 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
#include "clang/AST/RecursiveASTVisitor.h"
2222
#include "llvm/ADT/DenseMap.h"
2323
#include "llvm/ADT/StringMap.h"
24-
#include "llvm/Support/PrettyStackTrace.h"
2524
#include "llvm/Support/Timer.h"
2625
#include <deque>
2726
#include <memory>
@@ -761,67 +760,11 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
761760
D);
762761
}
763762

764-
class TraceReporter : llvm::PrettyStackTraceEntry {
765-
public:
766-
TraceReporter(const MatchASTVisitor &MV) : MV(MV) {}
767-
void print(raw_ostream &OS) const override {
768-
if (!MV.CurMatched) {
769-
OS << "ASTMatcher: Not currently matching\n";
770-
return;
771-
}
772-
assert(MV.ActiveASTContext &&
773-
"ActiveASTContext should be set if there is a matched callback");
774-
775-
OS << "ASTMatcher: Processing '" << MV.CurMatched->getID() << "'\n";
776-
const BoundNodes::IDToNodeMap &Map = MV.CurBoundNodes->getMap();
777-
if (Map.empty()) {
778-
OS << "No bound nodes\n";
779-
return;
780-
}
781-
OS << "--- Bound Nodes Begin ---\n";
782-
for (const auto &Item : Map) {
783-
OS << " " << Item.first << " - { ";
784-
if (const auto *D = Item.second.get<Decl>()) {
785-
OS << D->getDeclKindName() << "Decl ";
786-
if (const auto *ND = dyn_cast<NamedDecl>(D)) {
787-
ND->printQualifiedName(OS);
788-
OS << " : ";
789-
} else
790-
OS << ": ";
791-
D->getSourceRange().print(OS,
792-
MV.ActiveASTContext->getSourceManager());
793-
} else if (const auto *S = Item.second.get<Stmt>()) {
794-
OS << S->getStmtClassName() << " : ";
795-
S->getSourceRange().print(OS,
796-
MV.ActiveASTContext->getSourceManager());
797-
} else if (const auto *T = Item.second.get<Type>()) {
798-
OS << T->getTypeClassName() << "Type : ";
799-
QualType(T, 0).print(OS, MV.ActiveASTContext->getPrintingPolicy());
800-
} else if (const auto *QT = Item.second.get<QualType>()) {
801-
OS << "QualType : ";
802-
QT->print(OS, MV.ActiveASTContext->getPrintingPolicy());
803-
} else {
804-
OS << Item.second.getNodeKind().asStringRef() << " : ";
805-
Item.second.getSourceRange().print(
806-
OS, MV.ActiveASTContext->getSourceManager());
807-
}
808-
OS << " }\n";
809-
}
810-
OS << "--- Bound Nodes End ---\n";
811-
}
812-
813-
private:
814-
const MatchASTVisitor &MV;
815-
};
816-
817763
private:
818764
bool TraversingASTNodeNotSpelledInSource = false;
819765
bool TraversingASTNodeNotAsIs = false;
820766
bool TraversingASTChildrenNotSpelledInSource = false;
821767

822-
const MatchCallback *CurMatched = nullptr;
823-
const BoundNodes *CurBoundNodes = nullptr;
824-
825768
struct ASTNodeNotSpelledInSourceScope {
826769
ASTNodeNotSpelledInSourceScope(MatchASTVisitor *V, bool B)
827770
: MV(V), MB(V->TraversingASTNodeNotSpelledInSource) {
@@ -888,7 +831,7 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
888831
Timer.setBucket(&TimeByBucket[MP.second->getID()]);
889832
BoundNodesTreeBuilder Builder;
890833
if (MP.first.matches(Node, this, &Builder)) {
891-
MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
834+
MatchVisitor Visitor(ActiveASTContext, MP.second);
892835
Builder.visitMatches(&Visitor);
893836
}
894837
}
@@ -920,7 +863,7 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
920863
}
921864

922865
if (MP.first.matches(DynNode, this, &Builder)) {
923-
MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
866+
MatchVisitor Visitor(ActiveASTContext, MP.second);
924867
Builder.visitMatches(&Visitor);
925868
}
926869
}
@@ -1106,36 +1049,18 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
11061049
// Implements a BoundNodesTree::Visitor that calls a MatchCallback with
11071050
// the aggregated bound nodes for each match.
11081051
class MatchVisitor : public BoundNodesTreeBuilder::Visitor {
1109-
struct CurBoundScope {
1110-
CurBoundScope(MatchASTVisitor &MV, const BoundNodes &BN) : MV(MV) {
1111-
assert(MV.CurMatched && !MV.CurBoundNodes);
1112-
MV.CurBoundNodes = &BN;
1113-
}
1114-
1115-
~CurBoundScope() { MV.CurBoundNodes = nullptr; }
1116-
1117-
private:
1118-
MatchASTVisitor &MV;
1119-
};
1120-
11211052
public:
1122-
MatchVisitor(MatchASTVisitor &MV, ASTContext *Context,
1123-
MatchFinder::MatchCallback *Callback)
1124-
: MV(MV), Context(Context), Callback(Callback) {
1125-
assert(!MV.CurMatched && !MV.CurBoundNodes);
1126-
MV.CurMatched = Callback;
1127-
}
1128-
1129-
~MatchVisitor() { MV.CurMatched = nullptr; }
1053+
MatchVisitor(ASTContext* Context,
1054+
MatchFinder::MatchCallback* Callback)
1055+
: Context(Context),
1056+
Callback(Callback) {}
11301057

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

11371063
private:
1138-
MatchASTVisitor &MV;
11391064
ASTContext* Context;
11401065
MatchFinder::MatchCallback* Callback;
11411066
};
@@ -1545,7 +1470,6 @@ void MatchFinder::match(const clang::DynTypedNode &Node, ASTContext &Context) {
15451470

15461471
void MatchFinder::matchAST(ASTContext &Context) {
15471472
internal::MatchASTVisitor Visitor(&Matchers, Options);
1548-
internal::MatchASTVisitor::TraceReporter StackTrace(Visitor);
15491473
Visitor.set_active_ast_context(&Context);
15501474
Visitor.onStartOfTranslationUnit();
15511475
Visitor.TraverseAST(Context);

‎clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

Lines changed: 0 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,8 @@
1212
#include "clang/ASTMatchers/ASTMatchers.h"
1313
#include "clang/Tooling/Tooling.h"
1414
#include "llvm/ADT/Triple.h"
15-
#include "llvm/Config/config.h"
1615
#include "llvm/Support/Host.h"
1716
#include "llvm/Testing/Support/SupportHelpers.h"
18-
#include "gmock/gmock.h"
1917
#include "gtest/gtest.h"
2018

2119
namespace clang {
@@ -36,87 +34,6 @@ TEST(HasNameDeathTest, DiesOnEmptyPattern) {
3634
EXPECT_TRUE(notMatches("class X {};", HasEmptyName));
3735
}, "");
3836
}
39-
40-
#if ENABLE_BACKTRACES
41-
42-
template <typename MatcherT>
43-
static void crashTestNodeDump(MatcherT Matcher,
44-
ArrayRef<StringRef> MatchedNodes,
45-
StringRef Code) {
46-
llvm::EnablePrettyStackTrace();
47-
MatchFinder Finder;
48-
49-
struct CrashCallback : public MatchFinder::MatchCallback {
50-
void run(const MatchFinder::MatchResult &Result) override {
51-
LLVM_BUILTIN_TRAP;
52-
}
53-
llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
54-
return TK_IgnoreUnlessSpelledInSource;
55-
}
56-
StringRef getID() const override { return "CrashTester"; }
57-
} Callback;
58-
Finder.addMatcher(std::move(Matcher), &Callback);
59-
if (MatchedNodes.empty()) {
60-
ASSERT_DEATH(tooling::runToolOnCode(
61-
newFrontendActionFactory(&Finder)->create(), Code),
62-
testing::HasSubstr(
63-
"ASTMatcher: Processing 'CrashTester'\nNo bound nodes"));
64-
} else {
65-
std::vector<testing::PolymorphicMatcher<
66-
testing::internal::HasSubstrMatcher<std::string>>>
67-
Matchers;
68-
Matchers.reserve(MatchedNodes.size());
69-
for (auto Node : MatchedNodes) {
70-
Matchers.push_back(testing::HasSubstr(Node.str()));
71-
}
72-
auto CrashMatcher = testing::AllOf(
73-
testing::HasSubstr(
74-
"ASTMatcher: Processing 'CrashTester'\n--- Bound Nodes Begin ---"),
75-
testing::HasSubstr("--- Bound Nodes End ---"),
76-
testing::AllOfArray(Matchers));
77-
78-
ASSERT_DEATH(tooling::runToolOnCode(
79-
newFrontendActionFactory(&Finder)->create(), Code),
80-
CrashMatcher);
81-
}
82-
}
83-
TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
84-
crashTestNodeDump(forStmt(), {}, "void foo() { for(;;); }");
85-
crashTestNodeDump(
86-
forStmt(hasLoopInit(declStmt(hasSingleDecl(
87-
varDecl(hasType(qualType().bind("QT")),
88-
hasType(type().bind("T")),
89-
hasInitializer(
90-
integerLiteral().bind("IL")))
91-
.bind("VD")))
92-
.bind("DS")))
93-
.bind("FS"),
94-
{"FS - { ForStmt : <input.cc:3:5, line:4:5> }",
95-
"DS - { DeclStmt : <input.cc:3:10, col:19> }",
96-
"IL - { IntegerLiteral : <input.cc:3:18> }", "QT - { QualType : int }",
97-
"T - { BuiltinType : int }",
98-
"VD - { VarDecl I : <input.cc:3:10, col:18> }"},
99-
R"cpp(
100-
void foo() {
101-
for (int I = 0; I < 5; ++I) {
102-
}
103-
}
104-
)cpp");
105-
crashTestNodeDump(
106-
cxxRecordDecl(hasMethod(cxxMethodDecl(hasName("operator+")).bind("Op+")))
107-
.bind("Unnamed"),
108-
{"Unnamed - { CXXRecordDecl (anonymous) : <input.cc:1:1, col:36> }",
109-
"Op+ - { CXXMethodDecl (anonymous struct)::operator+ : <input.cc:1:10, "
110-
"col:29> }"},
111-
"struct { int operator+(int) const; } Unnamed;");
112-
crashTestNodeDump(
113-
cxxRecordDecl(hasMethod(cxxConstructorDecl(isDefaulted()).bind("Ctor")),
114-
hasMethod(cxxDestructorDecl(isDefaulted()).bind("Dtor"))),
115-
{"Ctor - { CXXConstructorDecl Foo::Foo : <input.cc:1:14, col:28> }",
116-
"Dtor - { CXXDestructorDecl Foo::~Foo : <input.cc:1:31, col:46> }"},
117-
"struct Foo { Foo() = default; ~Foo() = default; };");
118-
}
119-
#endif // ENABLE_BACKTRACES
12037
#endif
12138

12239
TEST(ConstructVariadic, MismatchedTypes_Regression) {

0 commit comments

Comments
 (0)