gribozavr added inline comments.
================ Comment at: include/clang/ASTMatchers/ASTMatchers.h:6456 + // Note that we have recieved a *matcher* for the clause, not the + // OpenMPClauseKind. We now need to extract the 'return' type of said matcher, + // and convert it to the OpenMPClauseKind, so we can finally use that. ---------------- lebedev.ri wrote: > gribozavr wrote: > > Why not make `isAllowedToContainClause` take an `OpenMPClauseKind` enum > > value? > > > > I don't see right now advantages for taking a matcher. (For example, it > > can't be a more complex matcher with inner matchers, it can't be a > > disjunction of matchers etc.) > I don't feel like it, it's uglier. > The matcher is documented, `OpenMPClauseKind` is not documented. > Also, how will passing some random enum work with e.g. clang-query? > There are dozens of clauses in `OpenMPClauseKind`. We would have to replicate them all as matchers to provide a useful API. > Also, how will passing some random enum work with e.g. clang-query? See `llvm/tools/clang/lib/ASTMatchers/Dynamic/Marshallers.h`. ================ Comment at: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:2283 +})"; + EXPECT_TRUE(notMatchesWithOpenMP(Source0, Matcher)); + ---------------- lebedev.ri wrote: > gribozavr wrote: > > I'm not sure if breaking out the source code into the "SourceX" variables > > improves readability. WDYT about inlining the code into the EXPECT_TRUE > > code like in other tests in this file? > > > > If you want to break it out, I'd suggest to drop "`void x() {`" down to the > > next line, so that all code lines start at the same column. > > I'm not sure if breaking out the source code into the "SourceX" variables > > improves readability > > It's not about readability. Inlining will break the build, rC354201. Other tests in this file use string concatenation, see `TEST(DeclarationMatcher, MatchHasRecursiveAllOf)` for example. 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