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

Peter Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 11 06:31:08 PDT 2017


Lekensteyn added a comment.

In https://reviews.llvm.org/D33094#752153, @aaron.ballman wrote:

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


Will do, thanks for the hint.



================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3827
+/// \brief Instantiations for the \c equals matcher.
+/// TODO add support for FloatingLiteral and CharacterLiteral
+/// @{
----------------
aaron.ballman wrote:
> Why not add this support immediately rather than a TODO?
The parser would need additional changes for it to be usable in clang-query (see D33093 for boolean support), my initial focus was on supporting IntegerLiterals but bool was added to have an overload.

I'll look into adding support for other types.


================
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(          \
----------------
aaron.ballman wrote:
> 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).
Not sure how `__LINE__` would help, uniqueness is not the only requirement, it must also be a fixed name such that Registry can refer to it. (Uniqueness is also needed, otherwise it would be ambiguous).

Alternatively, the overload name can be removed, but then the returntype and paramtype for the marshaller should be explicitly specified. This would remove the magic numbers (for overload ID), is this an approach worth looking into?


================
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(                       \
----------------
aaron.ballman wrote:
> `const ParamType &Param` per the coding guidelines.
Will fix (this was copied from the above macros).


https://reviews.llvm.org/D33094





More information about the cfe-commits mailing list