[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
Manuel Klimek
klimek at google.com
Mon Sep 3 07:34:41 PDT 2012
On Fri, Aug 31, 2012 at 3:07 AM, Sean Silva <silvas at purdue.edu> 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 <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