[cfe-commits] [PATCH] Modifying BoundNodes to store bindings as void* pointers and use getNodeAs instead of getDeclAs and getStmtAs.

[Michael Diamond]

================
Comment at: google3/third_party/llvm/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:77
@@ +76,3 @@
+
+/// \brief Indicates the base type of a bound AST node.
+/// Used for storing nodes as void*.
----------------
Daniel Jasper wrote:
> You'll need a blank comment line so doxygen understands where \brief ends.
Done.

================
Comment at: google3/third_party/llvm/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:86
@@ +85,3 @@
+
+/// \brief Utility to manipulate nodes of a given base type.
+/// We use template specialization on the node base type to enable us to
----------------
Daniel Jasper wrote:
> You'll need a blank comment line so doxygen understands where \brief ends.
Done.

================
Comment at: google3/third_party/llvm/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:88
@@ +87,3 @@
+/// We use template specialization on the node base type to enable us to
+/// get at the appropriate NodeBaseType objects and do approrpiate static_casts.
+template <typename BaseType>
----------------
Daniel Jasper wrote:
> Manuel Klimek wrote:
> > appropriate
> nit: appropriate static_casts (-r)
Done.

================
Comment at: google3/third_party/llvm/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:126-128
@@ +125,5 @@
+/// @{
+NODE_BASE_TYPE_UTIL(Decl);
+NODE_BASE_TYPE_UTIL(Stmt);
+NODE_BASE_TYPE_UTIL(QualType);
+/// @}
----------------
Daniel Jasper wrote:
> It is very important that this list is in sync with the enum declaring NT_*. Ideally, we would not need both, but if we do, could we put the right next to each other in the code?
Unfortunately, no. We require NodeBaseType in the definition of NodeBaseTypeUtil. I've added a comment to the enum to update both get _base_type and the list of specializations.

================
Comment at: google3/third_party/llvm/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:140
@@ +139,3 @@
+public:
+  /// \brief Adds \c Node to the map with key \c ID.
+  /// The node's base type should be in NodeBaseType or it will be unaccessible.
----------------
Daniel Jasper wrote:
> You'll need a blank comment line so doxygen understands where \brief ends.
Done.

================
Comment at: google3/third_party/llvm/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:147
@@ +146,3 @@
+
+  /// \brief Returns the AST node bound to \c ID.
+  /// Returns NULL if there was no node bound to \c ID or if there is a node but
----------------
Daniel Jasper wrote:
> You'll need a blank comment line so doxygen understands where \brief ends.
Done.

================
Comment at: google3/third_party/llvm/llvm/tools/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:71
@@ +70,3 @@
+struct get_base_type {
+  typedef GET_BASE_TYPE(Decl,
+          GET_BASE_TYPE(Stmt,
----------------
Daniel Jasper wrote:
> First I thought that these recursive macros are bad, but on second thought, I kind of like it :-).
It makes is much easier to add new types. :)


http://llvm-reviews.chandlerc.com/D25