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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits