[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.cpp

Manuel Klimek klimek at google.com
Thu Aug 30 12:41:06 PDT 2012


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(





More information about the cfe-commits mailing list