[cfe-commits] r162936 - in /cfe/trunk: include/clang/ASTMatchers/ASTMatchers.h include/clang/ASTMatchers/ASTMatchersInternal.h include/clang/ASTMatchers/ASTTypeTraits.h lib/ASTMatchers/ASTMatchersInternal.cpp unittests/ASTMatchers/ASTMatchersTest
Sean Silva
silvas at purdue.edu
Thu Aug 30 18:07:08 PDT 2012
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`.
+ // 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.
--Sean Silva
On Thu, Aug 30, 2012 at 3:41 PM, Manuel Klimek <klimek at google.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list