[ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

[lebedev.ri]

Summary

OMPClause is the base class, it is not descendant from any other class,
therefore for it to work with e.g. VariadicDynCastAllOfMatcher<>, it needs to be handled here.

Now, i'm not quite sure what else should be here,
i have simply followed the preexisting, prevalent pattern in these source files.

[ABataev]

lib/AST/ASTTypeTraits.cpp

127

Well, I think it would be good to filter out OMPC_flush somehow because there is no such clause actually, it is a pseudo clause for better handling of the flush directive.

184

const auto *

[lebedev.ri]

@ABataev Thank you for taking a look!

lib/AST/ASTTypeTraits.cpp

127

Are OMPC_threadprivate and OMPC_uniform also in the same boat?
I don't see those clauses in spec.

Perhaps OMPC_flush should be made more like those other two?
(i.e. handled outside of OPENMP_CLAUSE macro)

[ABataev]

lib/AST/ASTTypeTraits.cpp

127

OMPC_threadprivate is also a special kind of pseudo-clause.
OMPC_flush is in the enum, because it has the corresponding class. You can try to exclude it from the enum, but it may require some additional work.
OMPC_uniform is a normal clause, but it has the corresponding class. This clause can be used on declare simd directive, which is represented as an attribute.

127

I mean, OOMPC_uniform has no(!) corresponding class. Mistyped

[lebedev.ri]

lib/AST/ASTTypeTraits.cpp

127

As one would expect, simply adding

case OMPC_flush: // Pseudo clause, does not exist (keep before including .def)
  llvm_unreachable("unexpected OpenMP clause kind");

results in a

[58/1118 5.6/sec] Building CXX object tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o
FAILED: tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o 
/usr/bin/g++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/lib/AST -I/build/clang/lib/AST -I/build/clang/include -Itools/clang/include -I/usr/include/libxml2 -Iinclude -I/build/llvm/include -pipe -O2 -g0 -UNDEBUG -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -pipe -O2 -g0 -UNDEBUG -fPIC   -UNDEBUG  -fno-exceptions -fno-rtti -MD -MT tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o -MF tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o.d -o tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o -c /build/clang/lib/AST/ASTTypeTraits.cpp
/build/clang/include/clang/Basic/OpenMPKinds.def: In static member function ‘static clang::ast_type_traits::ASTNodeKind clang::ast_type_traits::ASTNodeKind::getFromNode(const clang::OMPClause&)’:
/build/clang/lib/AST/ASTTypeTraits.cpp:116:5: error: duplicate case value
     case OMPC_##Name: return ASTNodeKind(NKI_##Class);
     ^~~~
/build/clang/include/clang/Basic/OpenMPKinds.def:261:1: note: in expansion of macro ‘OPENMP_CLAUSE’
 OPENMP_CLAUSE(flush, OMPFlushClause)
 ^~~~~~~~~~~~~
/build/clang/lib/AST/ASTTypeTraits.cpp:113:3: note: previously used here
   case OMPC_flush: // Pseudo clause, does not exist (keep before including .def)
   ^~~~

So one will need to pull OMPC_flush out of clang/Basic/OpenMPKinds.def.

[lebedev.ri]

lib/AST/ASTTypeTraits.cpp

127

D57280, will rebase this.

[lebedev.ri]

Rebased ontop of D57280, use const auto*.

[lebedev.ri]

Store OpenMP clause spelling in the ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[], not the stringified clang AST class name.

[lebedev.ri]

lib/AST/ASTTypeTraits.cpp

40–43

Humm, though i guess this will need to be changed back for when the matchers evolve
from the check into the ASTMatchers, for better clang-query expirience.

Would it be ok to add the opposite direction functionality
as compared to static ASTNodeKind getFromNode(const OMPClause &C);?
I.e. roughly OMPClauseKind getOMPClauseKindFromASTNodeKind() const; ?

[lebedev.ri]

• Rebased
• Go back to storing the actual AST class type as string in ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[].
• Add OpenMPClauseKind ASTNodeKind::getOMPClauseKind() to address the regression of D57113 due to the previous ^ change. This is better anyway, since it avoids string comparisons, and is a simple switch over an enum.

[lebedev.ri]

Ping. I'm not quite sure who is the best suited to review this..

[lebedev.ri]

Ping @hokein / @alexfh (as per git blame).
Not sure who is best suited to review this.

[alexfh]

In D57112#1399516, @lebedev.ri wrote:

Ping @hokein / @alexfh (as per git blame).
Not sure who is best suited to review this.

I only made a couple of random fixes to these files, so I don't feel particularly competent to review them. Though overall the changes in this patch seem reasonable to me. No concerns from me ;)

[lebedev.ri]

Maybe ping @klimek then? :)

In D57112#1400046, @alexfh wrote:

In D57112#1399516, @lebedev.ri wrote:

Ping @hokein / @alexfh (as per git blame).
Not sure who is best suited to review this.

I only made a couple of random fixes to these files, so I don't feel particularly competent to review them. Though overall the changes in this patch seem reasonable to me. No concerns from me ;)

Makes sense, thanks for looking!

[lebedev.ri]

ping @klimek @hokein thanks.
The fun of of the code that is so well separated that no one
touches it for years and thus can't review patches for it :)

[gribozavr]

The test coverage i have will be in the clang-tidy check.

Please add unit tests to clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp. You can pull matchers from D57113 into the shared matchers library (instead of keeping them private to the check), and add unit tests.

include/clang/AST/ASTTypeTraits.h

83

"asOMPClauseKind()"?

lib/AST/ASTTypeTraits.cpp

41

Why "DERIVED"? The definition of OPENMP_CLAUSE calls the second argument "Class" which I think is more appropriate.

70

Why "DERIVED"? The definition of OPENMP_CLAUSE calls the second argument "Class" which I think is more appropriate.

127

Well, I think it would be good to filter out OMPC_flush somehow because there is no such clause actually, it is a pseudo clause for better handling of the flush directive.

Sorry to be late for this discussion, but I don't think this conclusion follows. ASTMatchers are supposed to match the AST as it is. Even if OMPC_flush is synthetic, it exists in the AST, and users might want to match it. I think users would find anything else (trying to filter out AST nodes that are not in the source code) to be surprising. For example, there's a matcher materializeTemporaryExpr even though this AST node is a Clang invention and is not a part of the C++ spec.

Matching only constructs that appear in the source code is not feasible with ASTMatchers, because they are based on Clang's AST that exposes tons of semantic information, and its design is dictated by the structure of the semantic information. See "RFC: Tree-based refactorings with Clang" in cfe-dev for a library that will focus on representing source code as faithfully as possible.

Not to even mention that this code is in ASTTypeTraits, a general library for handling AST nodes, not specifically for AST Matchers...

[lebedev.ri]

lib/AST/ASTTypeTraits.cpp

127

So, uh, i should have checked beforehand.
https://godbolt.org/z/aanQ8U
D57280 is unjustified (and thus incorrect) and needs to be reverted.

[lebedev.ri]

Split off matchers from from D57113, added tests.

[aaron.ballman]

include/clang/AST/ASTTypeTraits.h

81

These markings are a bit strange, can you explain them to me?

include/clang/ASTMatchers/ASTMatchers.h

6389

Given OpenMP -> Given an OpenMP

lib/ASTMatchers/Dynamic/Registry.cpp

509–512

Please keep these alphabetically sorted.

[lebedev.ri]

include/clang/AST/ASTTypeTraits.h

81

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?

[aaron.ballman]

include/clang/AST/ASTTypeTraits.h

81

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).

[lebedev.ri]

include/clang/AST/ASTTypeTraits.h

81

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.

[aaron.ballman]

include/clang/AST/ASTTypeTraits.h

81

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.

[gribozavr]

include/clang/ASTMatchers/ASTMatchers.h

6390

Other comments usually don't explain the interactions with inner matchers, unless the semantics are unusual.

"Matches any clause in an OpenMP directive."

6400

Looking at other similar matches, they usually follow the naming pattern hasAny~, WDYT?

(hasAnyDeclaration, hasAnyConstructorInitializer etc.)

6411

Please follow the existing comment style. Either:

Given

\code
<code snippet>
\endcode

<matcher expression> matches "<code snippet>".

or

Example: <matcher expression> matches "<code snippet>" in 

\code
<code snippet>
\endcode

For example:

Given

 \code
   #pragma omp parallel default(none)
   #pragma omp parallel default(shared)
   #pragma omp parallel
 \endcode

``ompDefaultClause()`` matches ``default(none)` and ``default(shared)``.

Similarly for other comments in this patch.

6433

Also add isSharedKind? It is weird that we have one but not the other.

6449

"while this matcher"

6456

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.)

unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

2283

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.

[lebedev.ri]

include/clang/ASTMatchers/ASTMatchers.h

6456

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?

unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

2283

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.

[gribozavr]

include/clang/ASTMatchers/ASTMatchers.h

6456

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.

unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

2283

Other tests in this file use string concatenation, see TEST(DeclarationMatcher, MatchHasRecursiveAllOf) for example.

[lebedev.ri]

include/clang/ASTMatchers/ASTMatchers.h

6456

True. Also, but there's dosens of Stmt types, and there is no overload that takes StmtClass enum.

unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

2283

I'm sorry, but i fail to see how that is relevant?
I'm using multiline raw string literals, and inlining it will break the build, like i linked.
You are pointing at the code that is not using multiline raw string literals.
You only suggested inlining, not switching away from multiline raw string literals, i believe?

Not using multiline raw string literals looked even worse, because then you need to manually add "\n"

[gribozavr]

include/clang/ASTMatchers/ASTMatchers.h

6456

For Stmts, we do have dozens of individual matchers for them.

The point of your work is to add ASTMatchers for OpenMP, right? However, if there are no matchers for a reasonable amount of AST surface, it is as good as if the matchers are not there, because prospective users won't be able to use them.

I don't particularly care how exactly this is achieved, through individual matchers or through a matcher that takes an enum. However, I want to make sure that if you're going through all this trouble to add matchers, the resulting API should cover a good amount of AST.

The reason why I suggested to pass the enum to the matcher is simply because it is less code duplication, less work, and more reliable code (since there will be only one matcher to review, test, and maintain, instead of combinations of matchers).

Another reason to not use an inner matcher here is the peculiar semantics of this function -- it does not evaluate the matcher, and it does not accept a matcher expression of any shape.

unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

2283

You only suggested inlining, not switching away from multiline raw string literals, i believe?

I am now suggesting to switch away from raw string literals.

Not using multiline raw string literals looked even worse, because then you need to manually add "\n"

I believe that adding "\n" manually is better than having lots of similarly-named SourceX variables, which can easily cause copy-paste mistakes (define a SourceX variable, use SourceY in the EXPECT_TRUE line).

However, this is a minor point, up to you. I only wanted to explain my reasoning why I prefer inline code snippets.

[lebedev.ri]

include/clang/ASTMatchers/ASTMatchers.h

6456

The point of your work is to add ASTMatchers for OpenMP, right?

Absolutely not.
D57113 + D59466 is the one and only point, to address the bugs i have personally encountered.
The whole reason why i have started off with NOT adding these matchers to the ASTMatchers.h,
but keeping them at least initially internal to the checks was to avoid all this bikeshedding.

[gribozavr]

include/clang/ASTMatchers/ASTMatchers.h

6456

However, I do care about the AST matchers being usable by other clients.

I also care about the API following existing patterns:

Another reason to not use an inner matcher here is the peculiar semantics of this function -- it does not evaluate the matcher, and it does not accept a matcher expression of any shape.

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

6400

Yeah, I think this would be a good change to make. Good catch!

6433

Given that there's only two options, it's not strictly required to have them both. But I don't see a reason not to add it, either.

6456

Also, how will passing some random enum work with e.g. clang-query?

See llvm/tools/clang/lib/ASTMatchers/Dynamic/Marshallers.h.

That doesn't mean it works super well, though. String literals more easily contain silent typos, don't have autocomplete support, etc. I can definitely sympathize with not wanting to use an enum here.

However, I see that there are 50+ enumerations in this list -- that seems like too many matchers to want to expose. I think an enum will be the better, more maintainable option. The current approach won't scale well.

unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

2283

FWIW, I tend to prefer avoiding local variables and just inline the string literal into the matches() arguments. The local variable adds no value unless the snippet is very complex.

[lebedev.ri]

Rebased, addressed all(?) nits.

include/clang/ASTMatchers/ASTMatchers.h

6456

Okay, but apparently clang-query will needs to be fixed too:

clang-query> match stmt(ompExecutableDirective(isAllowedToContainClause(OMPC_default)))
1:1: Error parsing argument 1 for matcher stmt.
1:6: Error parsing argument 1 for matcher ompExecutableDirective.
1:29: Error parsing argument 1 for matcher isAllowedToContainClause.
1:58: Error parsing matcher. Found token <_> while looking for '('.

https://bugs.llvm.org/show_bug.cgi?id=41176

unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

2283

Same as with D59214, i have put the first line of the multiline string literal on the first line.
I can not inline it into the call, because that call is within the macro, and as i have linked in that issue, that will break build.
I do not believe avoiding multiline string literal (and thus explicitly spelling every "\n") would be better.

[lebedev.ri]

@gribozavr thank you for the review!
@aaron.ballman any further comments?

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

6456

clang-query requires enumerations to be quoted string literals. If you switch to that in your test, does it work for you? I was spotting some odd behavior with a different matcher (the attribute one, which documents the quoting requirement).

6465–6467

See the matcher for hasAttr() -- we should use similar exposition here.

[lebedev.ri]

Address @aaron.ballman comment nit.

[aaron.ballman]

LGTM aside from a small docs nit (be sure to regenerate the docs after you fix it).

include/clang/ASTMatchers/ASTMatchers.h

6398

hasAnyClause()

[lebedev.ri]

Comment nit.

include/clang/ASTMatchers/ASTMatchers.h

6398

whoops, thanks.

[lebedev.ri]

In D57112#1437954, @aaron.ballman wrote:

LGTM

Great, thank you for the review!

docs/LibASTMatchersReference.html

namespace ns {
     void g() { A::f(); }
   }
   ns::A a;
 nestedNameSpecifier()
   matches "ns::" and both "A::"
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1OMPClause.html">OMPClause</a>&gt;</td><td class="name" onclick="toggle('ompDefaultClause0')"><a name="ompDefaultClause0Anchor">ompDefaultClause</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1OMPDefaultClause.html">OMPDefaultClause</a>&gt;...</td></tr>
+<tr><td colspan="4" class="doc" id="ompDefaultClause0"><pre>Matches OpenMP ``default`` clause.
+
+Example:
+
+Given, `ompDefaultClause()`:
+
+  #pragma omp parallel default(none)   // &lt;- will match ``default(none)``
+  #pragma omp parallel default(shared) // &lt;- will match ``default(shared)``
+  #pragma omp parallel                 // &lt;- no match
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1QualType.html">QualType</a>&gt;</td><td class="name" onclick="toggle('qualType0')"><a name="qualType0Anchor">qualType</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1QualType.html">QualType</a>&gt;...</td></tr>
 <tr><td colspan="4" class="doc" id="qualType0"><pre>Matches QualTypes in the clang AST.
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt;</td><td class="name" onclick="toggle('addrLabelExpr0')"><a name="addrLabelExpr0Anchor">addrLabelExpr</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1AddrLabelExpr.html">AddrLabelExpr</a>&gt;...</td></tr>
 <tr><td colspan="4" class="doc" id="addrLabelExpr0"><pre>Matches address of label statements (GNU extension).
 
Given
   namespace n {
   inline namespace m {}
   }
 functionDecl(isInline()) will match ::f().
 namespaceDecl(isInline()) will match n::m.
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1OMPDefaultClause.html">OMPDefaultClause</a>&gt;</td><td class="name" onclick="toggle('isNoneKind0')"><a name="isNoneKind0Anchor">isNoneKind</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="isNoneKind0"><pre>Matches if the OpenMP ``default`` clause has ``none`` kind specified.
+
+Example:
+
+Given, `ompDefaultClause(isNoneKind())`:
+
+  #pragma omp parallel default(none)   // &lt;- match
+  #pragma omp parallel default(shared) // &lt;- no match
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html">ObjCMessageExpr</a>&gt;</td><td class="name" onclick="toggle('argumentCountIs2')"><a name="argumentCountIs2Anchor">argumentCountIs</a></td><td>unsigned N</td></tr>
 <tr><td colspan="4" class="doc" id="argumentCountIs2"><pre>Checks that a call expression or a constructor call expression has
 a specific number of arguments (including absent default arguments).
 
 Example matches f(0, 0) (matcher = callExpr(argumentCountIs(2)))
   void f(int x, int y);
   f(0, 0);
 </pre></td></tr>
Given
   A::B::C c;
 nestedNameSpecifier(specifiesType(
   hasDeclaration(cxxRecordDecl(hasName("A")))
 ))
   matches "A::"
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1OMPExecutableDirective.html">OMPExecutableDirective</a>&gt;</td><td class="name" onclick="toggle('hasClause0')"><a name="hasClause0Anchor">hasClause</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1OMPClause.html">OMPClause</a>&gt; InnerMatcher</td></tr>
+<tr><td colspan="4" class="doc" id="hasClause0"><pre>Given OpenMP directive, matches the first clause (out of all specified),
+that matches InnerMatcher.
+
+Example:
+
+Given ``ompExecutableDirective(hasClause(anything()))``:
+
+  #pragma omp parallel               // &lt;- no clauses, no match
+  #pragma omp parallel default(none) // &lt;- has clauses, match
+</pre></td></tr>
+
+
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1OMPExecutableDirective.html">OMPExecutableDirective</a>&gt;</td><td class="name" onclick="toggle('isAllowedToContainClause0')"><a name="isAllowedToContainClause0Anchor">isAllowedToContainClause</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1OMPClause.html">OMPClause</a>&gt; ClauseMatcher</td></tr>
+<tr><td colspan="4" class="doc" id="isAllowedToContainClause0"><pre>Matches if the OpenMP directive is allowed to contain the specified OpenMP
+clause kind.
+
+Example:
+
+Given,
+  ``ompExecutableDirective(isAllowedToContainClause(ompDefaultClause()))``:
+
+  #pragma omp parallel     // &lt;- match
+  #pragma omp parallel for // &lt;- match
+  #pragma omp          for // &lt;- no match
+
+NOTE: while the matcher takes the *matcher* for the OpenMP clause,
+      it does *NOT* actually match that matcher. It only fetches the
+      desired OpenMP clause kind out of the matchers return type,
+      and uses said kind for the decision.
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html">ObjCMessageExpr</a>&gt;</td><td class="name" onclick="toggle('hasAnyArgument3')"><a name="hasAnyArgument3Anchor">hasAnyArgument</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasAnyArgument3"><pre>Matches any argument of a call expression or a constructor call
 expression, or an ObjC-message-send expression.
 
 Given
   void x(int, int, int) { int y; x(1, y, 42); }
 callExpr(hasAnyArgument(declRefExpr()))
   matches x(1, y, 42)

include/clang/AST/ASTTypeTraits.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_AST_ASTTYPETRAITS_H
 #define LLVM_CLANG_AST_ASTTYPETRAITS_H
 
 #include "clang/AST/ASTFwd.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/OpenMPClause.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Support/AlignOf.h"
 
 namespace llvm {
static ASTNodeKind getFromNodeKind() {
     return ASTNodeKind(KindToKindId<T>::Id);
   }
 
   /// \{
   /// Construct an identifier for the dynamic type of the node
   static ASTNodeKind getFromNode(const Decl &D);
   static ASTNodeKind getFromNode(const Stmt &S);
   static ASTNodeKind getFromNode(const Type &T);
+  static ASTNodeKind getFromNode(const OMPClause &C);
   /// \}
 
   /// Returns \c true if \c this and \c Other represent the same kind.
   bool isSame(ASTNodeKind Other) const {
     return KindId != NKI_None && KindId == Other.KindId;
   }
 
   /// Returns \c true only for the default \c ASTNodeKind()
   bool isNone() const { return KindId == NKI_None; }
 
   /// Returns \c true if \c this is a base kind of (or same as) \c Other.
   /// \param Distance If non-null, used to return the distance between \c this
   /// and \c Other in the class hierarchy.
   bool isBaseOf(ASTNodeKind Other, unsigned *Distance = nullptr) const;
 
   /// String representation of the kind.
   StringRef asStringRef() const;
 
+  /// \{
+  /// Return the AST node kind of this ASTNodeKind.
+  OpenMPClauseKind asOMPClauseKind() const;
+  /// \}
+
   /// Strict weak ordering for ASTNodeKind.
   bool operator<(const ASTNodeKind &Other) const {
     return KindId < Other.KindId;
   }
 
   /// Return the most derived type between \p Kind1 and \p Kind2.
   ///
   /// Return ASTNodeKind() if they are not related.

 #define DECL(DERIVED, BASE) NKI_##DERIVED##Decl,
 #include "clang/AST/DeclNodes.inc"
     NKI_Stmt,
 #define STMT(DERIVED, BASE) NKI_##DERIVED,
 #include "clang/AST/StmtNodes.inc"
     NKI_Type,
 #define TYPE(DERIVED, BASE) NKI_##DERIVED##Type,
 #include "clang/AST/TypeNodes.def"
+    NKI_OMPClause,
+#define OPENMP_CLAUSE(TextualSpelling, Class) NKI_##Class,
+#include "clang/Basic/OpenMPKinds.def"
     NKI_NumberOfKinds
   };
 
   /// Use getFromNodeKind<T>() to construct the kind.
   ASTNodeKind(NodeKindId KindId) : KindId(KindId) {}
 
   /// Returns \c true if \c Base is a base kind of (or same as) \c
   ///   Derived.

 KIND_TO_KIND_ID(TemplateName)
 KIND_TO_KIND_ID(NestedNameSpecifier)
 KIND_TO_KIND_ID(NestedNameSpecifierLoc)
 KIND_TO_KIND_ID(QualType)
 KIND_TO_KIND_ID(TypeLoc)
 KIND_TO_KIND_ID(Decl)
 KIND_TO_KIND_ID(Stmt)
 KIND_TO_KIND_ID(Type)
+KIND_TO_KIND_ID(OMPClause)
 #define DECL(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED##Decl)
 #include "clang/AST/DeclNodes.inc"
 #define STMT(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED)
 #include "clang/AST/StmtNodes.inc"
 #define TYPE(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED##Type)
 #include "clang/AST/TypeNodes.def"
+#define OPENMP_CLAUSE(TextualSpelling, Class) KIND_TO_KIND_ID(Class)
+#include "clang/Basic/OpenMPKinds.def"
 #undef KIND_TO_KIND_ID
 
 inline raw_ostream &operator<<(raw_ostream &OS, ASTNodeKind K) {
   OS << K.asStringRef();
   return OS;
 }
 
 /// A dynamically typed AST node container.
struct DynTypedNode::BaseConverter<
     T, typename std::enable_if<std::is_base_of<Stmt, T>::value>::type>
     : public DynCastPtrConverter<T, Stmt> {};
 
 template <typename T>
 struct DynTypedNode::BaseConverter<
     T, typename std::enable_if<std::is_base_of<Type, T>::value>::type>
     : public DynCastPtrConverter<T, Type> {};
 
+template <typename T>
+struct DynTypedNode::BaseConverter<
+    T, typename std::enable_if<std::is_base_of<OMPClause, T>::value>::type>
+    : public DynCastPtrConverter<T, OMPClause> {};
+
 template <>
 struct DynTypedNode::BaseConverter<
     NestedNameSpecifier, void> : public PtrConverter<NestedNameSpecifier> {};
 
 template <>
 struct DynTypedNode::BaseConverter<
     CXXCtorInitializer, void> : public PtrConverter<CXXCtorInitializer> {};
 

include/clang/ASTMatchers/ASTMatchers.h

 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/OpenMPClause.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"

 ///
 /// \code
 ///   #pragma omp parallel
 ///   #pragma omp for <...>
 /// \endcode
 extern const internal::VariadicDynCastAllOfMatcher<Stmt, OMPExecutableDirective>
     ompExecutableDirective;
 
+/// Given OpenMP directive, matches the first clause (out of all specified),
+/// that matches InnerMatcher.
+///
+/// Example:
+///
+/// Given ``ompExecutableDirective(hasClause(anything()))``:
+///
+/// \code
+///   #pragma omp parallel               // <- no clauses, no match
+///   #pragma omp parallel default(none) // <- has clauses, match
+/// \endcode
+AST_MATCHER_P(OMPExecutableDirective, hasClause, internal::Matcher<OMPClause>,
+              InnerMatcher) {
+  ArrayRef<OMPClause *> Clauses = Node.clauses();
+  return matchesFirstInPointerRange(InnerMatcher, Clauses.begin(),
+                                    Clauses.end(), Finder, Builder);
+}
+
+/// Matches OpenMP ``default`` clause.
+///
+/// Example:
+///
+/// Given, `ompDefaultClause()`:
+///
+/// \code
+///   #pragma omp parallel default(none)   // <- will match ``default(none)``
+///   #pragma omp parallel default(shared) // <- will match ``default(shared)``
+///   #pragma omp parallel                 // <- no match
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher<OMPClause, OMPDefaultClause>
+    ompDefaultClause;
+
+/// Matches if the OpenMP ``default`` clause has ``none`` kind specified.
+///
+/// Example:
+///
+/// Given, `ompDefaultClause(isNoneKind())`:
+///
+/// \code
+///   #pragma omp parallel default(none)   // <- match
+///   #pragma omp parallel default(shared) // <- no match
+/// \endcode
+AST_MATCHER(OMPDefaultClause, isNoneKind) {
+  return Node.getDefaultKind() == OMPC_DEFAULT_none;
+}
+
+/// Matches if the OpenMP directive is allowed to contain the specified OpenMP
+/// clause kind.
+///
+/// Example:
+///
+/// Given,
+///   ``ompExecutableDirective(isAllowedToContainClause(ompDefaultClause()))``:
+///
+/// \code
+///   #pragma omp parallel     // <- match
+///   #pragma omp parallel for // <- match
+///   #pragma omp          for // <- no match
+/// \endcode
+///
+/// NOTE: while the matcher takes the *matcher* for the OpenMP clause,
+///       it does *NOT* actually match that matcher. It only fetches the
+///       desired OpenMP clause kind out of the matchers return type,
+///       and uses said kind for the decision.
+AST_MATCHER_P(OMPExecutableDirective, isAllowedToContainClause,
+              internal::Matcher<OMPClause>, ClauseMatcher) {
+  // 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.
+
+  internal::DynTypedMatcher::MatcherIDType ID = ClauseMatcher.getID();
+  ast_type_traits::ASTNodeKind NodeKind = ID.first;
+  assert(!NodeKind.isNone() &&
+         "expected to have a matcher for some OpenMP clause, not the matcher "
+         "for the OMPClause itself");
+  OpenMPClauseKind CKind = NodeKind.asOMPClauseKind();
+  assert(CKind != OMPC_unknown && "Bad ClauseMatcher specified.");
+  return isAllowedClauseForDirective(Node.getDirectiveKind(), CKind);
+}
+
 //----------------------------------------------------------------------------//
 // End OpenMP handling.
 //----------------------------------------------------------------------------//
 
 } // namespace ast_matchers
 } // namespace clang
 
 #endif // LLVM_CLANG_ASTMATCHERS_ASTMATCHERS_H

lib/AST/ASTTypeTraits.cpp

 #define DECL(DERIVED, BASE) { NKI_##BASE, #DERIVED "Decl" },
 #include "clang/AST/DeclNodes.inc"
   { NKI_None, "Stmt" },
 #define STMT(DERIVED, BASE) { NKI_##BASE, #DERIVED },
 #include "clang/AST/StmtNodes.inc"
   { NKI_None, "Type" },
 #define TYPE(DERIVED, BASE) { NKI_##BASE, #DERIVED "Type" },
 #include "clang/AST/TypeNodes.def"
+  { NKI_None, "OMPClause" },
+#define OPENMP_CLAUSE(TextualSpelling, Class) {NKI_OMPClause, #Class},
+#include "clang/Basic/OpenMPKinds.def"
 };
 
 bool ASTNodeKind::isBaseOf(ASTNodeKind Other, unsigned *Distance) const {
   return isBaseOf(KindId, Other.KindId, Distance);
 }
 
 bool ASTNodeKind::isBaseOf(NodeKindId Base, NodeKindId Derived,
                            unsigned *Distance) {
   if (Base == NKI_None || Derived == NKI_None) return false;
   unsigned Dist = 0;
   while (Derived != Base && Derived != NKI_None) {
     Derived = AllKindInfo[Derived].ParentId;
     ++Dist;
   }
   if (Distance)
     *Distance = Dist;
   return Derived == Base;
 }
 
 StringRef ASTNodeKind::asStringRef() const { return AllKindInfo[KindId].Name; }
 
+OpenMPClauseKind ASTNodeKind::asOMPClauseKind() const {
+  assert(isBaseOf(NKI_OMPClause, KindId, nullptr) && KindId != NKI_OMPClause &&
+         "Should be an OpenMP clause node");
+  switch (KindId) {
+  default:
+    llvm_unreachable("Unknown clause kind!");
+#define OPENMP_CLAUSE(TextualSpelling, Class)                                  \
+  case NKI_##Class:                                                            \
+    return OMPC_##TextualSpelling;
+#include "clang/Basic/OpenMPKinds.def"
+  }
+}
+
 ASTNodeKind ASTNodeKind::getMostDerivedType(ASTNodeKind Kind1,
                                             ASTNodeKind Kind2) {
   if (Kind1.isBaseOf(Kind2)) return Kind2;
   if (Kind2.isBaseOf(Kind1)) return Kind1;
   return ASTNodeKind();
 }
 
 ASTNodeKind ASTNodeKind::getMostDerivedCommonAncestor(ASTNodeKind Kind1,

 ASTNodeKind ASTNodeKind::getFromNode(const Type &T) {
   switch (T.getTypeClass()) {
 #define TYPE(Class, Base)                                                      \
     case Type::Class: return ASTNodeKind(NKI_##Class##Type);
 #define ABSTRACT_TYPE(Class, Base)
 #include "clang/AST/TypeNodes.def"
   }
   llvm_unreachable("invalid type kind");
  }
 
+ASTNodeKind ASTNodeKind::getFromNode(const OMPClause &C) {
+  switch (C.getClauseKind()) {
+#define OPENMP_CLAUSE(Name, Class)                                             \
+    case OMPC_##Name: return ASTNodeKind(NKI_##Class);
+#include "clang/Basic/OpenMPKinds.def"
+  case OMPC_allocate:
+  case OMPC_threadprivate:
+  case OMPC_uniform:
+  case OMPC_unknown:
+    llvm_unreachable("unexpected OpenMP clause kind");
+  }
+  llvm_unreachable("invalid stmt kind");
+}
+
 void DynTypedNode::print(llvm::raw_ostream &OS,
                          const PrintingPolicy &PP) const {
   if (const TemplateArgument *TA = get<TemplateArgument>())
     TA->print(PP, OS);
   else if (const TemplateName *TN = get<TemplateName>())
     TN->print(OS, PP);
   else if (const NestedNameSpecifier *NNS = get<NestedNameSpecifier>())
     NNS->print(OS, PP);
SourceRange DynTypedNode::getSourceRange() const {
   if (const NestedNameSpecifierLoc *NNSL = get<NestedNameSpecifierLoc>())
     return NNSL->getSourceRange();
   if (const TypeLoc *TL = get<TypeLoc>())
     return TL->getSourceRange();
   if (const Decl *D = get<Decl>())
     return D->getSourceRange();
   if (const Stmt *S = get<Stmt>())
     return S->getSourceRange();
+  if (const auto *C = get<OMPClause>())
+    return SourceRange(C->getBeginLoc(), C->getEndLoc());
   return SourceRange();
 }
 
 } // end namespace ast_type_traits
 } // end namespace clang

lib/ASTMatchers/ASTMatchersInternal.cpp

AST_TYPELOC_TRAVERSE_MATCHER_DEF(hasValueType,
                                  AST_POLYMORPHIC_SUPPORTED_TYPES(AtomicType));
 AST_TYPELOC_TRAVERSE_MATCHER_DEF(
     pointee,
     AST_POLYMORPHIC_SUPPORTED_TYPES(BlockPointerType, MemberPointerType,
                                     PointerType, ReferenceType));
 
 const internal::VariadicDynCastAllOfMatcher<Stmt, OMPExecutableDirective>
     ompExecutableDirective;
+const internal::VariadicDynCastAllOfMatcher<OMPClause, OMPDefaultClause>
+    ompDefaultClause;
 
 } // end namespace ast_matchers
 } // end namespace clang

lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(usingDirectiveDecl);
   REGISTER_MATCHER(valueDecl);
   REGISTER_MATCHER(varDecl);
   REGISTER_MATCHER(variableArrayType);
   REGISTER_MATCHER(voidType);
   REGISTER_MATCHER(whileStmt);
   REGISTER_MATCHER(withInitializer);
   REGISTER_MATCHER(ompExecutableDirective);
+  REGISTER_MATCHER(hasClause);
+  REGISTER_MATCHER(ompDefaultClause);
+  REGISTER_MATCHER(isNoneKind);
+  REGISTER_MATCHER(isAllowedToContainClause);
 }
 
 RegistryMaps::~RegistryMaps() = default;
 
 static llvm::ManagedStatic<RegistryMaps> RegistryData;
 
 // static
 llvm::Optional<MatcherCtor> Registry::lookupMatcherCtor(StringRef MatcherName) {

unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

 TEST(Matcher, isMain) {
   EXPECT_TRUE(
     matches("int main() {}", functionDecl(isMain())));
 
   EXPECT_TRUE(
     notMatches("int main2() {}", functionDecl(isMain())));
 }
 
+TEST(OMPExecutableDirective, hasClause) {
+  auto Matcher = ompExecutableDirective(hasClause(anything()));
+
+  const std::string Source0 = R"(void x() {
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source0, Matcher));
+
+  const std::string Source1 = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source1, Matcher));
+
+  const std::string Source2 = R"(void x() {
+#pragma omp parallel default(none)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source2, Matcher));
+
+  const std::string Source3 = R"(void x() {
+#pragma omp parallel default(shared)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
+
+  const std::string Source4 = R"(void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+}
+
+TEST(OMPDefaultClause, isNoneKind) {
+  auto Matcher =
+      ompExecutableDirective(hasClause(ompDefaultClause(isNoneKind())));
+
+  const std::string Source0 = R"(void x() {
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source0, Matcher));
+
+  const std::string Source1 = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source1, Matcher));
+
+  const std::string Source2 = R"(void x() {
+#pragma omp parallel default(none)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source2, Matcher));
+
+  const std::string Source3 = R"(void x() {
+#pragma omp parallel default(shared)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source3, Matcher));
+
+  const std::string Source4 = R"(void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+}
+
+TEST(OMPExecutableDirective, isAllowedToContainClause) {
+  auto Matcher =
+      ompExecutableDirective(isAllowedToContainClause(ompDefaultClause()));
+
+  const std::string Source0 = R"(void x() {
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source0, Matcher));
+
+  const std::string Source1 = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source1, Matcher));
+
+  const std::string Source2 = R"(void x() {
+#pragma omp parallel default(none)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source2, Matcher));
+
+  const std::string Source3 = R"(void x() {
+#pragma omp parallel default(shared)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
+
+  const std::string Source4 = R"(void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(void x() {
+#pragma omp taskyield
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
+
+  const std::string Source6 = R"(void x() {
+#pragma omp task
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source6, Matcher));
+}
+
 } // namespace ast_matchers
 } // namespace clang

unittests/ASTMatchers/ASTMatchersNodeTest.cpp

;
   EXPECT_TRUE(matchesWithOpenMP(SourceGood, Matcher));
 
   const std::string SourceBad = R"(void x() {
 ;
 })";
   EXPECT_TRUE(notMatchesWithOpenMP(SourceBad, Matcher));
 }
 
+TEST(OMPDefaultClause, Matches) {
+  auto Matcher = ompExecutableDirective(hasClause(ompDefaultClause()));
+
+  const std::string Source0 = R"(void x() {
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source0, Matcher));
+
+  const std::string Source1 = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source1, Matcher));
+
+  const std::string Source2 = R"(void x() {
+#pragma omp parallel default(none)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source2, Matcher));
+
+  const std::string Source3 = R"(void x() {
+#pragma omp parallel default(shared)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
+
+  const std::string Source4 = R"(void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+}
+
 } // namespace ast_matchers
 } // namespace clang