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