Add a new AST matcher 'optionally'.

[air20]

Summary

This matcher matches any node and at the same time executes all its
inner matchers to produce any possbile result bindings.

This is useful when a user wants certain supplementary information
that's not always present along with the main match result.

[kristina]

Add some reviewers.

[aaron.ballman]

Just to make sure I understand the purpose -- the goal here is to optionally match one or more inner matchers without failing even if none of the inner matchers match anything, and this is a different use case than anyOf() because that would fail when none of the inner matchers match?

clang/include/clang/ASTMatchers/ASTMatchers.h

2538

This needs to be registered in the dynamic matcher registry (Registry.cpp) as well. Also, you should regenerate the documentation by running clang\docs\tools\dump_ast_matchers.py.

I think this matcher could use some example uses in the documentation as well.

clang/lib/ASTMatchers/ASTMatchersInternal.cpp

362

You can elide the braces here.

[air20]

Fixed Registry.cpp and documentation as per comments.

[air20]

Included base commits that was missing in the previous diff.

[aaron.ballman]

LGTM aside from a minor nit. Do you need someone to commit on your behalf?

clang/lib/ASTMatchers/Dynamic/Registry.cpp

283

Why did this one move? Please keep the list sorted alphabetically.

[air20]

In D72233#1806483, @aaron.ballman wrote:

Just to make sure I understand the purpose -- the goal here is to optionally match one or more inner matchers without failing even if none of the inner matchers match anything, and this is a different use case than anyOf() because that would fail when none of the inner matchers match?

That’s exactly right.

In D72233#1807698, @aaron.ballman wrote:

LGTM aside from a minor nit. Do you need someone to commit on your behalf?

How do I submit a patch from here?

clang/lib/ASTMatchers/Dynamic/Registry.cpp

283

I think this wasn’t sorted correctly before.

[aaron.ballman]

In D72233#1808125, @air20 wrote:

In D72233#1807698, @aaron.ballman wrote:

LGTM aside from a minor nit. Do you need someone to commit on your behalf?

How do I submit a patch from here?

You would need git commit credentials for the project (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access). If you don't have those credentials, I'm happy to commit on your behalf.

clang/lib/ASTMatchers/Dynamic/Registry.cpp

283

I think it was sorted properly before (i comes before S even if you ignore capitalization), but regardless, it's a change unrelated to the patch, which we usually ask to be a separate patch.

[air20]

In D72233#1808890, @aaron.ballman wrote:

In D72233#1808125, @air20 wrote:

In D72233#1807698, @aaron.ballman wrote:

LGTM aside from a minor nit. Do you need someone to commit on your behalf?

How do I submit a patch from here?

You would need git commit credentials for the project (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access). If you don't have those credentials, I'm happy to commit on your behalf.

I don't have commit access for now. It'd be great if you can submit it for me :D

clang/lib/ASTMatchers/Dynamic/Registry.cpp

283

I trust my Vim for this :) I think uppercase letters have a smaller ASCII value so they come first.

Line 354-355 are sorted correctly, for example.

REGISTER_MATCHER(isConstQualified);
REGISTER_MATCHER(isConstexpr);

I'd rather not upload another patch for this though.

[aaron.ballman]

clang/lib/ASTMatchers/Dynamic/Registry.cpp

283

I trust my Vim for this :) I think uppercase letters have a smaller ASCII value so they come first.

I think it depends on your sort criteria and whether you are sorting this list for humans or computers. I sort it for humans where i comes before s alphabetically because the goal of alphabetizing this list is for a human to quickly scan it visually. (Computer sort order is unimportant in this case -- these are all being stored into a hash map where the order does not matter.)

I'd rather not upload another patch for this though.

This is a change unrelated to the patch at hand, so it should be removed from this patch. If you feel strongly about changing this, please start a separate patch.

[air20]

clang/lib/ASTMatchers/Dynamic/Registry.cpp

283

Reverted. But I thought the purpose of sorting is to make it look better and maintain *one* canonical order. Not a big deal anyhow.

[aaron.ballman]

Thank you for the patch! I've commit on your behalf in 2823e91d55891e33a7a8b9a4016db4ec9e2765ae

[steveire]

LGTM too. I described a more ad-hoc implementation previously: https://steveire.wordpress.com/2018/11/20/composing-ast-matchers-in-clang-tidy/ but I prefer this implementation.

clang/docs/LibASTMatchersReference.html

 Given
 void f() { for (;;) { int x = 42; if (true) { int x = 43; } } }
 compoundStmt(hasParent(ifStmt())) matches "{ int x = 43; }".
 
 Usable as: Any Matcher
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;*&gt;</td><td class="name" onclick="toggle('optionally0')"><a name="optionally0Anchor">optionally</a></td><td>Matcher&lt;*&gt;, ..., Matcher&lt;*&gt;</td></tr>
+<tr><td colspan="4" class="doc" id="optionally0"><pre>Matches any node regardless of the submatchers.
+
+However, optionally will generate a result binding for each matching
+submatcher.
+
+Useful when additional information which may or may not present about a
+main matching node is desired.
+
+For example, in:
+  class Foo {
+    int bar;
+  }
+The matcher:
+  cxxRecordDecl(
+    optionally(has(
+      fieldDecl(hasName("bar")).bind("var")
+  ))).bind("record")
+will produce a result binding for both "record" and "var".
+The matcher will produce a "record" binding for even if there is no data
+member named "bar" in that class.
+
+Usable as: Any Matcher
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1AbstractConditionalOperator.html">AbstractConditionalOperator</a>&gt;</td><td class="name" onclick="toggle('hasCondition5')"><a name="hasCondition5Anchor">hasCondition</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="hasCondition5"><pre>Matches the condition expression of an if statement, for loop,
 switch statement or conditional operator.
 
 Example matches true (matcher = hasCondition(cxxBoolLiteral(equals(true))))
   if (true) {}
 </pre></td></tr>
 
with compoundStmt()
   matching '{}'
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXForRangeStmt.html">CXXForRangeStmt</a>&gt;</td><td class="name" onclick="toggle('hasInitStatement2')"><a name="hasInitStatement2Anchor">hasInitStatement</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasInitStatement2"><pre>Matches selection statements with initializer.
 
 Given:
- void foo() { 
+ void foo() {
    if (int i = foobar(); i &gt; 0) {}
    switch (int i = foobar(); i) {}
-   for (auto&amp; a = get_range(); auto&amp; x : a) {} 
+   for (auto&amp; a = get_range(); auto&amp; x : a) {}
  }
- void bar() { 
+ void bar() {
    if (foobar() &gt; 0) {}
    switch (foobar()) {}
-   for (auto&amp; x : get_range()) {} 
+   for (auto&amp; x : get_range()) {}
  }
 ifStmt(hasInitStatement(anything()))
   matches the if statement in foo but not in bar.
 switchStmt(hasInitStatement(anything()))
   matches the switch statement in foo but not in bar.
 cxxForRangeStmt(hasInitStatement(anything()))
   matches the range for statement in foo but not in bar.
 </pre></td></tr>
Examples matches the if statement
   if (false) false; else true;
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1IfStmt.html">IfStmt</a>&gt;</td><td class="name" onclick="toggle('hasInitStatement0')"><a name="hasInitStatement0Anchor">hasInitStatement</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasInitStatement0"><pre>Matches selection statements with initializer.
 
 Given:
- void foo() { 
+ void foo() {
    if (int i = foobar(); i &gt; 0) {}
    switch (int i = foobar(); i) {}
-   for (auto&amp; a = get_range(); auto&amp; x : a) {} 
+   for (auto&amp; a = get_range(); auto&amp; x : a) {}
  }
- void bar() { 
+ void bar() {
    if (foobar() &gt; 0) {}
    switch (foobar()) {}
-   for (auto&amp; x : get_range()) {} 
+   for (auto&amp; x : get_range()) {}
  }
 ifStmt(hasInitStatement(anything()))
   matches the if statement in foo but not in bar.
 switchStmt(hasInitStatement(anything()))
   matches the switch statement in foo but not in bar.
 cxxForRangeStmt(hasInitStatement(anything()))
   matches the range for statement in foo but not in bar.
 </pre></td></tr>
Example matches true (matcher = hasCondition(cxxBoolLiteral(equals(true))))
   if (true) {}
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1SwitchStmt.html">SwitchStmt</a>&gt;</td><td class="name" onclick="toggle('hasInitStatement1')"><a name="hasInitStatement1Anchor">hasInitStatement</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasInitStatement1"><pre>Matches selection statements with initializer.
 
 Given:
- void foo() { 
+ void foo() {
    if (int i = foobar(); i &gt; 0) {}
    switch (int i = foobar(); i) {}
-   for (auto&amp; a = get_range(); auto&amp; x : a) {} 
+   for (auto&amp; a = get_range(); auto&amp; x : a) {}
  }
- void bar() { 
+ void bar() {
    if (foobar() &gt; 0) {}
    switch (foobar()) {}
-   for (auto&amp; x : get_range()) {} 
+   for (auto&amp; x : get_range()) {}
  }
 ifStmt(hasInitStatement(anything()))
   matches the if statement in foo but not in bar.
 switchStmt(hasInitStatement(anything()))
   matches the switch statement in foo but not in bar.
 cxxForRangeStmt(hasInitStatement(anything()))
   matches the range for statement in foo but not in bar.
 </pre></td></tr>

clang/include/clang/ASTMatchers/ASTMatchers.h

 
 /// Matches if all given matchers match.
 ///
 /// Usable as: Any Matcher
 extern const internal::VariadicOperatorMatcherFunc<
     2, std::numeric_limits<unsigned>::max()>
     allOf;
 
+/// Matches any node regardless of the submatchers.
+///
+/// However, \c optionally will generate a result binding for each matching
+/// submatcher.
+///
+/// Useful when additional information which may or may not present about a
+/// main matching node is desired.
+///
+/// For example, in:
+/// \code
+///   class Foo {
+///     int bar;
+///   }
+/// \endcode
+/// The matcher:
+/// \code
+///   cxxRecordDecl(
+///     optionally(has(
+///       fieldDecl(hasName("bar")).bind("var")
+///   ))).bind("record")
+/// \endcode
+/// will produce a result binding for both "record" and "var".
+/// The matcher will produce a "record" binding for even if there is no data
+/// member named "bar" in that class.
+///
+/// Usable as: Any Matcher
+extern const internal::VariadicOperatorMatcherFunc<
+    1, std::numeric_limits<unsigned>::max()>
+    optionally;
+
 /// Matches sizeof (C99), alignof (C++11) and vec_step (OpenCL)
 ///
 /// Given
 /// \code
 ///   Foo x = bar;
 ///   int y = sizeof(x) + alignof(x);
 /// \endcode
 /// unaryExprOrTypeTraitExpr()

clang/include/clang/ASTMatchers/ASTMatchersInternal.h

enum VariadicOperator {
     /// Matches nodes for which at least one of the provided matchers
     /// matches.
     VO_AnyOf,
 
     /// Matches nodes for which at least one of the provided matchers
     /// matches, but doesn't stop at the first match.
     VO_EachOf,
 
+    /// Matches any node but executes all inner matchers to find result
+    /// bindings.
+    VO_Optionally,
+
     /// Matches nodes that do not match the provided matcher.
     ///
     /// Uses the variadic matcher interface, but fails if
     /// InnerMatchers.size() != 1.
     VO_UnaryNot
   };
 
   static DynTypedMatcher

clang/lib/ASTMatchers/ASTMatchersInternal.cpp

bool EachOfVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
                             BoundNodesTreeBuilder *Builder,
                             ArrayRef<DynTypedMatcher> InnerMatchers);
 
 bool AnyOfVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
                            ASTMatchFinder *Finder,
                            BoundNodesTreeBuilder *Builder,
                            ArrayRef<DynTypedMatcher> InnerMatchers);
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+                                ASTMatchFinder *Finder,
+                                BoundNodesTreeBuilder *Builder,
+                                ArrayRef<DynTypedMatcher> InnerMatchers);
+
 void BoundNodesTreeBuilder::visitMatches(Visitor *ResultVisitor) {
   if (Bindings.empty())
     Bindings.push_back(BoundNodesMap());
   for (BoundNodesMap &Binding : Bindings) {
     ResultVisitor->visitMatch(BoundNodes(Binding));
   }
 }
 
return DynTypedMatcher(
         SupportedKind, RestrictKind,
         new VariadicMatcher<AnyOfVariadicOperator>(std::move(InnerMatchers)));
 
   case VO_EachOf:
     return DynTypedMatcher(
         SupportedKind, RestrictKind,
         new VariadicMatcher<EachOfVariadicOperator>(std::move(InnerMatchers)));
 
+  case VO_Optionally:
+    return DynTypedMatcher(SupportedKind, RestrictKind,
+                           new VariadicMatcher<OptionallyVariadicOperator>(
+                               std::move(InnerMatchers)));
+
   case VO_UnaryNot:
     // FIXME: Implement the Not operator to take a single matcher instead of a
     // vector.
     return DynTypedMatcher(
         SupportedKind, RestrictKind,
         new VariadicMatcher<NotUnaryOperator>(std::move(InnerMatchers)));
   }
   llvm_unreachable("Invalid Op value.");
for (const DynTypedMatcher &InnerMatcher : InnerMatchers) {
     if (InnerMatcher.matches(DynNode, Finder, &Result)) {
       *Builder = std::move(Result);
       return true;
     }
   }
   return false;
 }
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+                                ASTMatchFinder *Finder,
+                                BoundNodesTreeBuilder *Builder,
+                                ArrayRef<DynTypedMatcher> InnerMatchers) {
+  BoundNodesTreeBuilder Result;
+  for (const DynTypedMatcher &InnerMatcher : InnerMatchers) {
+    BoundNodesTreeBuilder BuilderInner(*Builder);
+    if (InnerMatcher.matches(DynNode, Finder, &BuilderInner))
+      Result.addMatch(BuilderInner);
+  }
+  *Builder = std::move(Result);
+  return true;
+}
+
 inline static
 std::vector<std::string> vectorFromRefs(ArrayRef<const StringRef *> NameRefs) {
   std::vector<std::string> Names;
   for (auto *Name : NameRefs)
     Names.emplace_back(*Name);
   return Names;
 }
 
const internal::VariadicOperatorMatcherFunc<
     2, std::numeric_limits<unsigned>::max()>
     eachOf = {internal::DynTypedMatcher::VO_EachOf};
 const internal::VariadicOperatorMatcherFunc<
     2, std::numeric_limits<unsigned>::max()>
     anyOf = {internal::DynTypedMatcher::VO_AnyOf};
 const internal::VariadicOperatorMatcherFunc<
     2, std::numeric_limits<unsigned>::max()>
     allOf = {internal::DynTypedMatcher::VO_AllOf};
+const internal::VariadicOperatorMatcherFunc<
+    1, std::numeric_limits<unsigned>::max()>
+    optionally = {internal::DynTypedMatcher::VO_Optionally};
 const internal::VariadicFunction<internal::Matcher<NamedDecl>, StringRef,
                                  internal::hasAnyNameFunc>
     hasAnyName = {};
 const internal::VariadicFunction<internal::Matcher<ObjCMessageExpr>, StringRef,
                                  internal::hasAnySelectorFunc>
     hasAnySelector = {};
 const internal::ArgumentAdaptingMatcherFunc<internal::HasMatcher> has = {};
 const internal::ArgumentAdaptingMatcherFunc<internal::HasDescendantMatcher>

clang/lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasGlobalStorage);
   REGISTER_MATCHER(hasImplicitDestinationType);
   REGISTER_MATCHER(hasInClassInitializer);
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasInitStatement);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
   REGISTER_MATCHER(hasLocalStorage);
   REGISTER_MATCHER(hasLoopInit);
   REGISTER_MATCHER(hasLoopVariable);
   REGISTER_MATCHER(hasMethod);
   REGISTER_MATCHER(hasName);
   REGISTER_MATCHER(hasNullSelector);
RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(objcThrowStmt);
   REGISTER_MATCHER(objcTryStmt);
   REGISTER_MATCHER(ofClass);
   REGISTER_MATCHER(ompDefaultClause);
   REGISTER_MATCHER(ompExecutableDirective);
   REGISTER_MATCHER(on);
   REGISTER_MATCHER(onImplicitObjectArgument);
   REGISTER_MATCHER(opaqueValueExpr);
+  REGISTER_MATCHER(optionally);
   REGISTER_MATCHER(parameterCountIs);
   REGISTER_MATCHER(parenExpr);
   REGISTER_MATCHER(parenListExpr);
   REGISTER_MATCHER(parenType);
   REGISTER_MATCHER(parmVarDecl);
   REGISTER_MATCHER(pointee);
   REGISTER_MATCHER(pointerType);
   REGISTER_MATCHER(predefinedExpr);

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

recordDecl(eachOf(has(fieldDecl(hasName("a")).bind("v")),
                       has(fieldDecl(hasName("b")).bind("v")))),
     std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v", 1)));
   EXPECT_TRUE(notMatches(
     "class A { int c; int d; };",
     recordDecl(eachOf(has(fieldDecl(hasName("a")).bind("v")),
                       has(fieldDecl(hasName("b")).bind("v"))))));
 }
 
+TEST(Optionally, SubmatchersDoNotMatch) {
+  EXPECT_TRUE(matchAndVerifyResultFalse(
+      "class A { int a; int b; };",
+      recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
+                            has(fieldDecl(hasName("d")).bind("v")))),
+      std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v")));
+}
+
+TEST(Optionally, SubmatchersMatch) {
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+      "class A { int a; int c; };",
+      recordDecl(optionally(has(fieldDecl(hasName("a")).bind("v")),
+                            has(fieldDecl(hasName("b")).bind("v")))),
+      std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v", 1)));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+      "class A { int c; int b; };",
+      recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
+                            has(fieldDecl(hasName("b")).bind("v")))),
+      std::make_unique<VerifyIdIsBoundTo<FieldDecl>>("v", 2)));
+}
+
 TEST(IsTemplateInstantiation, MatchesImplicitClassTemplateInstantiation) {
   // Make sure that we can both match the class by name (::X) and by the type
   // the template was instantiated with (via a field).
 
   EXPECT_TRUE(matches(
     "template <typename T> class X {}; class A {}; X<A> x;",
     cxxRecordDecl(hasName("::X"), isTemplateInstantiation())));