================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to