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.

llvm-svn: 250889
This commit is contained in:
Benjamin Kramer 2015-10-21 10:07:26 +00:00
parent 7dacc242d9
commit e8c51fdbd6
9 changed files with 36 additions and 147 deletions

View File

@ -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;

View File

@ -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

View File

@ -2068,10 +2068,8 @@ internal::Matcher<T> findAll(const internal::Matcher<T> &Matcher) {
///
/// 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::ArgumentAdaptingMatcherFunc<
///
/// 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.
///

View File

@ -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);
}

View File

@ -8678,23 +8678,6 @@ bool ASTContext::AtomicUsesUnsupportedLibcall(const AtomicExpr *E) const {
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 NestedNameSpecifierLoc &Node) {
/// 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 NestedNameSpecifierLoc &Node) {
}
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 NestedNameSpecifierLoc &Node) {
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 NestedNameSpecifierLoc &Node) {
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 NestedNameSpecifierLoc &Node) {
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;
}

View File

@ -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::TraverseNestedNameSpecifier(NestedNameSpecifier *NNS) {
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())

View File

@ -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(

View File

@ -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);
}

View File

@ -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>...)"));