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:
> > 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.
> 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 thought so as well, but there's no grouping here. I'd probably drop them
until it's needed.
> 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.
I agree.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57112/new/
https://reviews.llvm.org/D57112
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits