]> granicus.if.org Git - clang/commitdiff
Fixes a bug for binding memoized match results.
authorManuel Klimek <klimek@google.com>
Thu, 30 Aug 2012 19:41:06 +0000 (19:41 +0000)
committerManuel Klimek <klimek@google.com>
Thu, 30 Aug 2012 19:41:06 +0000 (19:41 +0000)
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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@162936 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/ASTMatchers/ASTMatchers.h
include/clang/ASTMatchers/ASTMatchersInternal.h
include/clang/ASTMatchers/ASTTypeTraits.h [new file with mode: 0644]
lib/ASTMatchers/ASTMatchersInternal.cpp
unittests/ASTMatchers/ASTMatchersTest.cpp

index 6d52b665d81a8331fde4d3f68670ea89597ba16b..7cc829a9c575d2620522067be050cb76b74000c2 100644 (file)
@@ -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 <typename T>
-  const T *getNodeAs(StringRef ID) const {
+  const T getNodeAs(StringRef ID) const {
     return MyBoundNodes.getNodeAs<T>(ID);
   }
 
@@ -76,11 +76,11 @@ public:
   /// @{
   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);
   }
   /// @}
 
index e0fed029727955a312216d0e458028906ed63e8f..7f585ef3e75672de6e737eed8f23b2d0762bc139 100644 (file)
@@ -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 @@ 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<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 @@ public:
   /// 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 @@ 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 <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 @@ public:
   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 @@ 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 (file)
index 0000000..e90446f
--- /dev/null
@@ -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
+
index f69b347319443aab30d6a8e93b07719f2d215df9..e7ee8ee7ed378962082d47468f57bfd0e8840836 100644 (file)
@@ -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(
index 951bca9a301592021ffd470e6f75dfe2ea5e055a..595f79cb619d5c03edb755ca420a2e15c24f1a97 100644 (file)
@@ -685,6 +685,18 @@ TEST(Matcher, BindTheSameNameInAlternatives) {
       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(