Re: [PATCH] D14011: [AST] Re-add TypeLocs and NestedNameSpecifierLocs to the ParentMap.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL251101: [AST] Re-add TypeLocs and NestedNameSpecifierLocs to
the ParentMap. (authored by d0k).
Changed prior to commit:
http://reviews.llvm.org/D14011?vs=38216=38220#toc
Repository:
rL LLVM
http://reviews.llvm.org/D14011
Files:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/AST/ASTTypeTraits.h
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
cfe/trunk/unittests/AST/ASTContextParentMapTest.cpp
cfe/trunk/unittests/ASTMatchers/Dynamic/ParserTest.cpp
cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -793,8 +793,15 @@
}
void ASTContext::ReleaseParentMapEntries() {
- if (!AllParents) return;
- for (const auto : *AllParents) {
+ if (!PointerParents) return;
+ for (const auto : *PointerParents) {
+if (Entry.second.is()) {
+ delete Entry.second.get();
+} else if (Entry.second.is()) {
+ delete Entry.second.get();
+}
+ }
+ for (const auto : *OtherParents) {
if (Entry.second.is()) {
delete Entry.second.get();
} else if (Entry.second.is()) {
@@ -8670,15 +8677,32 @@
namespace {
-ast_type_traits::DynTypedNode
-getSingleDynTypedNodeFromParentMap(ASTContext::ParentMap::mapped_type U) {
+ast_type_traits::DynTypedNode getSingleDynTypedNodeFromParentMap(
+ASTContext::ParentMapPointers::mapped_type U) {
if (const auto *D = U.dyn_cast())
return ast_type_traits::DynTypedNode::create(*D);
if (const auto *S = U.dyn_cast())
return ast_type_traits::DynTypedNode::create(*S);
return *U.get();
}
+/// Template specializations to abstract away from pointers and TypeLocs.
+/// @{
+template
+ast_type_traits::DynTypedNode createDynTypedNode(const T ) {
+ return ast_type_traits::DynTypedNode::create(*Node);
+}
+template <>
+ast_type_traits::DynTypedNode createDynTypedNode(const TypeLoc ) {
+ return ast_type_traits::DynTypedNode::create(Node);
+}
+template <>
+ast_type_traits::DynTypedNode
+createDynTypedNode(const NestedNameSpecifierLoc ) {
+ return ast_type_traits::DynTypedNode::create(Node);
+}
+/// @}
+
/// \brief A \c RecursiveASTVisitor that builds a map from nodes to their
/// parents as defined by the \c RecursiveASTVisitor.
///
@@ -8693,17 +8717,21 @@
/// \brief Builds and returns the translation unit's parent map.
///
/// The caller takes ownership of the returned \c ParentMap.
-static ASTContext::ParentMap *buildMap(TranslationUnitDecl ) {
- ParentMapASTVisitor Visitor(new ASTContext::ParentMap);
+static std::pair
+buildMap(TranslationUnitDecl ) {
+ ParentMapASTVisitor Visitor(new ASTContext::ParentMapPointers,
+ new ASTContext::ParentMapOtherNodes);
Visitor.TraverseDecl();
- return Visitor.Parents;
+ return std::make_pair(Visitor.Parents, Visitor.OtherParents);
}
private:
typedef RecursiveASTVisitor VisitorBase;
-ParentMapASTVisitor(ASTContext::ParentMap *Parents) : Parents(Parents) {
-}
+ParentMapASTVisitor(ASTContext::ParentMapPointers *Parents,
+ASTContext::ParentMapOtherNodes *OtherParents)
+: Parents(Parents), OtherParents(OtherParents) {}
bool shouldVisitTemplateInstantiations() const {
return true;
@@ -8717,8 +8745,9 @@
return false;
}
-template
-bool TraverseNode(T *Node, bool(VisitorBase:: *traverse) (T *)) {
+template
+bool TraverseNode(T Node, MapNodeTy MapNode,
+ bool (VisitorBase::*traverse)(T), MapTy *Parents) {
if (!Node)
return true;
if (ParentStack.size() > 0) {
@@ -8732,7 +8761,7 @@
// map. The main problem there is to implement hash functions /
// comparison operators for all types that DynTypedNode supports that
// do not have pointer identity.
-auto = (*Parents)[Node];
+auto = (*Parents)[MapNode];
if (NodeOrVector.isNull()) {
if (const auto *D = ParentStack.back().get())
NodeOrVector = D;
@@ -8765,49 +8794,70 @@
Vector->push_back(ParentStack.back());
}
}
- ParentStack.push_back(ast_type_traits::DynTypedNode::create(*Node));
+ ParentStack.push_back(createDynTypedNode(Node));
bool Result = (this ->* traverse) (Node);
ParentStack.pop_back();
return Result;
}
bool TraverseDecl(Decl *DeclNode) {
- return TraverseNode(DeclNode, ::TraverseDecl);
+ return TraverseNode(DeclNode, DeclNode, ::TraverseDecl,
+ Parents);
}
bool
[PATCH] D14011: [AST] Re-add TypeLocs and NestedNameSpecifierLocs to the ParentMap.
bkramer created this revision.
bkramer added reviewers: klimek, djasper.
bkramer added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
This relands r250831 after some fixes to shrink the ParentMap overall
with one addtional tweak: nodes with pointer identity (e.g. Decl* and
friends) can be store more efficiently so I put them in a separate map.
All other nodes (so far only TypeLoc and NNSLoc) go in a different map
keyed on DynTypedNode. This further uglifies the code but significantly
reduces memory overhead.
Overall this change still make ParentMap significantly larger but it's
nowhere as bad as before. I see about 25 MB over baseline (pre-r251008)
on X86ISelLowering.cpp. If this becomes an issue we could consider
splitting the maps further as DynTypedNode is still larger (32 bytes)
than a single TypeLoc (16 bytes) but I didn't want to introduce even
more complexity now.
http://reviews.llvm.org/D14011
Files:
include/clang/AST/ASTContext.h
include/clang/AST/ASTTypeTraits.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/ASTMatchers/ASTMatchersInternal.h
lib/AST/ASTContext.cpp
lib/ASTMatchers/ASTMatchFinder.cpp
unittests/AST/ASTContextParentMapTest.cpp
unittests/ASTMatchers/Dynamic/ParserTest.cpp
unittests/ASTMatchers/Dynamic/RegistryTest.cpp
Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -448,7 +448,9 @@
CompVector Comps = getCompletions();
// Overloaded
EXPECT_TRUE(hasCompletion(
- Comps, "hasParent(", "Matcher hasParent(Matcher)"));
+ Comps, "hasParent(",
+ "Matcher "
+ "hasParent(Matcher)"));
// Variadic.
EXPECT_TRUE(hasCompletion(Comps, "whileStmt(",
"Matcher whileStmt(Matcher...)"));
@@ -464,7 +466,9 @@
EXPECT_TRUE(hasCompletion(WhileComps, "hasBody(",
"Matcher hasBody(Matcher)"));
EXPECT_TRUE(hasCompletion(WhileComps, "hasParent(",
-"Matcher hasParent(Matcher)"));
+"Matcher "
+"hasParent(Matcher)"));
EXPECT_TRUE(
hasCompletion(WhileComps, "allOf(", "Matcher allOf(Matcher...)"));
Index: unittests/ASTMatchers/Dynamic/ParserTest.cpp
===
--- unittests/ASTMatchers/Dynamic/ParserTest.cpp
+++ unittests/ASTMatchers/Dynamic/ParserTest.cpp
@@ -318,8 +318,10 @@
Comps[1].MatcherDecl);
EXPECT_EQ("arent(", Comps[2].TypedText);
- EXPECT_EQ("Matcher hasParent(Matcher)",
-Comps[2].MatcherDecl);
+ EXPECT_EQ(
+ "Matcher "
+ "hasParent(Matcher)",
+ Comps[2].MatcherDecl);
}
} // end anonymous namespace
Index: unittests/AST/ASTContextParentMapTest.cpp
===
--- unittests/AST/ASTContextParentMapTest.cpp
+++ unittests/AST/ASTContextParentMapTest.cpp
@@ -38,6 +38,19 @@
ifStmt(hasParent(compoundStmt();
}
+TEST(GetParents, ReturnsParentForTypeLoc) {
+ MatchVerifier Verifier;
+ EXPECT_TRUE(
+ Verifier.match("namespace a { class b {}; } void f(a::b) {}",
+ typeLoc(hasParent(typeLoc(hasParent(functionDecl()));
+}
+
+TEST(GetParents, ReturnsParentForNestedNameSpecifierLoc) {
+ MatchVerifier Verifier;
+ EXPECT_TRUE(Verifier.match("namespace a { class b {}; } void f(a::b) {}",
+ nestedNameSpecifierLoc(hasParent(typeLoc();
+}
+
TEST(GetParents, ReturnsParentInsideTemplateInstantiations) {
MatchVerifier DeclVerifier;
EXPECT_TRUE(DeclVerifier.match(
Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -621,9 +621,6 @@
if (Node.get() ==
ActiveASTContext->getTranslationUnitDecl())
return false;
-assert(Node.getMemoizationData() &&
- "Invariant broken: only nodes that support memoization may be "
- "used in the parent map.");
MatchKey Key;
Key.MatcherID = Matcher.getID();
@@ -867,7 +864,11 @@
bool MatchASTVisitor::TraverseNestedNameSpecifierLoc(
NestedNameSpecifierLoc NNS) {
+ if (!NNS)
+return true;
+
match(NNS);
+
// We only match the nested name specifier here (as opposed to traversing it)
// because the traversal is already done in the parallel "Loc"-hierarchy.
if (NNS.hasQualifier())
Index: lib/AST/ASTContext.cpp
Re: [PATCH] D14011: [AST] Re-add TypeLocs and NestedNameSpecifierLocs to the ParentMap.
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg; let's see whether it sticks this time :) Comment at: include/clang/AST/ASTContext.h:456 @@ -455,1 +455,3 @@ + /// pointer identity only, which are more common and we can save space by + /// only storing an unique pointer to them. typedef llvm::DenseMaphttp://reviews.llvm.org/D14011 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
