Skip to content

Commit 61d67c8

Browse files
njames93njames93
committed
Revert "[ASTMatchers] Output currently matching node on crash"
This reverts commit 6e33e45. Fails to build on 32bit machines due to PointerUnion limitations
1 parent e8673f2 commit 61d67c8

File tree

2 files changed

+59
-184
lines changed

2 files changed

+59
-184
lines changed

‎clang/lib/ASTMatchers/ASTMatchFinder.cpp

Lines changed: 55 additions & 158 deletions
Original file line numberDiff line numberDiff line change
@@ -761,173 +761,67 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
761761
D);
762762
}
763763

764-
private:
765-
bool TraversingASTNodeNotSpelledInSource = false;
766-
bool TraversingASTNodeNotAsIs = false;
767-
bool TraversingASTChildrenNotSpelledInSource = false;
768-
769-
class CurMatchData {
770-
public:
771-
CurMatchData() = default;
772-
773-
template <typename NodeType>
774-
void SetCallbackAndRawNode(const MatchCallback *CB, const NodeType &N) {
775-
assertEmpty();
776-
Callback = CB;
777-
MatchingNode = &N;
778-
}
779-
780-
const MatchCallback *getCallback() const { return Callback; }
781-
782-
void SetBoundNodes(const BoundNodes &BN) {
783-
assertHoldsState();
784-
BNodes = &BN;
785-
}
786-
787-
void clearBoundNodes() {
788-
assertHoldsState();
789-
BNodes = nullptr;
790-
}
791-
792-
template <typename T> const T *getNode() const {
793-
assertHoldsState();
794-
return MatchingNode.dyn_cast<const T *>();
795-
}
796-
797-
const BoundNodes *getBoundNodes() const {
798-
assertHoldsState();
799-
return BNodes;
800-
}
801-
802-
void reset() {
803-
assertHoldsState();
804-
Callback = nullptr;
805-
MatchingNode = nullptr;
806-
}
807-
808-
private:
809-
void assertHoldsState() const {
810-
assert(Callback != nullptr && !MatchingNode.isNull());
811-
}
812-
813-
void assertEmpty() const {
814-
assert(Callback == nullptr && MatchingNode.isNull() && BNodes == nullptr);
815-
}
816-
817-
const MatchCallback *Callback = nullptr;
818-
const BoundNodes *BNodes = nullptr;
819-
820-
llvm::PointerUnion<
821-
const QualType *, const TypeLoc *, const NestedNameSpecifier *,
822-
const NestedNameSpecifierLoc *, const CXXCtorInitializer *,
823-
const TemplateArgumentLoc *, const Attr *, const DynTypedNode *>
824-
MatchingNode;
825-
} CurMatchState;
826-
827-
struct CurMatchRAII {
828-
template <typename NodeType>
829-
CurMatchRAII(MatchASTVisitor &MV, const MatchCallback *CB,
830-
const NodeType &NT)
831-
: MV(MV) {
832-
MV.CurMatchState.SetCallbackAndRawNode(CB, NT);
833-
}
834-
835-
~CurMatchRAII() { MV.CurMatchState.reset(); }
836-
837-
private:
838-
MatchASTVisitor &MV;
839-
};
840-
841-
public:
842764
class TraceReporter : llvm::PrettyStackTraceEntry {
843-
static void dumpNode(const ASTContext &Ctx, const DynTypedNode &Node,
844-
raw_ostream &OS) {
845-
if (const auto *D = Node.get<Decl>()) {
846-
OS << D->getDeclKindName() << "Decl ";
847-
if (const auto *ND = dyn_cast<NamedDecl>(D)) {
848-
ND->printQualifiedName(OS);
849-
OS << " : ";
850-
} else
851-
OS << ": ";
852-
D->getSourceRange().print(OS, Ctx.getSourceManager());
853-
} else if (const auto *S = Node.get<Stmt>()) {
854-
OS << S->getStmtClassName() << " : ";
855-
S->getSourceRange().print(OS, Ctx.getSourceManager());
856-
} else if (const auto *T = Node.get<Type>()) {
857-
OS << T->getTypeClassName() << "Type : ";
858-
QualType(T, 0).print(OS, Ctx.getPrintingPolicy());
859-
} else if (const auto *QT = Node.get<QualType>()) {
860-
OS << "QualType : ";
861-
QT->print(OS, Ctx.getPrintingPolicy());
862-
} else {
863-
OS << Node.getNodeKind().asStringRef() << " : ";
864-
Node.getSourceRange().print(OS, Ctx.getSourceManager());
865-
}
866-
}
867-
868-
static void dumpNodeFromState(const ASTContext &Ctx,
869-
const CurMatchData &State, raw_ostream &OS) {
870-
if (const DynTypedNode *MatchNode = State.getNode<DynTypedNode>()) {
871-
dumpNode(Ctx, *MatchNode, OS);
872-
} else if (const auto *QT = State.getNode<QualType>()) {
873-
dumpNode(Ctx, DynTypedNode::create(*QT), OS);
874-
} else if (const auto *TL = State.getNode<TypeLoc>()) {
875-
dumpNode(Ctx, DynTypedNode::create(*TL), OS);
876-
} else if (const auto *NNS = State.getNode<NestedNameSpecifier>()) {
877-
dumpNode(Ctx, DynTypedNode::create(*NNS), OS);
878-
} else if (const auto *NNSL = State.getNode<NestedNameSpecifierLoc>()) {
879-
dumpNode(Ctx, DynTypedNode::create(*NNSL), OS);
880-
} else if (const auto *CtorInit = State.getNode<CXXCtorInitializer>()) {
881-
dumpNode(Ctx, DynTypedNode::create(*CtorInit), OS);
882-
} else if (const auto *TAL = State.getNode<TemplateArgumentLoc>()) {
883-
dumpNode(Ctx, DynTypedNode::create(*TAL), OS);
884-
} else if (const auto *At = State.getNode<Attr>()) {
885-
dumpNode(Ctx, DynTypedNode::create(*At), OS);
886-
}
887-
}
888-
889765
public:
890766
TraceReporter(const MatchASTVisitor &MV) : MV(MV) {}
891767
void print(raw_ostream &OS) const override {
892-
const CurMatchData &State = MV.CurMatchState;
893-
const MatchCallback *CB = State.getCallback();
894-
if (!CB) {
768+
if (!MV.CurMatched) {
895769
OS << "ASTMatcher: Not currently matching\n";
896770
return;
897771
}
898-
899772
assert(MV.ActiveASTContext &&
900773
"ActiveASTContext should be set if there is a matched callback");
901774

902-
ASTContext &Ctx = MV.getASTContext();
903-
904-
if (const BoundNodes *Nodes = State.getBoundNodes()) {
905-
OS << "ASTMatcher: Processing '" << CB->getID() << "' against:\n\t";
906-
dumpNodeFromState(Ctx, State, OS);
907-
const BoundNodes::IDToNodeMap &Map = Nodes->getMap();
908-
if (Map.empty()) {
909-
OS << "\nNo bound nodes\n";
910-
return;
911-
}
912-
OS << "\n--- Bound Nodes Begin ---\n";
913-
for (const auto &Item : Map) {
914-
OS << " " << Item.first << " - { ";
915-
dumpNode(Ctx, Item.second, OS);
916-
OS << " }\n";
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());
917807
}
918-
OS << "--- Bound Nodes End ---\n";
919-
} else {
920-
OS << "ASTMatcher: Matching '" << CB->getID() << "' against:\n\t";
921-
dumpNodeFromState(Ctx, State, OS);
922-
OS << '\n';
808+
OS << " }\n";
923809
}
810+
OS << "--- Bound Nodes End ---\n";
924811
}
925812

926813
private:
927814
const MatchASTVisitor &MV;
928815
};
929816

930817
private:
818+
bool TraversingASTNodeNotSpelledInSource = false;
819+
bool TraversingASTNodeNotAsIs = false;
820+
bool TraversingASTChildrenNotSpelledInSource = false;
821+
822+
const MatchCallback *CurMatched = nullptr;
823+
const BoundNodes *CurBoundNodes = nullptr;
824+
931825
struct ASTNodeNotSpelledInSourceScope {
932826
ASTNodeNotSpelledInSourceScope(MatchASTVisitor *V, bool B)
933827
: MV(V), MB(V->TraversingASTNodeNotSpelledInSource) {
@@ -993,7 +887,6 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
993887
if (EnableCheckProfiling)
994888
Timer.setBucket(&TimeByBucket[MP.second->getID()]);
995889
BoundNodesTreeBuilder Builder;
996-
CurMatchRAII RAII(*this, MP.second, Node);
997890
if (MP.first.matches(Node, this, &Builder)) {
998891
MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
999892
Builder.visitMatches(&Visitor);
@@ -1026,7 +919,6 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
1026919
continue;
1027920
}
1028921

1029-
CurMatchRAII RAII(*this, MP.second, DynNode);
1030922
if (MP.first.matches(DynNode, this, &Builder)) {
1031923
MatchVisitor Visitor(*this, ActiveASTContext, MP.second);
1032924
Builder.visitMatches(&Visitor);
@@ -1215,30 +1107,35 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>,
12151107
// the aggregated bound nodes for each match.
12161108
class MatchVisitor : public BoundNodesTreeBuilder::Visitor {
12171109
struct CurBoundScope {
1218-
CurBoundScope(MatchASTVisitor::CurMatchData &State, const BoundNodes &BN)
1219-
: State(State) {
1220-
State.SetBoundNodes(BN);
1110+
CurBoundScope(MatchASTVisitor &MV, const BoundNodes &BN) : MV(MV) {
1111+
assert(MV.CurMatched && !MV.CurBoundNodes);
1112+
MV.CurBoundNodes = &BN;
12211113
}
12221114

1223-
~CurBoundScope() { State.clearBoundNodes(); }
1115+
~CurBoundScope() { MV.CurBoundNodes = nullptr; }
12241116

12251117
private:
1226-
MatchASTVisitor::CurMatchData &State;
1118+
MatchASTVisitor &MV;
12271119
};
12281120

12291121
public:
12301122
MatchVisitor(MatchASTVisitor &MV, ASTContext *Context,
12311123
MatchFinder::MatchCallback *Callback)
1232-
: State(MV.CurMatchState), Context(Context), Callback(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; }
12331130

12341131
void visitMatch(const BoundNodes& BoundNodesView) override {
12351132
TraversalKindScope RAII(*Context, Callback->getCheckTraversalKind());
1236-
CurBoundScope RAII2(State, BoundNodesView);
1133+
CurBoundScope RAII2(MV, BoundNodesView);
12371134
Callback->run(MatchFinder::MatchResult(BoundNodesView, Context));
12381135
}
12391136

12401137
private:
1241-
MatchASTVisitor::CurMatchData &State;
1138+
MatchASTVisitor &MV;
12421139
ASTContext* Context;
12431140
MatchFinder::MatchCallback* Callback;
12441141
};

‎clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -39,24 +39,10 @@ TEST(HasNameDeathTest, DiesOnEmptyPattern) {
3939
// FIXME: Figure out why back traces aren't being generated on clang builds on
4040
// windows.
4141
#if ENABLE_BACKTRACES && (!defined(_MSC_VER) || !defined(__clang__))
42-
43-
AST_MATCHER(Decl, causeCrash) {
44-
abort();
45-
return true;
46-
}
47-
48-
TEST(MatcherCrashDeathTest, CrashOnMatcherDump) {
49-
llvm::EnablePrettyStackTrace();
50-
auto Matcher = testing::HasSubstr(
51-
"ASTMatcher: Matching '<unknown>' against:\n\tFunctionDecl foo : "
52-
"<input.cc:1:1, col:10>");
53-
ASSERT_DEATH(matches("void foo();", functionDecl(causeCrash())), Matcher);
54-
}
55-
5642
template <typename MatcherT>
5743
static void crashTestNodeDump(MatcherT Matcher,
5844
ArrayRef<StringRef> MatchedNodes,
59-
StringRef Against, StringRef Code) {
45+
StringRef Code) {
6046
llvm::EnablePrettyStackTrace();
6147
MatchFinder Finder;
6248

@@ -72,9 +58,7 @@ static void crashTestNodeDump(MatcherT Matcher,
7258
ASSERT_DEATH(tooling::runToolOnCode(
7359
newFrontendActionFactory(&Finder)->create(), Code),
7460
testing::HasSubstr(
75-
("ASTMatcher: Processing 'CrashTester' against:\n\t" +
76-
Against + "\nNo bound nodes")
77-
.str()));
61+
"ASTMatcher: Processing 'CrashTester'\nNo bound nodes"));
7862
} else {
7963
std::vector<testing::PolymorphicMatcher<
8064
testing::internal::HasSubstrMatcher<std::string>>>
@@ -85,9 +69,7 @@ static void crashTestNodeDump(MatcherT Matcher,
8569
}
8670
auto CrashMatcher = testing::AllOf(
8771
testing::HasSubstr(
88-
("ASTMatcher: Processing 'CrashTester' against:\n\t" + Against +
89-
"\n--- Bound Nodes Begin ---")
90-
.str()),
72+
"ASTMatcher: Processing 'CrashTester'\n--- Bound Nodes Begin ---"),
9173
testing::HasSubstr("--- Bound Nodes End ---"),
9274
testing::AllOfArray(Matchers));
9375

@@ -97,8 +79,7 @@ static void crashTestNodeDump(MatcherT Matcher,
9779
}
9880
}
9981
TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
100-
crashTestNodeDump(forStmt(), {}, "ForStmt : <input.cc:1:14, col:21>",
101-
"void foo() { for(;;); }");
82+
crashTestNodeDump(forStmt(), {}, "void foo() { for(;;); }");
10283
crashTestNodeDump(
10384
forStmt(hasLoopInit(declStmt(hasSingleDecl(
10485
varDecl(hasType(qualType().bind("QT")),
@@ -113,7 +94,6 @@ TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
11394
"IL - { IntegerLiteral : <input.cc:3:18> }", "QT - { QualType : int }",
11495
"T - { BuiltinType : int }",
11596
"VD - { VarDecl I : <input.cc:3:10, col:18> }"},
116-
"ForStmt : <input.cc:3:5, line:4:5>",
11797
R"cpp(
11898
void foo() {
11999
for (int I = 0; I < 5; ++I) {
@@ -126,14 +106,12 @@ TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
126106
{"Unnamed - { CXXRecordDecl (anonymous) : <input.cc:1:1, col:36> }",
127107
"Op+ - { CXXMethodDecl (anonymous struct)::operator+ : <input.cc:1:10, "
128108
"col:29> }"},
129-
"CXXRecordDecl (anonymous) : <input.cc:1:1, col:36>",
130109
"struct { int operator+(int) const; } Unnamed;");
131110
crashTestNodeDump(
132111
cxxRecordDecl(hasMethod(cxxConstructorDecl(isDefaulted()).bind("Ctor")),
133112
hasMethod(cxxDestructorDecl(isDefaulted()).bind("Dtor"))),
134113
{"Ctor - { CXXConstructorDecl Foo::Foo : <input.cc:1:14, col:28> }",
135114
"Dtor - { CXXDestructorDecl Foo::~Foo : <input.cc:1:31, col:46> }"},
136-
"CXXRecordDecl Foo : <input.cc:1:1, col:49>",
137115
"struct Foo { Foo() = default; ~Foo() = default; };");
138116
}
139117
#endif // ENABLE_BACKTRACES

0 commit comments

Comments
 (0)