On Fri, Aug 31, 2012 at 3:07 AM, Sean Silva <[email protected]> wrote: > I'm really liking DynTypedNode :) And seeing all those macros get nuked! > > The traits-based approach is a really good fit for this. > > +/// \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<T>() to retrieve the node as type T if the types match. > +class DynTypedNode { > > I would briefly mention in this comment that the reason this exists is > because Clang's AST types are organized in multiple node hierarchies. > I would also mention that the different supported base types are > listed in the enum `NodeTypeTag`.
Addressed in a subsequent change. > + // 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. > > This by-value vs. by-ref stuff should be able to be parametrized by > extending BaseConverter to be more generally an `ASTHierarchyTraits` > (or whatever). Does that sound reasonable? It seems like a natural > place to put it. Also I think that a relatively recently added > AlignedCharArrayUnion > <http://llvm.org/docs/doxygen/html/unionllvm_1_1AlignedCharArrayUnion.html> > would be handy to handle the storage here. Addressed in a subsequent change. > --Sean Silva > > On Thu, Aug 30, 2012 at 3:41 PM, Manuel Klimek <[email protected]> wrote: >> Author: klimek >> Date: Thu Aug 30 14:41:06 2012 >> New Revision: 162936 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=162936&view=rev >> Log: >> 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<T> 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. >> >> Added: >> cfe/trunk/include/clang/ASTMatchers/ASTTypeTraits.h >> Modified: >> cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h >> cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h >> cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp >> cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp >> >> Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=162936&r1=162935&r2=162936&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original) >> +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Thu Aug 30 14:41:06 >> 2012 >> @@ -68,7 +68,7 @@ >> /// 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 <typename T> >> - const T *getNodeAs(StringRef ID) const { >> + const T getNodeAs(StringRef ID) const { >> return MyBoundNodes.getNodeAs<T>(ID); >> } >> >> @@ -76,11 +76,11 @@ >> /// @{ >> template <typename T> >> const T *getDeclAs(StringRef ID) const { >> - return getNodeAs<T>(ID); >> + return getNodeAs<T*>(ID); >> } >> template <typename T> >> const T *getStmtAs(StringRef ID) const { >> - return getNodeAs<T>(ID); >> + return getNodeAs<T*>(ID); >> } >> /// @} >> >> >> Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h?rev=162936&r1=162935&r2=162936&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h (original) >> +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h Thu Aug 30 >> 14:41:06 2012 >> @@ -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 <map> >> @@ -59,85 +60,6 @@ >> 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<llvm::is_base_of<BaseType, T>::value, >> BaseType, \ >> - NextBaseType>::type >> - >> -/// \brief Meta-template to get base class of a node. >> -template <typename T> >> -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 <typename BaseType> >> -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 <typename T> >> - 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<BaseType> { \ >> - static NodeBaseType getNodeBaseType() { \ >> - return NT_##BaseType; \ >> - } \ >> - \ >> - template <typename T> \ >> - static const T *castNode(const NodeBaseType &ActualBaseType, \ >> - const void *Node) { \ >> - if (ActualBaseType == NT_##BaseType) { \ >> - return llvm::dyn_cast<T>(static_cast<const BaseType*>(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 <typename T> >> -static NodeBaseType getBaseType(const T*) { >> - return NodeBaseTypeUtil<typename >> get_base_type<T>::type>::getNodeBaseType(); >> -} >> - >> /// \brief Internal version of BoundNodes. Holds all the bound nodes. >> class BoundNodesMap { >> public: >> @@ -146,7 +68,10 @@ >> /// The node's base type should be in NodeBaseType or it will be >> unaccessible. >> template <typename T> >> void addNode(StringRef ID, const T* Node) { >> - NodeMap[ID] = std::make_pair(getBaseType(Node), Node); >> + NodeMap[ID] = ast_type_traits::DynTypedNode::create<const T*>(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 @@ >> /// 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 <typename T> >> - 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<typename get_base_type<T>::type>:: >> - template castNode<T>(It->second.first, It->second.second); >> + return It->second.get<T>(); >> } >> >> /// \brief Copies all ID/Node pairs to BoundNodesTreeBuilder \c Builder. >> @@ -171,11 +94,8 @@ >> void copyTo(BoundNodesMap *Other) const; >> >> private: >> - /// \brief A node and its base type. >> - typedef std::pair<NodeBaseType, const void*> NodeTypePair; >> - >> - /// \brief A map from IDs to node/type pairs. >> - typedef std::map<std::string, NodeTypePair> IDToNodeMap; >> + /// \brief A map from IDs to the bound nodes. >> + typedef std::map<std::string, ast_type_traits::DynTypedNode> IDToNodeMap; >> >> IDToNodeMap NodeMap; >> }; >> @@ -243,6 +163,9 @@ >> 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); >> >> Added: cfe/trunk/include/clang/ASTMatchers/ASTTypeTraits.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTTypeTraits.h?rev=162936&view=auto >> ============================================================================== >> --- cfe/trunk/include/clang/ASTMatchers/ASTTypeTraits.h (added) >> +++ cfe/trunk/include/clang/ASTMatchers/ASTTypeTraits.h Thu Aug 30 14:41:06 >> 2012 >> @@ -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<T>() 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 <typename T> >> + static DynTypedNode create(T Node) { >> + return BaseConverter<T>::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 <typename T> >> + T get() const { >> + return llvm::dyn_cast<typename llvm::remove_pointer<T>::type>( >> + BaseConverter<T>::get(Tag, Node)); >> + } >> + >> +private: >> + /// \brief Takes care of converting from and to \c T. >> + template <typename T, typename EnablerT = void> 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<void*>(Node)) {} >> +}; >> +template<typename T> struct DynTypedNode::BaseConverter<T, >> + typename llvm::enable_if<llvm::is_base_of< >> + Decl, typename llvm::remove_pointer<T>::type > >::type > { >> + static Decl *get(NodeTypeTag Tag, void *Node) { >> + if (Tag == NT_Decl) return static_cast<Decl*>(Node); >> + return NULL; >> + } >> + static DynTypedNode create(const Decl *Node) { >> + return DynTypedNode(NT_Decl, Node); >> + } >> +}; >> +template<typename T> struct DynTypedNode::BaseConverter<T, >> + typename llvm::enable_if<llvm::is_base_of< >> + Stmt, typename llvm::remove_pointer<T>::type > >::type > { >> + static Stmt *get(NodeTypeTag Tag, void *Node) { >> + if (Tag == NT_Stmt) return static_cast<Stmt*>(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 >> + >> >> Modified: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp?rev=162936&r1=162935&r2=162936&view=diff >> ============================================================================== >> --- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp (original) >> +++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp Thu Aug 30 14:41:06 >> 2012 >> @@ -22,7 +22,7 @@ >> 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 @@ >> inserter(Other->NodeMap, Other->NodeMap.begin())); >> } >> >> - >> BoundNodesTree::BoundNodesTree() {} >> >> BoundNodesTree::BoundNodesTree( >> >> Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=162936&r1=162935&r2=162936&view=diff >> ============================================================================== >> --- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original) >> +++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Thu Aug 30 14:41:06 >> 2012 >> @@ -685,6 +685,18 @@ >> new VerifyIdIsBoundToStmt<CallExpr>("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<Decl>("x", 2))); >> +} >> + >> TEST(HasType, TakesQualTypeMatcherAndMatchesExpr) { >> TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X"))); >> EXPECT_TRUE( >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
