[ASTMatcher] Extend hasAnyArgument to ObjCMessageExpr

[george.karpenkov]

Summary

Currently hasArgument works with both ObjC messages and function calls, but not hasAnyArgument.
This patch fixes that discrepancy, as it's often more convenient to use hasAnyArgument.

On a more general note, it would be great to have a common superclass for objc-call and function call, and a matcher matching that, but that's probably a job for another commit.

[aaron.ballman]

You should also regenerate the documentation by running docs\tools\dump_ast_matchers.py.

include/clang/ASTMatchers/ASTMatchers.h

3419 ↗

(On Diff #137237)

I think it would be beneficial to add an example to the documentation.

unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

412 ↗

(On Diff #137237)

I'm not particularly keen on the raw string literal here. We don't use it all that often, but if you have strong feelings in favor of using them, we seem to use R"( rather than R"<<<(.

[george.karpenkov]

include/clang/ASTMatchers/ASTMatchers.h

3419 ↗

(On Diff #137237)

There are tests, there is no example for C++ either, and ObjectiveC examples tend to be fairly verbose.
There is also no example for hasArgument.

unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

412 ↗

(On Diff #137237)

Why not? Having ( forces you to care about brackets inside the string, and in this case it does have brackets.
Forcing everything inside a single line would seriously hamper readability.

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

3414–3415 ↗

(On Diff #137237)

Comment is now out of date.

3419 ↗

(On Diff #137237)

Most AST matcher users are more familiar with C++ than ObjC. Having some example that shows what will and won't be matched is useful for them.

If it's too verbose to make an example, that's a different matter, but it seems like the example would only need to be: [foo bar:12 baz:bing]; level of exposition.

unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

412 ↗

(On Diff #137237)

Some editors still struggle to understand it's a string literal. Nothing requires this to be a single line, as string literal concatenation is perfectly reasonable (and used elsewhere). See TEST(SubstTemplateTypeParmType, HasReplacementType) as an example.

[george.karpenkov]

include/clang/ASTMatchers/ASTMatchers.h

3419 ↗

(On Diff #137237)

OK

unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

412 ↗

(On Diff #137237)

@aaron.ballman OK I'll change it, but in general I don't think we should use an argument "there exist an editor not supporting this feature" to enforce a certain coding style.

[aaron.ballman]

unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

412 ↗

(On Diff #137237)

Fair.

We may want to consider updating the LLVM Coding Standard about use of raw string literals. There's no mention one way or the other and we usually avoid things not explicitly listed as known to be well-supported, but the list of what is supported looks rather out of date. (Not suggesting you should do anything, more just an observation based on my own curiosity with the style guide.)

[george.karpenkov]

@aaron.ballman I think this version is OK?

[aaron.ballman]

LGTM with a small documentation nit (note, the docs will need to be regenerated as well).

include/clang/ASTMatchers/ASTMatchers.h

3429 ↗

(On Diff #137295)

I believe there's a missing semicolon on the message send.

[george.karpenkov]

You should also regenerate the documentation by running docs\tools\dump_ast_matchers.py.

Unfortunately for me it dies with:

Probing http://clang.llvm.org/doxygen/classclang_1_1CUDAKernelCallExpr.html...
*** Unparsable: " } // namespace ast_matchers } // namespace clang " ***

I don't think it's from my changes: with them removed it dies as well.
Should this really be done manually by the programmer? Do we have a buildbot checking for those? Wouldn't it make more sense for the buildbot to upload the documentation directly?
IMO the repo should not contain build byproducts.

unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

412 ↗

(On Diff #137237)

E.g. analyzer code for dumping HTML and CSS looked rather horrendous before raw strings (though that might be also used as an argument to have files in the toolchain rather then in the compiler...) I tend to quite like them for those cases.

cfe/trunk/docs/LibASTMatchersReference.html

with declRefExpr(...)
   matching int y
 and parmVarDecl(...)
   matching int i
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1CXXConstructExpr.html">CXXConstructExpr</a>&gt;</td><td class="name" onclick="toggle('hasAnyArgument1')"><a name="hasAnyArgument1Anchor">hasAnyArgument</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasAnyArgument1"><pre>Matches any argument of a call expression or a constructor call
-expression.
+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)
 with hasAnyArgument(...)
   matching y
+
+For ObjectiveC, given
+  @interface I - (void) f:(int) y; @end
+  void foo(I *i) { [i f:12] }
+objcMessageExpr(hasAnyArgument(integerLiteral(equals(12))))
+  matches [i f:12]
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1CXXConstructExpr.html">CXXConstructExpr</a>&gt;</td><td class="name" onclick="toggle('hasArgument1')"><a name="hasArgument1Anchor">hasArgument</a></td><td>unsigned N, Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasArgument1"><pre>Matches the n'th argument of a call expression or a constructor
 call expression.
 
 Example matches y in x(y)
with declRefExpr(...)
   matching int y
 and parmVarDecl(...)
   matching int i
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1CallExpr.html">CallExpr</a>&gt;</td><td class="name" onclick="toggle('hasAnyArgument0')"><a name="hasAnyArgument0Anchor">hasAnyArgument</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasAnyArgument0"><pre>Matches any argument of a call expression or a constructor call
-expression.
+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)
 with hasAnyArgument(...)
   matching y
+
+For ObjectiveC, given
+  @interface I - (void) f:(int) y; @end
+  void foo(I *i) { [i f:12] }
+objcMessageExpr(hasAnyArgument(integerLiteral(equals(12))))
+  matches [i f:12]
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1CallExpr.html">CallExpr</a>&gt;</td><td class="name" onclick="toggle('hasArgument0')"><a name="hasArgument0Anchor">hasArgument</a></td><td>unsigned N, Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasArgument0"><pre>Matches the n'th argument of a call expression or a constructor
 call expression.
 
 Example matches y in x(y)
Given
   A::B::C c;
 nestedNameSpecifier(specifiesType(
   hasDeclaration(cxxRecordDecl(hasName("A")))
 ))
   matches "A::"
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html">ObjCMessageExpr</a>&gt;</td><td class="name" onclick="toggle('hasAnyArgument2')"><a name="hasAnyArgument2Anchor">hasAnyArgument</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt; InnerMatcher</td></tr>
+<tr><td colspan="4" class="doc" id="hasAnyArgument2"><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)
+with hasAnyArgument(...)
+  matching y
+
+For ObjectiveC, given
+  @interface I - (void) f:(int) y; @end
+  void foo(I *i) { [i f:12] }
+objcMessageExpr(hasAnyArgument(integerLiteral(equals(12))))
+  matches [i f:12]
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html">ObjCMessageExpr</a>&gt;</td><td class="name" onclick="toggle('hasArgument2')"><a name="hasArgument2Anchor">hasArgument</a></td><td>unsigned N, Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasArgument2"><pre>Matches the n'th argument of a call expression or a constructor
 call expression.
 
 Example matches y in x(y)
     (matcher = callExpr(hasArgument(0, declRefExpr())))
   void x(int) { int y; x(y); }
 </pre></td></tr>

cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h

 /// \endcode
 /// cxxConstructorDecl(hasAnyConstructorInitializer(isMemberInitializer()))
 ///   will match D(int), but not match E().
 AST_MATCHER(CXXCtorInitializer, isMemberInitializer) {
   return Node.isMemberInitializer();
 }
 
 /// \brief Matches any argument of a call expression or a constructor call
-/// expression.
+/// expression, or an ObjC-message-send expression.
 ///
 /// Given
 /// \code
 ///   void x(int, int, int) { int y; x(1, y, 42); }
 /// \endcode
 /// callExpr(hasAnyArgument(declRefExpr()))
 ///   matches x(1, y, 42)
 /// with hasAnyArgument(...)
 ///   matching y
+///
+/// For ObjectiveC, given
+/// \code
+///   @interface I - (void) f:(int) y; @end
+///   void foo(I *i) { [i f:12]; }
+/// \endcode
+/// objcMessageExpr(hasAnyArgument(integerLiteral(equals(12))))
+///   matches [i f:12]
 AST_POLYMORPHIC_MATCHER_P(hasAnyArgument,
                           AST_POLYMORPHIC_SUPPORTED_TYPES(CallExpr,
-                                                          CXXConstructExpr),
+                                                          CXXConstructExpr,
+                                                          ObjCMessageExpr),
                           internal::Matcher<Expr>, InnerMatcher) {
   for (const Expr *Arg : Node.arguments()) {
     BoundNodesTreeBuilder Result(*Builder);
     if (InnerMatcher.matches(*Arg, Finder, &Result)) {
       *Builder = std::move(Result);
       return true;
     }
   }

cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

TEST(Matcher, Argument) {
   EXPECT_TRUE(notMatches("void x(int) { int z; x(z); }", CallArgumentY));
 
   StatementMatcher WrongIndex = callExpr(
     hasArgument(42, declRefExpr(to(varDecl(hasName("y"))))));
   EXPECT_TRUE(notMatches("void x(int) { int y; x(y); }", WrongIndex));
 }
 
 TEST(Matcher, AnyArgument) {
-  StatementMatcher CallArgumentY = callExpr(
-    hasAnyArgument(
-      ignoringParenImpCasts(declRefExpr(to(varDecl(hasName("y")))))));
+  auto HasArgumentY = hasAnyArgument(
+      ignoringParenImpCasts(declRefExpr(to(varDecl(hasName("y"))))));
+  StatementMatcher CallArgumentY = callExpr(HasArgumentY);
+  StatementMatcher ObjCCallArgumentY = objcMessageExpr(HasArgumentY);
   EXPECT_TRUE(matches("void x(int, int) { int y; x(1, y); }", CallArgumentY));
   EXPECT_TRUE(matches("void x(int, int) { int y; x(y, 42); }", CallArgumentY));
+  EXPECT_TRUE(matchesObjC("@interface I -(void)f:(int) y; @end "
+                          "void x(I* i) { int y; [i f:y]; }",
+                          ObjCCallArgumentY));
+  EXPECT_FALSE(matchesObjC("@interface I -(void)f:(int) z; @end "
+                           "void x(I* i) { int z; [i f:z]; }",
+                           ObjCCallArgumentY));
   EXPECT_TRUE(notMatches("void x(int, int) { x(1, 2); }", CallArgumentY));
 
   StatementMatcher ImplicitCastedArgument = callExpr(
     hasAnyArgument(implicitCastExpr()));
   EXPECT_TRUE(matches("void x(long) { int y; x(y); }", ImplicitCastedArgument));
 }
 
 TEST(ForEachArgumentWithParam, ReportsNoFalsePositives) {