[PATCH] D72233: Add a new AST matcher 'optionally'.

[Rihan Yang via Phabricator via cfe-commits]

air20 updated this revision to Diff 236510.
air20 added a comment.

Fixed Registry.cpp and documentation as per comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72233/new/

https://reviews.llvm.org/D72233

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp

Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -278,8 +278,8 @@
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
-  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasInitStatement);
+  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
@@ -455,6 +455,7 @@
   REGISTER_MATCHER(on);
   REGISTER_MATCHER(onImplicitObjectArgument);
   REGISTER_MATCHER(opaqueValueExpr);
+  REGISTER_MATCHER(optionally);
   REGISTER_MATCHER(parameterCountIs);
   REGISTER_MATCHER(parenExpr);
   REGISTER_MATCHER(parenListExpr);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2532,7 +2532,27 @@
 /// Matches any node regardless of the submatchers.
 ///
 /// However, \c optionally will generate a result binding for each matching
-/// submatchers.
+/// 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<
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -4689,6 +4689,32 @@
 
 
 
+Matcher<*>optionallyMatcher<*>, ..., Matcher<*>
+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
+
+
+
 MatcherAbstractConditionalOperator>hasConditionMatcherExpr> InnerMatcher
 Matches the condition expression of an if statement, for loop,
 switch statement or conditional operator.
@@ -5098,15 +5124,15 @@
 Matches selection statements with initializer.
 
 Given:
- void foo() { 
+ void foo() {
if (int i = foobar(); i > 0) {}
switch (int i = foobar(); i) {}
-   for (auto& a = get_range(); auto& x : a) {} 
+   for (auto& a = get_range(); auto& x : a) {}
  }
- void bar() { 
+ void bar() {
if (foobar() > 0) {}
switch (foobar()) {}
-   for (auto& x : get_range()) {} 
+   for (auto& x : get_range()) {}
  }
 ifStmt(hasInitStatement(anything()))
   matches the if statement in foo but not in bar.
@@ -6245,15 +6271,15 @@
 Matches selection statements with initializer.
 
 Given:
- void foo() { 
+ void foo() {
if (int i = foobar(); i > 0) {}
switch (int i = foobar(); i) {}
-   for (auto& a = get_range(); auto& x : a) {} 
+   for (auto& a = get_range(); auto& x : a) {}
  }
- void bar() { 
+ void bar() {
if (foobar() > 0) {}
switch (foobar()) {}
-   for (auto& x : get_range()) {} 
+   for (auto& x : get_range()) {}
  }
 ifStmt(hasInitStatement(anything()))
   matches the if statement in foo but not in bar.
@@ -7005,15 +7031,15 @@
 Matches selection statements with initializer.
 
 Given:
- void foo() { 
+ void foo() {
if (int i = foobar(); i > 0) {}
switch (int i = foobar(); i) {}
-   for (auto& a = get_range(); auto& x : a) {} 
+   for (auto& a = get_range(); auto& x : a) {}
  }
- void bar() { 
+ void bar() {
if (foobar() > 0) {}
switch (foobar()) {}
-   for (auto& x : get_range()) {} 
+   for (auto& x : get_range()) {}
  }
 ifStmt(hasInitStatement(an

[PATCH] D72233: Add a new AST matcher 'optionally'.

[Rihan Yang via Phabricator via cfe-commits]

air20 updated this revision to Diff 236513.
air20 added a comment.

Included base commits that was missing in the previous diff.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72233/new/

https://reviews.llvm.org/D72233

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1866,6 +1866,27 @@
   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>("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>("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>("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).
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -278,8 +278,8 @@
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
-  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasInitStatement);
+  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
@@ -455,6 +455,7 @@
   REGISTER_MATCHER(on);
   REGISTER_MATCHER(onImplicitObjectArgument);
   REGISTER_MATCHER(opaqueValueExpr);
+  REGISTER_MATCHER(optionally);
   REGISTER_MATCHER(parameterCountIs);
   REGISTER_MATCHER(parenExpr);
   REGISTER_MATCHER(parenListExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -68,6 +68,11 @@
BoundNodesTreeBuilder *Builder,
ArrayRef InnerMatchers);
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+ASTMatchFinder *Finder,
+BoundNodesTreeBuilder *Builder,
+ArrayRef InnerMatchers);
+
 void BoundNodesTreeBuilder::visitMatches(Visitor *ResultVisitor) {
   if (Bindings.empty())
 Bindings.push_back(BoundNodesMap());
@@ -179,6 +184,11 @@
 SupportedKind, RestrictKind,
 new VariadicMatcher(std::move(InnerMatchers)));
 
+  case VO_Optionally:
+return DynTypedMatcher(SupportedKind, RestrictKind,
+   new VariadicMatcher(
+   std::move(InnerMatchers)));
+
   case VO_UnaryNot:
 // FIXME: Implement the Not operator to take a single matcher instead of a
 // vector.
@@ -342,6 +352,21 @@
   return false;
 }
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+ASTMatchFinder *Finder,
+BoundNodesTreeBuilder *Builder,
+ArrayRef 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 vectorFromRefs(ArrayRef NameRefs) {
   std::vector Names;
@@ -792,6 +817,9 @@
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 allOf = {internal::DynTypedMatcher::VO_AllOf};
+const internal::VariadicOperatorMatcherFunc<
+1, std::numeric_limits::max()>
+optionally = {internal::DynTypedMatcher::VO_Optionally};
 const internal::VariadicFunction, StringRef,

[PATCH] D72233: Add a new AST matcher 'optionally'.

[Rihan Yang via Phabricator via cfe-commits]

air20 updated this revision to Diff 236514.
air20 marked an inline comment as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72233/new/

https://reviews.llvm.org/D72233

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1866,6 +1866,27 @@
   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>("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>("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>("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).
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -278,8 +278,8 @@
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
-  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasInitStatement);
+  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
@@ -455,6 +455,7 @@
   REGISTER_MATCHER(on);
   REGISTER_MATCHER(onImplicitObjectArgument);
   REGISTER_MATCHER(opaqueValueExpr);
+  REGISTER_MATCHER(optionally);
   REGISTER_MATCHER(parameterCountIs);
   REGISTER_MATCHER(parenExpr);
   REGISTER_MATCHER(parenListExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -68,6 +68,11 @@
BoundNodesTreeBuilder *Builder,
ArrayRef InnerMatchers);
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+ASTMatchFinder *Finder,
+BoundNodesTreeBuilder *Builder,
+ArrayRef InnerMatchers);
+
 void BoundNodesTreeBuilder::visitMatches(Visitor *ResultVisitor) {
   if (Bindings.empty())
 Bindings.push_back(BoundNodesMap());
@@ -179,6 +184,11 @@
 SupportedKind, RestrictKind,
 new VariadicMatcher(std::move(InnerMatchers)));
 
+  case VO_Optionally:
+return DynTypedMatcher(SupportedKind, RestrictKind,
+   new VariadicMatcher(
+   std::move(InnerMatchers)));
+
   case VO_UnaryNot:
 // FIXME: Implement the Not operator to take a single matcher instead of a
 // vector.
@@ -342,6 +352,20 @@
   return false;
 }
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+ASTMatchFinder *Finder,
+BoundNodesTreeBuilder *Builder,
+ArrayRef 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 vectorFromRefs(ArrayRef NameRefs) {
   std::vector Names;
@@ -792,6 +816,9 @@
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 allOf = {internal::DynTypedMatcher::VO_AllOf};
+const internal::VariadicOperatorMatcherFunc<
+1, std::numeric_limits::max()>
+optionally = {internal::DynTypedMatcher::VO_Optionally};
 const internal::VariadicFunction, StringRef,
  internal::hasAnyNameFunc>

[PATCH] D72233: Add a new AST matcher 'optionally'.

[Rihan Yang via Phabricator via cfe-commits]

air20 marked an inline comment as done.
air20 added a comment.

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?




Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:282
   REGISTER_MATCHER(hasInitStatement);
+  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);

aaron.ballman wrote:
> Why did this one move? Please keep the list sorted alphabetically.
I think this wasn’t sorted correctly before.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72233/new/

https://reviews.llvm.org/D72233



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72233: Add a new AST matcher 'optionally'.

[Rihan Yang via Phabricator via cfe-commits]

air20 marked an inline comment as done.
air20 added a comment.

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




Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:282
   REGISTER_MATCHER(hasInitStatement);
+  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);

aaron.ballman wrote:
> air20 wrote:
> > aaron.ballman wrote:
> > > Why did this one move? Please keep the list sorted alphabetically.
> > I think this wasn’t sorted correctly before.
> 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.
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72233/new/

https://reviews.llvm.org/D72233



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72233: Add a new AST matcher 'optionally'.

[Rihan Yang via Phabricator via cfe-commits]

air20 updated this revision to Diff 236866.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72233/new/

https://reviews.llvm.org/D72233

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1866,6 +1866,27 @@
   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>("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>("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>("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).
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -455,6 +455,7 @@
   REGISTER_MATCHER(on);
   REGISTER_MATCHER(onImplicitObjectArgument);
   REGISTER_MATCHER(opaqueValueExpr);
+  REGISTER_MATCHER(optionally);
   REGISTER_MATCHER(parameterCountIs);
   REGISTER_MATCHER(parenExpr);
   REGISTER_MATCHER(parenListExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -68,6 +68,11 @@
BoundNodesTreeBuilder *Builder,
ArrayRef InnerMatchers);
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+ASTMatchFinder *Finder,
+BoundNodesTreeBuilder *Builder,
+ArrayRef InnerMatchers);
+
 void BoundNodesTreeBuilder::visitMatches(Visitor *ResultVisitor) {
   if (Bindings.empty())
 Bindings.push_back(BoundNodesMap());
@@ -179,6 +184,11 @@
 SupportedKind, RestrictKind,
 new VariadicMatcher(std::move(InnerMatchers)));
 
+  case VO_Optionally:
+return DynTypedMatcher(SupportedKind, RestrictKind,
+   new VariadicMatcher(
+   std::move(InnerMatchers)));
+
   case VO_UnaryNot:
 // FIXME: Implement the Not operator to take a single matcher instead of a
 // vector.
@@ -342,6 +352,20 @@
   return false;
 }
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+ASTMatchFinder *Finder,
+BoundNodesTreeBuilder *Builder,
+ArrayRef 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 vectorFromRefs(ArrayRef NameRefs) {
   std::vector Names;
@@ -792,6 +816,9 @@
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 allOf = {internal::DynTypedMatcher::VO_AllOf};
+const internal::VariadicOperatorMatcherFunc<
+1, std::numeric_limits::max()>
+optionally = {internal::DynTypedMatcher::VO_Optionally};
 const internal::VariadicFunction, StringRef,
  internal::hasAnyNameFunc>
 hasAnyName = {};
Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -363,6 +363,10 @@
 /// matches, but doesn't stop at the first match.
 VO_EachOf,
 
+/// Matches a

[PATCH] D72233: Add a new AST matcher 'optionally'.

[Rihan Yang via Phabricator via cfe-commits]

air20 marked an inline comment as done.
air20 added inline comments.



Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:282
   REGISTER_MATCHER(hasInitStatement);
+  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);

aaron.ballman wrote:
> air20 wrote:
> > aaron.ballman wrote:
> > > air20 wrote:
> > > > aaron.ballman wrote:
> > > > > Why did this one move? Please keep the list sorted alphabetically.
> > > > I think this wasn’t sorted correctly before.
> > > 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.
> > 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.
> > 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.
 Reverted. But I thought the purpose of sorting is to make it look better and 
maintain *one* canonical order. Not a big deal anyhow.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72233/new/

https://reviews.llvm.org/D72233



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits