aaron.ballman added inline comments.

================
Comment at: include/clang/AST/ASTTypeTraits.h:81
 
+  /// \{
+  /// Return the AST node kind of this ASTNodeKind.
----------------
lebedev.ri wrote:
> aaron.ballman wrote:
> > These markings are a bit strange, can you explain them to me?
> It is weird, but i think this is the right solution.
> See `isAllowedToContainClause()` narrower.
> This `asOMPClauseKind()` allows to pass a whole matcher, and then distill the 
> inner output node type.
> I.e. now i can spell 
> `ompExecutableDirective(isAllowedToContainClause(ompDefaultClause()))`
> (and the `ompDefaultClause()` won't actually be used for matching!), instead 
> of doing something like e.g.
> `ompExecutableDirective(isAllowedToContainClause(OMPC_default))`
> which looks horrible, and will likely not work well with `clang-query`?
I think we may be talking about different things. I only meant the `\{` and 
`\}` comment markers. :-D I think the design is reasonable enough, but I'm not 
an OpenMP expert. Truth be told, I was mostly surprised that these pragmas will 
have corresponding AST matchers. I thought they were preprocessor directives 
and thus would be handled at that level, rather than the AST level. My only 
concern about the design of this is a pretty minor one: what happens if someone 
is using preprocessor callbacks and AST matchers at the same time -- will they 
get duplicate results for OpenMP directives? I suspect they will, but that 
doesn't mean we shouldn't AST match OpenMP clauses (they are in the AST, after 
all).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57112/new/

https://reviews.llvm.org/D57112



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to