From: Manuel Klimek Date: Thu, 30 Aug 2012 19:41:06 +0000 (+0000) Subject: Fixes a bug for binding memoized match results. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=66341c596f93d0c6475d839db94072b8ebd1cf5b;p=clang Fixes a bug for binding memoized match results. Intorduces an abstraction for DynTypedNode which makes is impossible to create in ways that introduced the bug; also hides the implementation details of the template magic away from the user and prepares the code for adding QualType and TypeLoc bindings, as well as using DynTypedNode instead of overloads for child and ancestor matching. getNodeAs was changed towards a non-pointer type, as we'll want QualType and TypeLoc nodes to be returned by value (the alternative would be to create new storage which is prohibitively costly if we want to use it for child / ancestor matching). DynTypedNode is moved into a new header ASTTypeTraits.h, as it is completely independent of the rest of the matcher infrastructure - if the need comes up, we can move it to a more common place. The interface for users before the introduction of the common storage change remains the same, minus the introduced bug, for which a regression test was added. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@162936 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/ASTMatchers/ASTMatchers.h b/include/clang/ASTMatchers/ASTMatchers.h index 6d52b665d8..7cc829a9c5 100644 --- a/include/clang/ASTMatchers/ASTMatchers.h +++ b/include/clang/ASTMatchers/ASTMatchers.h @@ -68,7 +68,7 @@ public: /// Returns NULL if there was no node bound to \c ID or if there is a node but /// it cannot be converted to the specified type. template - const T *getNodeAs(StringRef ID) const { + const T getNodeAs(StringRef ID) const { return MyBoundNodes.getNodeAs(ID); } @@ -76,11 +76,11 @@ public: /// @{ template const T *getDeclAs(StringRef ID) const { - return getNodeAs(ID); + return getNodeAs(ID); } template const T *getStmtAs(StringRef ID) const { - return getNodeAs(ID); + return getNodeAs(ID); } /// @} diff --git a/include/clang/ASTMatchers/ASTMatchersInternal.h b/include/clang/ASTMatchers/ASTMatchersInternal.h index e0fed02972..7f585ef3e7 100644 --- a/include/clang/ASTMatchers/ASTMatchersInternal.h +++ b/include/clang/ASTMatchers/ASTMatchersInternal.h @@ -40,6 +40,7 @@ #include "clang/AST/ExprCXX.h" #include "clang/AST/Stmt.h" #include "clang/AST/Type.h" +#include "clang/ASTMatchers/ASTTypeTraits.h" #include "llvm/ADT/VariadicFunction.h" #include "llvm/Support/type_traits.h" #include @@ -59,85 +60,6 @@ class BoundNodes; namespace internal { class BoundNodesTreeBuilder; - -/// \brief Indicates the base type of a bound AST node. -/// -/// Used for storing nodes as void*. -/// If you are adding an element to this enum, you must also update -/// get_base_type and the list of NodeBaseTypeUtil specializations. -enum NodeBaseType { - NT_Decl, - NT_Stmt, - NT_QualType, - NT_Unknown -}; - -/// \brief Macro for adding a base type to get_base_type. -#define GET_BASE_TYPE(BaseType, NextBaseType) \ - typename llvm::conditional::value, BaseType, \ - NextBaseType>::type - -/// \brief Meta-template to get base class of a node. -template -struct get_base_type { - typedef GET_BASE_TYPE(Decl, - GET_BASE_TYPE(Stmt, - GET_BASE_TYPE(QualType, - void))) type; -}; - -/// \brief Utility to manipulate nodes of a given base type. -/// -/// We use template specialization on the node base type to enable us to -/// get at the appopriate NodeBaseType objects and do approrpiate static_casts. -template -struct NodeBaseTypeUtil { - /// \brief Returns the NodeBaseType corresponding to \c BaseType. - static NodeBaseType getNodeBaseType() { - return NT_Unknown; - } - - /// \brief Casts \c Node to \c T if \c ActualBaseType matches \c BaseType. - /// Otherwise, NULL is returned. - template - static const T* castNode(const NodeBaseType &ActualBaseType, - const void *Node) { - return NULL; - } -}; - -/// \brief Macro for adding a template specialization of NodeBaseTypeUtil. -#define NODE_BASE_TYPE_UTIL(BaseType) \ -template <> \ -struct NodeBaseTypeUtil { \ - static NodeBaseType getNodeBaseType() { \ - return NT_##BaseType; \ - } \ - \ - template \ - static const T *castNode(const NodeBaseType &ActualBaseType, \ - const void *Node) { \ - if (ActualBaseType == NT_##BaseType) { \ - return llvm::dyn_cast(static_cast(Node)); \ - } else { \ - return NULL; \ - } \ - } \ -} - -/// \brief Template specialization of NodeBaseTypeUtil. See main definition. -/// @{ -NODE_BASE_TYPE_UTIL(Decl); -NODE_BASE_TYPE_UTIL(Stmt); -NODE_BASE_TYPE_UTIL(QualType); -/// @} - -/// \brief Gets the NodeBaseType of a node with type \c T. -template -static NodeBaseType getBaseType(const T*) { - return NodeBaseTypeUtil::type>::getNodeBaseType(); -} - /// \brief Internal version of BoundNodes. Holds all the bound nodes. class BoundNodesMap { public: @@ -146,7 +68,10 @@ public: /// The node's base type should be in NodeBaseType or it will be unaccessible. template void addNode(StringRef ID, const T* Node) { - NodeMap[ID] = std::make_pair(getBaseType(Node), Node); + NodeMap[ID] = ast_type_traits::DynTypedNode::create(Node); + } + void addNode(StringRef ID, ast_type_traits::DynTypedNode Node) { + NodeMap[ID] = Node; } /// \brief Returns the AST node bound to \c ID. @@ -154,14 +79,12 @@ public: /// Returns NULL if there was no node bound to \c ID or if there is a node but /// it cannot be converted to the specified type. template - const T *getNodeAs(StringRef ID) const { + const T getNodeAs(StringRef ID) const { IDToNodeMap::const_iterator It = NodeMap.find(ID); if (It == NodeMap.end()) { return NULL; } - - return NodeBaseTypeUtil::type>:: - template castNode(It->second.first, It->second.second); + return It->second.get(); } /// \brief Copies all ID/Node pairs to BoundNodesTreeBuilder \c Builder. @@ -171,11 +94,8 @@ public: void copyTo(BoundNodesMap *Other) const; private: - /// \brief A node and its base type. - typedef std::pair NodeTypePair; - - /// \brief A map from IDs to node/type pairs. - typedef std::map IDToNodeMap; + /// \brief A map from IDs to the bound nodes. + typedef std::map IDToNodeMap; IDToNodeMap NodeMap; }; @@ -243,6 +163,9 @@ public: void setBinding(const std::string &Id, const T *Node) { Bindings.addNode(Id, Node); } + void setBinding(const std::string &Id, ast_type_traits::DynTypedNode Node) { + Bindings.addNode(Id, Node); + } /// \brief Adds a branch in the tree. void addMatch(const BoundNodesTree& Bindings); diff --git a/include/clang/ASTMatchers/ASTTypeTraits.h b/include/clang/ASTMatchers/ASTTypeTraits.h new file mode 100644 index 0000000000..e90446f534 --- /dev/null +++ b/include/clang/ASTMatchers/ASTTypeTraits.h @@ -0,0 +1,99 @@ +//===--- ASTMatchersTypeTraits.h --------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Provides a dynamically typed node container that can be used to store +// an AST base node at runtime in the same storage in a type safe way. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_AST_MATCHERS_AST_TYPE_TRAITS_H +#define LLVM_CLANG_AST_MATCHERS_AST_TYPE_TRAITS_H + +#include "clang/AST/Decl.h" +#include "clang/AST/Stmt.h" + +namespace clang { +namespace ast_type_traits { + +/// \brief A dynamically typed AST node container. +/// +/// Stores an AST node in a type safe way. +/// Use \c create(Node) to create a \c DynTypedNode from an AST node, +/// and \c get() to retrieve the node as type T if the types match. +class DynTypedNode { +public: + /// \brief Creates a NULL-node, which is needed to be able to use + /// \c DynTypedNodes in STL data structures. + DynTypedNode() : Tag(), Node(NULL) {} + + /// \brief Creates a \c DynTypedNode from \c Node. + template + static DynTypedNode create(T Node) { + return BaseConverter::create(Node); + } + + /// \brief Retrieve the stored node as type \c T. + /// + /// Returns NULL if the stored node does not have a type that is + /// convertible to \c T. + template + T get() const { + return llvm::dyn_cast::type>( + BaseConverter::get(Tag, Node)); + } + +private: + /// \brief Takes care of converting from and to \c T. + template struct BaseConverter; + + /// \brief Supported base node types. + enum NodeTypeTag { + NT_Decl, + NT_Stmt + } Tag; + + /// \brief Stores the data of the node. + // FIXME: We really want to store a union, as we want to support + // storing TypeLoc nodes by-value. + // FIXME: Add QualType storage: we'll want to use QualType::getAsOpaquePtr() + // and getFromOpaquePtr(...) to convert to and from void*, but return the + // QualType objects by value. + void *Node; + + DynTypedNode(NodeTypeTag Tag, const void *Node) + : Tag(Tag), Node(const_cast(Node)) {} +}; +template struct DynTypedNode::BaseConverter::type > >::type > { + static Decl *get(NodeTypeTag Tag, void *Node) { + if (Tag == NT_Decl) return static_cast(Node); + return NULL; + } + static DynTypedNode create(const Decl *Node) { + return DynTypedNode(NT_Decl, Node); + } +}; +template struct DynTypedNode::BaseConverter::type > >::type > { + static Stmt *get(NodeTypeTag Tag, void *Node) { + if (Tag == NT_Stmt) return static_cast(Node); + return NULL; + } + static DynTypedNode create(const Stmt *Node) { + return DynTypedNode(NT_Stmt, Node); + } +}; + +} // end namespace ast_type_traits +} // end namespace clang + +#endif // LLVM_CLANG_AST_MATCHERS_AST_TYPE_TRAITS_H + diff --git a/lib/ASTMatchers/ASTMatchersInternal.cpp b/lib/ASTMatchers/ASTMatchersInternal.cpp index f69b347319..e7ee8ee7ed 100644 --- a/lib/ASTMatchers/ASTMatchersInternal.cpp +++ b/lib/ASTMatchers/ASTMatchersInternal.cpp @@ -22,7 +22,7 @@ void BoundNodesMap::copyTo(BoundNodesTreeBuilder *Builder) const { for (IDToNodeMap::const_iterator It = NodeMap.begin(); It != NodeMap.end(); ++It) { - Builder->setBinding(It->first, It->second.second); + Builder->setBinding(It->first, It->second); } } @@ -31,7 +31,6 @@ void BoundNodesMap::copyTo(BoundNodesMap *Other) const { inserter(Other->NodeMap, Other->NodeMap.begin())); } - BoundNodesTree::BoundNodesTree() {} BoundNodesTree::BoundNodesTree( diff --git a/unittests/ASTMatchers/ASTMatchersTest.cpp b/unittests/ASTMatchers/ASTMatchersTest.cpp index 951bca9a30..595f79cb61 100644 --- a/unittests/ASTMatchers/ASTMatchersTest.cpp +++ b/unittests/ASTMatchers/ASTMatchersTest.cpp @@ -685,6 +685,18 @@ TEST(Matcher, BindTheSameNameInAlternatives) { new VerifyIdIsBoundToStmt("x"))); } +TEST(Matcher, BindsIDForMemoizedResults) { + // Using the same matcher in two match expressions will make memoization + // kick in. + DeclarationMatcher ClassX = recordDecl(hasName("X")).bind("x"); + EXPECT_TRUE(matchAndVerifyResultTrue( + "class A { class B { class X {}; }; };", + DeclarationMatcher(anyOf( + recordDecl(hasName("A"), hasDescendant(ClassX)), + recordDecl(hasName("B"), hasDescendant(ClassX)))), + new VerifyIdIsBoundToDecl("x", 2))); +} + TEST(HasType, TakesQualTypeMatcherAndMatchesExpr) { TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X"))); EXPECT_TRUE(