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

Reply via email to