r250889 - Revert "[AST] Put TypeLocs and NestedNameSpecifierLocs into the ParentMap."
Benjamin Kramer via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 21 03:07:27 PDT 2015
Author: d0k
Date: Wed Oct 21 05:07:26 2015
New Revision: 250889
URL: http://llvm.org/viewvc/llvm-project?rev=250889&view=rev
Log:
Revert "[AST] Put TypeLocs and NestedNameSpecifierLocs into the ParentMap."
Putting DynTypedNode in the ParentMap bloats its memory foot print.
Before the void* key had 8 bytes, now we're at 40 bytes per key which
can mean multiple gigabytes increase for large ASTs and this count
doesn't even include all the added TypeLoc nodes. Revert until I come
up with a better data structure.
This reverts commit r250831.
Modified:
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
Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=250889&r1=250888&r2=250889&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Wed Oct 21 05:07:26 2015
@@ -452,7 +452,7 @@ public:
typedef llvm::SmallVector<ast_type_traits::DynTypedNode, 2> ParentVector;
/// \brief Maps from a node to its parents.
- typedef llvm::DenseMap<ast_type_traits::DynTypedNode,
+ typedef llvm::DenseMap<const void *,
llvm::PointerUnion<ast_type_traits::DynTypedNode *,
ParentVector *>> ParentMap;
Modified: cfe/trunk/include/clang/AST/ASTTypeTraits.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTTypeTraits.h?rev=250889&r1=250888&r2=250889&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTTypeTraits.h (original)
+++ cfe/trunk/include/clang/AST/ASTTypeTraits.h Wed Oct 21 05:07:26 2015
@@ -253,30 +253,10 @@ public:
/// @{
/// \brief Imposes an order on \c DynTypedNode.
///
- /// FIXME: Implement comparsion for other node types.
+ /// Supports comparison of nodes that support memoization.
+ /// FIXME: Implement comparsion for other node types (currently
+ /// only Stmt, Decl, Type and NestedNameSpecifier return memoization data).
bool operator<(const DynTypedNode &Other) const {
- if (!NodeKind.isSame(Other.NodeKind))
- return NodeKind < Other.NodeKind;
-
- if (ASTNodeKind::getFromNodeKind<TypeLoc>().isSame(NodeKind)) {
- auto TLA = getUnchecked<TypeLoc>();
- auto TLB = Other.getUnchecked<TypeLoc>();
- return std::make_pair(TLA.getType().getAsOpaquePtr(),
- TLA.getOpaqueData()) <
- std::make_pair(TLB.getType().getAsOpaquePtr(),
- TLB.getOpaqueData());
- }
-
- if (ASTNodeKind::getFromNodeKind<NestedNameSpecifierLoc>().isSame(
- NodeKind)) {
- auto NNSLA = getUnchecked<NestedNameSpecifierLoc>();
- auto NNSLB = Other.getUnchecked<NestedNameSpecifierLoc>();
- return std::make_pair(NNSLA.getNestedNameSpecifier(),
- NNSLA.getOpaqueData()) <
- std::make_pair(NNSLB.getNestedNameSpecifier(),
- NNSLB.getOpaqueData());
- }
-
assert(getMemoizationData() && Other.getMemoizationData());
return getMemoizationData() < Other.getMemoizationData();
}
@@ -290,13 +270,6 @@ public:
if (ASTNodeKind::getFromNodeKind<QualType>().isSame(NodeKind))
return getUnchecked<QualType>() == Other.getUnchecked<QualType>();
- if (ASTNodeKind::getFromNodeKind<TypeLoc>().isSame(NodeKind))
- return getUnchecked<TypeLoc>() == Other.getUnchecked<TypeLoc>();
-
- if (ASTNodeKind::getFromNodeKind<NestedNameSpecifierLoc>().isSame(NodeKind))
- return getUnchecked<NestedNameSpecifierLoc>() ==
- Other.getUnchecked<NestedNameSpecifierLoc>();
-
assert(getMemoizationData() && Other.getMemoizationData());
return getMemoizationData() == Other.getMemoizationData();
}
@@ -305,47 +278,6 @@ public:
}
/// @}
- /// \brief Hooks for using DynTypedNode as a key in a DenseMap.
- struct DenseMapInfo {
- static inline DynTypedNode getEmptyKey() {
- DynTypedNode Node;
- Node.NodeKind = ASTNodeKind::DenseMapInfo::getEmptyKey();
- return Node;
- }
- static inline DynTypedNode getTombstoneKey() {
- DynTypedNode Node;
- Node.NodeKind = ASTNodeKind::DenseMapInfo::getTombstoneKey();
- return Node;
- }
- static unsigned getHashValue(const DynTypedNode &Val) {
- // FIXME: Add hashing support for the remaining types.
- if (ASTNodeKind::getFromNodeKind<TypeLoc>().isSame(Val.NodeKind)) {
- auto TL = Val.getUnchecked<TypeLoc>();
- return llvm::hash_combine(TL.getType().getAsOpaquePtr(),
- TL.getOpaqueData());
- }
-
- if (ASTNodeKind::getFromNodeKind<NestedNameSpecifierLoc>().isSame(
- Val.NodeKind)) {
- auto NNSL = Val.getUnchecked<NestedNameSpecifierLoc>();
- return llvm::hash_combine(NNSL.getNestedNameSpecifier(),
- NNSL.getOpaqueData());
- }
-
- assert(Val.getMemoizationData());
- return llvm::hash_value(Val.getMemoizationData());
- }
- static bool isEqual(const DynTypedNode &LHS, const DynTypedNode &RHS) {
- auto Empty = ASTNodeKind::DenseMapInfo::getEmptyKey();
- auto TombStone = ASTNodeKind::DenseMapInfo::getTombstoneKey();
- return (ASTNodeKind::DenseMapInfo::isEqual(LHS.NodeKind, Empty) &&
- ASTNodeKind::DenseMapInfo::isEqual(RHS.NodeKind, Empty)) ||
- (ASTNodeKind::DenseMapInfo::isEqual(LHS.NodeKind, TombStone) &&
- ASTNodeKind::DenseMapInfo::isEqual(RHS.NodeKind, TombStone)) ||
- LHS == RHS;
- }
- };
-
private:
/// \brief Takes care of converting from and to \c T.
template <typename T, typename EnablerT = void> struct BaseConverter;
@@ -488,10 +420,6 @@ template <>
struct DenseMapInfo<clang::ast_type_traits::ASTNodeKind>
: clang::ast_type_traits::ASTNodeKind::DenseMapInfo {};
-template <>
-struct DenseMapInfo<clang::ast_type_traits::DynTypedNode>
- : clang::ast_type_traits::DynTypedNode::DenseMapInfo {};
-
} // end namespace llvm
#endif
Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=250889&r1=250888&r2=250889&view=diff
==============================================================================
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Oct 21 05:07:26 2015
@@ -2068,10 +2068,8 @@ internal::Matcher<T> findAll(const inter
///
/// Usable as: Any Matcher
const internal::ArgumentAdaptingMatcherFunc<
- internal::HasParentMatcher,
- internal::TypeList<Decl, NestedNameSpecifierLoc, Stmt, TypeLoc>,
- internal::TypeList<Decl, NestedNameSpecifierLoc, Stmt, TypeLoc>>
- LLVM_ATTRIBUTE_UNUSED hasParent = {};
+ internal::HasParentMatcher, internal::TypeList<Decl, Stmt>,
+ internal::TypeList<Decl, Stmt> > LLVM_ATTRIBUTE_UNUSED hasParent = {};
/// \brief Matches AST nodes that have an ancestor that matches the provided
/// matcher.
@@ -2085,10 +2083,8 @@ const internal::ArgumentAdaptingMatcherF
///
/// Usable as: Any Matcher
const internal::ArgumentAdaptingMatcherFunc<
- internal::HasAncestorMatcher,
- internal::TypeList<Decl, NestedNameSpecifierLoc, Stmt, TypeLoc>,
- internal::TypeList<Decl, NestedNameSpecifierLoc, Stmt, TypeLoc>>
- LLVM_ATTRIBUTE_UNUSED hasAncestor = {};
+ internal::HasAncestorMatcher, internal::TypeList<Decl, Stmt>,
+ internal::TypeList<Decl, Stmt> > LLVM_ATTRIBUTE_UNUSED hasAncestor = {};
/// \brief Matches if the provided matcher does not match.
///
Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h?rev=250889&r1=250888&r2=250889&view=diff
==============================================================================
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h Wed Oct 21 05:07:26 2015
@@ -848,10 +848,8 @@ public:
BoundNodesTreeBuilder *Builder,
AncestorMatchMode MatchMode) {
static_assert(std::is_base_of<Decl, T>::value ||
- std::is_base_of<NestedNameSpecifierLoc, T>::value ||
- std::is_base_of<Stmt, T>::value ||
- std::is_base_of<TypeLoc, T>::value,
- "type not allowed for recursive matching");
+ std::is_base_of<Stmt, T>::value,
+ "only Decl or Stmt allowed for recursive matching");
return matchesAncestorOf(ast_type_traits::DynTypedNode::create(Node),
Matcher, Builder, MatchMode);
}
Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=250889&r1=250888&r2=250889&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Oct 21 05:07:26 2015
@@ -8678,23 +8678,6 @@ bool ASTContext::AtomicUsesUnsupportedLi
namespace {
-/// Template specializations to abstract away from pointers and TypeLocs.
-/// @{
-template <typename T>
-ast_type_traits::DynTypedNode createDynTypedNode(const T &Node) {
- return ast_type_traits::DynTypedNode::create(*Node);
-}
-template <>
-ast_type_traits::DynTypedNode createDynTypedNode(const TypeLoc &Node) {
- return ast_type_traits::DynTypedNode::create(Node);
-}
-template <>
-ast_type_traits::DynTypedNode
-createDynTypedNode(const NestedNameSpecifierLoc &Node) {
- 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.
///
@@ -8702,8 +8685,7 @@ createDynTypedNode(const NestedNameSpeci
/// traversal - there are other relationships (for example declaration context)
/// in the AST that are better modeled by special matchers.
///
-/// FIXME: Currently only builds up the map using \c Stmt, \c Decl,
-/// \c NestedNameSpecifierLoc and \c TypeLoc nodes.
+ /// FIXME: Currently only builds up the map using \c Stmt and \c Decl nodes.
class ParentMapASTVisitor : public RecursiveASTVisitor<ParentMapASTVisitor> {
public:
@@ -8735,11 +8717,21 @@ createDynTypedNode(const NestedNameSpeci
}
template <typename T>
- bool TraverseNode(T Node, bool (VisitorBase::*traverse)(T)) {
+ bool TraverseNode(T *Node, bool(VisitorBase:: *traverse) (T *)) {
if (!Node)
return true;
if (ParentStack.size() > 0) {
- auto &NodeOrVector = (*Parents)[createDynTypedNode(Node)];
+ // FIXME: Currently we add the same parent multiple times, but only
+ // when no memoization data is available for the type.
+ // For example when we visit all subexpressions of template
+ // instantiations; this is suboptimal, but benign: the only way to
+ // visit those is with hasAncestor / hasParent, and those do not create
+ // new matches.
+ // The plan is to enable DynTypedNode to be storable in a map or hash
+ // 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 &NodeOrVector = (*Parents)[Node];
if (NodeOrVector.isNull()) {
NodeOrVector = new ast_type_traits::DynTypedNode(ParentStack.back());
} else {
@@ -8765,7 +8757,7 @@ createDynTypedNode(const NestedNameSpeci
Vector->push_back(ParentStack.back());
}
}
- ParentStack.push_back(createDynTypedNode(Node));
+ ParentStack.push_back(ast_type_traits::DynTypedNode::create(*Node));
bool Result = (this ->* traverse) (Node);
ParentStack.pop_back();
return Result;
@@ -8779,15 +8771,6 @@ createDynTypedNode(const NestedNameSpeci
return TraverseNode(StmtNode, &VisitorBase::TraverseStmt);
}
- bool TraverseTypeLoc(TypeLoc TypeLocNode) {
- return TraverseNode(TypeLocNode, &VisitorBase::TraverseTypeLoc);
- }
-
- bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLocNode) {
- return TraverseNode(NNSLocNode,
- &VisitorBase::TraverseNestedNameSpecifierLoc);
- }
-
ASTContext::ParentMap *Parents;
llvm::SmallVector<ast_type_traits::DynTypedNode, 16> ParentStack;
@@ -8798,13 +8781,16 @@ createDynTypedNode(const NestedNameSpeci
ArrayRef<ast_type_traits::DynTypedNode>
ASTContext::getParents(const ast_type_traits::DynTypedNode &Node) {
+ assert(Node.getMemoizationData() &&
+ "Invariant broken: only nodes that support memoization may be "
+ "used in the parent map.");
if (!AllParents) {
// We always need to run over the whole translation unit, as
// hasAncestor can escape any subtree.
AllParents.reset(
ParentMapASTVisitor::buildMap(*getTranslationUnitDecl()));
}
- ParentMap::const_iterator I = AllParents->find(Node);
+ ParentMap::const_iterator I = AllParents->find(Node.getMemoizationData());
if (I == AllParents->end()) {
return None;
}
Modified: cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp?rev=250889&r1=250888&r2=250889&view=diff
==============================================================================
--- cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp Wed Oct 21 05:07:26 2015
@@ -621,6 +621,9 @@ private:
if (Node.get<TranslationUnitDecl>() ==
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();
@@ -864,11 +867,7 @@ bool MatchASTVisitor::TraverseNestedName
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())
Modified: cfe/trunk/unittests/AST/ASTContextParentMapTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTContextParentMapTest.cpp?rev=250889&r1=250888&r2=250889&view=diff
==============================================================================
--- cfe/trunk/unittests/AST/ASTContextParentMapTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTContextParentMapTest.cpp Wed Oct 21 05:07:26 2015
@@ -38,19 +38,6 @@ TEST(GetParents, ReturnsParentForStmt) {
ifStmt(hasParent(compoundStmt()))));
}
-TEST(GetParents, ReturnsParentForTypeLoc) {
- MatchVerifier<TypeLoc> Verifier;
- EXPECT_TRUE(
- Verifier.match("namespace a { class b {}; } void f(a::b) {}",
- typeLoc(hasParent(typeLoc(hasParent(functionDecl()))))));
-}
-
-TEST(GetParents, ReturnsParentForNestedNameSpecifierLoc) {
- MatchVerifier<NestedNameSpecifierLoc> Verifier;
- EXPECT_TRUE(Verifier.match("namespace a { class b {}; } void f(a::b) {}",
- nestedNameSpecifierLoc(hasParent(typeLoc()))));
-}
-
TEST(GetParents, ReturnsParentInsideTemplateInstantiations) {
MatchVerifier<Decl> DeclVerifier;
EXPECT_TRUE(DeclVerifier.match(
Modified: cfe/trunk/unittests/ASTMatchers/Dynamic/ParserTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/Dynamic/ParserTest.cpp?rev=250889&r1=250888&r2=250889&view=diff
==============================================================================
--- cfe/trunk/unittests/ASTMatchers/Dynamic/ParserTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/Dynamic/ParserTest.cpp Wed Oct 21 05:07:26 2015
@@ -318,8 +318,7 @@ TEST(ParserTest, CompletionNamedValues)
Comps[1].MatcherDecl);
EXPECT_EQ("arent(", Comps[2].TypedText);
- EXPECT_EQ("Matcher<Decl> "
- "hasParent(Matcher<NestedNameSpecifierLoc|TypeLoc|Decl|...>)",
+ EXPECT_EQ("Matcher<Decl> hasParent(Matcher<Decl|Stmt>)",
Comps[2].MatcherDecl);
}
Modified: cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp?rev=250889&r1=250888&r2=250889&view=diff
==============================================================================
--- cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp Wed Oct 21 05:07:26 2015
@@ -447,10 +447,8 @@ TEST_F(RegistryTest, Errors) {
TEST_F(RegistryTest, Completion) {
CompVector Comps = getCompletions();
// Overloaded
- EXPECT_TRUE(hasCompletion(Comps, "hasParent(",
- "Matcher<NestedNameSpecifierLoc|TypeLoc|Decl|...> "
- "hasParent(Matcher<NestedNameSpecifierLoc|TypeLoc|"
- "Decl|...>)"));
+ EXPECT_TRUE(hasCompletion(
+ Comps, "hasParent(", "Matcher<Decl|Stmt> hasParent(Matcher<Decl|Stmt>)"));
// Variadic.
EXPECT_TRUE(hasCompletion(Comps, "whileStmt(",
"Matcher<Stmt> whileStmt(Matcher<WhileStmt>...)"));
@@ -465,10 +463,8 @@ TEST_F(RegistryTest, Completion) {
EXPECT_TRUE(hasCompletion(WhileComps, "hasBody(",
"Matcher<WhileStmt> hasBody(Matcher<Stmt>)"));
- EXPECT_TRUE(hasCompletion(WhileComps, "hasParent(", "Matcher<Stmt> "
- "hasParent(Matcher<"
- "NestedNameSpecifierLoc|"
- "TypeLoc|Decl|...>)"));
+ EXPECT_TRUE(hasCompletion(WhileComps, "hasParent(",
+ "Matcher<Stmt> hasParent(Matcher<Decl|Stmt>)"));
EXPECT_TRUE(
hasCompletion(WhileComps, "allOf(", "Matcher<T> allOf(Matcher<T>...)"));
More information about the cfe-commits
mailing list