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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3827
+/// \brief Instantiations for the \c equals matcher.
+/// TODO add support for FloatingLiteral and CharacterLiteral
+/// @{
----------------
Lekensteyn wrote:
> 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.
If it turns out to be more work than you want to put in, don't feel obligated to do it. I just thought it might be nice to round it out.


================
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(          \
----------------
Lekensteyn wrote:
> 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?
> 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).

Ah, thank you for the explanation. The fixed name part makes this harder.

> 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?

I'm not certain what that solution would look like in practice, but if it removes the magic numbers, that sounds promising (assuming the result isn't even more fragile, of course).


https://reviews.llvm.org/D33094