[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 17 07:25:56 PDT 2019


lebedev.ri marked 3 inline comments as done.
lebedev.ri added inline comments.


================
Comment at: include/clang/AST/ASTTypeTraits.h:81
 
+  /// \{
+  /// Return the AST node kind of this ASTNodeKind.
----------------
aaron.ballman wrote:
> 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).
Oh duh xD
I just copied them from around the `getFromNode()` up there ^.
I believe they signify that these functions should be displayed as a group in doxygen.

I don't know what will happen when using preprocessor callbacks and AST matchers at the same time,
but i //think// that would be the issue of the user in question.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57112





More information about the cfe-commits mailing list