[PATCH] D33094: [ASTMatchers] Add equals support for integer and boolean literals

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 11 05:54:11 PDT 2017


aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.

Please be sure to regenerate the AST matcher documentation as well by running clang/docs/tools/dump_ast_matchers.py



================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3827
+/// \brief Instantiations for the \c equals matcher.
+/// TODO add support for FloatingLiteral and CharacterLiteral
+/// @{
----------------
Why not add this support immediately rather than a TODO?


================
Comment at: include/clang/ASTMatchers/ASTMatchersMacros.h:364
+/// comparing a \c ReturnType node against the a \c ParamType value.
+#define AST_CONCRETE_EQUALS_MATCHER(ReturnType, ParamType, OverloadId)         \
+  inline ::clang::ast_matchers::internal::Matcher<ReturnType> equals(          \
----------------
Instead of making the user of the macro pass in an overload id, could we make use of the `__LINE__` macro to automate it? Given the length of the macro name, I struggle to imagine many people accidentally defining two overloads on the same line (and we can document this macro appropriately, of course).


================
Comment at: include/clang/ASTMatchers/ASTMatchersMacros.h:366
+  inline ::clang::ast_matchers::internal::Matcher<ReturnType> equals(          \
+      ParamType const &Param) {                                                \
+    return ::clang::ast_matchers::internal::makeMatcher(                       \
----------------
`const ParamType &Param` per the coding guidelines.


================
Comment at: include/clang/ASTMatchers/ASTMatchersMacros.h:371
+  typedef ::clang::ast_matchers::internal::Matcher<ReturnType>(                \
+      &equals_Type##OverloadId)(ParamType const &Param)
+
----------------
Same here.


https://reviews.llvm.org/D33094





More information about the cfe-commits mailing list