[ASTMatchers] Extend hasParameter and hasAnyParameter matches to handle Objective-C methods

[aaron.ballman]

You should also regenerate the AST matcher documentation by running docs/tools/dump_ast_matchers.py

include/clang/ASTMatchers/ASTMatchers.h

3469–3470

Please add an ObjC example below.

3561

Same here.

[george.karpenkov]

@aaron.ballman OK.

BTW do you know how to fix the following:

clang-query> match anyOf( objcMethodDecl(), functionDecl() )
Input value has unresolved overloaded type: Matcher<Decl>&Matcher<Decl>

The underlying problem is that the only base class they have in common is Decl (though I'm not sure why is it a problem?..)

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

3487

This statement isn't quite correct -- it matches the objcMethodDecl() with hasAnyArgument(...) matching int y.

However, why is this calling hasAnyArgument instead of hasParameter?

3561

Should add a code example for hasAnyParameter.

unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

574

You should also have a test for hasParameter().

[george.karpenkov]

include/clang/ASTMatchers/ASTMatchers.h

3469–3470

Decided to revert this block, getting it right is a bit tricky, and I do not need that right now.

3496

Yes, this is ugly, and I do plan to eventually introduce a common superclass, so we could enforce same name for same operations.

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

3496

Both classes have a parameters() member function that returns an ArrayRef<ParmVarDecl *>, so I think this could be factored to use that function in both cases and have common count checking.

lib/Analysis/BodyFarm.cpp

317

This change looks unrelated to the patch.

[george.karpenkov]

include/clang/ASTMatchers/ASTMatchers.h

3496

👏 Thanks! Somehow completely missed that.

3504

For now don't know how to avoid this ugliness without a huge refactoring...

[aaron.ballman]

LGTM with a minor doc nit.

docs/LibASTMatchersReference.html

5336–5337

The doxygen use here seems to be causing fits for the generated docs. You can drop its use in these cases.

[george.karpenkov]

@aaron.ballman turns out larger changes for dynamic tests were required, could you take a brief look?

[george.karpenkov]

unittests/ASTMatchers/Dynamic/ParserTest.cpp

189 ↗

(On Diff #139383)

the problem here is that it's not a dyn_typed_matcher anymore.
There's a seemingly related code in Marshallers.h constructing VariantMatcher::PolymorphicMatcher out of what seems to be a polymorphic matcher, but it's not accessible from here.

336 ↗

(On Diff #139383)

seems that the order is not actually fixed

[aaron.ballman]

Adding some folks to review the dynamic matcher bits.

include/clang/StaticAnalyzer/Core/PathSensitive/WorkList.h

90 ↗

(On Diff #139383)

This change looks unrelated (but would be fine to commit on its own without waiting for review).

unittests/ASTMatchers/Dynamic/ParserTest.cpp

189 ↗

(On Diff #139383)

I'm not as familiar with the dynamic matcher infrastructure as I am other parts of the matchers; can you give a bit more background on what's happening here?

[george.karpenkov]

include/clang/StaticAnalyzer/Core/PathSensitive/WorkList.h

90 ↗

(On Diff #139383)

yeah somehow i always screw up the exact thing I am meant to sent to the diff, thanks!

unittests/ASTMatchers/Dynamic/ParserTest.cpp

189 ↗

(On Diff #139383)

@aaron.ballman OK.
Dynamic matcher kicks in clang-query where the query is typed as a string, and then libASTMatcher has to parse it itself, instead of relying on crazy C++ templates.
In this case, hasParameter is no longer a dyn_typed_matcher, so SingleMatcher can not be constructed out of it.
The construction is used to store matchers in a kind of "registry" which is then tested (for subsequent queries and autocompletion).
The change itself is pretty trivial.

[aaron.ballman]

unittests/ASTMatchers/Dynamic/ParserTest.cpp

189 ↗

(On Diff #139383)

Hmm, so will this break behavior for clang-query users where their use of hasParameter() is now ambiguous? Or are these changes only needed because we're not really being fully dynamic in the unit tests like we are with clang-query directly?

[george.karpenkov]

unittests/ASTMatchers/Dynamic/ParserTest.cpp

189 ↗

(On Diff #139383)

From what I have tried, hasParameter works in clang-query as before (just as hasArgument does).
I think the problem is in unit tests, as they try to save this object to a type which does not (directly) support that.

[aaron.ballman]

include/clang/StaticAnalyzer/Core/PathSensitive/WorkList.h

90 ↗

(On Diff #139383)

It happens to all of us. :-)

unittests/ASTMatchers/Dynamic/ParserTest.cpp

189 ↗

(On Diff #139383)

Okay, that makes sense to me. Then this continues to LGTM, thank you for double-checking!

[george.karpenkov]

unittests/ASTMatchers/Dynamic/ParserTest.cpp

189 ↗

(On Diff #139383)

From what I have tried, hasParameter works in clang-query as before (just as hasArgument does).

@aaron.ballman
Actually sorry, this was incorrect. Yes, it is breaking:

clang-query> match hasParameter(0, parmVarDecl())
Input value has unresolved overloaded type: Matcher<FunctionDecl|ObjCMethodDecl>

The previous behavior can easily obtained though:

clang-query> match functionDecl(hasParameter(0, parmVarDecl()))

Match #1:
...

[george.karpenkov]

unittests/ASTMatchers/Dynamic/ParserTest.cpp

189 ↗

(On Diff #139383)

The lack of a reasonable common supertype for functionDecl and objcMethodDecl is very annoying.

Strangely, this does not work:

clang-query> match decl(hasParameter(0, parmVarlDec))
1:2: Error building matcher decl.
1:7: Incorrect type for arg 1. (Expected = Matcher<Decl>) != (Actual = Matcher<FunctionDecl|ObjCMethodDecl>)

So we have to go through the hoops:

clang-query> match decl(anyOf(objcMethodDecl(hasParameter(0, parmVarDecl())), functionDecl(hasParameter(0, parmVarDecl()))))

[aaron.ballman]

unittests/ASTMatchers/Dynamic/ParserTest.cpp

189 ↗

(On Diff #139383)

Ah, that's what I was afraid of. However, because this is a traversal matcher, I have a hard time imagining that anyone uses the matcher in this way because there would be no current node to traverse *from* in a top-level hasParameter() match.

I think the behavior change will be reasonable. What do you think, @sbenza and @klimek?

[george.karpenkov]

unittests/ASTMatchers/Dynamic/ParserTest.cpp

189 ↗

(On Diff #139383)

I'm also sort of puzzled why the typechecker can not figure out a common superclass. Maybe just in order to be consistent with static behavior.

[george.karpenkov]

@alexfh This patch looks reasonable to you?

[alexfh]

In D44707#1048629, @george.karpenkov wrote:

@alexfh This patch looks reasonable to you?

Yep, no concerns from me.

docs/LibASTMatchersReference.html

   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] }
+  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.

   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] }
+  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:
     forStmt(hasLoopInit(declStmt()))
 matches 'int x = 0' in
     for (int x = 0; x &lt; N; ++x) { }
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html">FunctionDecl</a>&gt;</td><td class="name" onclick="toggle('hasAnyParameter0')"><a name="hasAnyParameter0Anchor">hasAnyParameter</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ParmVarDecl.html">ParmVarDecl</a>&gt; InnerMatcher</td></tr>
-<tr><td colspan="4" class="doc" id="hasAnyParameter0"><pre>Matches any parameter of a function declaration.
+<tr><td colspan="4" class="doc" id="hasAnyParameter0"><pre>Matches any parameter of a function or ObjC method declaration.
 
 Does not match the 'this' parameter of a method.
 
 Given
   class X { void f(int x, int y, int z) {} };
 cxxMethodDecl(hasAnyParameter(hasName("y")))
   matches f(int x, int y, int z) {}
 with hasAnyParameter(...)

 hasBody(compoundStmt())
   matches 'for (;;) {}'
 with compoundStmt()
   matching '{}'
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html">FunctionDecl</a>&gt;</td><td class="name" onclick="toggle('hasParameter0')"><a name="hasParameter0Anchor">hasParameter</a></td><td>unsigned N, Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ParmVarDecl.html">ParmVarDecl</a>&gt; InnerMatcher</td></tr>
-<tr><td colspan="4" class="doc" id="hasParameter0"><pre>Matches the n'th parameter of a function declaration.
+<tr><td colspan="4" class="doc" id="hasParameter0"><pre>Matches the n'th parameter of a function or ObjC method declaration.
 
 Given
   class X { void f(int x) {} };
 cxxMethodDecl(hasParameter(0, hasType(varDecl())))
   matches f(int x) {}
 with hasParameter(...)
   matching int x
+
+For ObjectiveC, given
+  @interface I - (void) f:(int) y; @end
+
+the matcher {@code objcMethodDecl(hasAnyArgument(hasName("y")))}
+matches the declaration of parameter {@code y}.
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html">FunctionDecl</a>&gt;</td><td class="name" onclick="toggle('hasTemplateArgument2')"><a name="hasTemplateArgument2Anchor">hasTemplateArgument</a></td><td>unsigned N, Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1TemplateArgument.html">TemplateArgument</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="hasTemplateArgument2"><pre>Matches classTemplateSpecializations, templateSpecializationType and
 functionDecl where the n'th TemplateArgument matches the given InnerMatcher.
 
 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] }
+  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.

 matcher = objCMessageExpr(hasReceiverType(asString("UIWebView *")));
 matches the [webView ...] message invocation.
   NSString *webViewJavaScript = ...
   UIWebView *webView = ...
   [webView stringByEvaluatingJavaScriptFromString:webViewJavascript];
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html">ObjCMethodDecl</a>&gt;</td><td class="name" onclick="toggle('hasAnyParameter1')"><a name="hasAnyParameter1Anchor">hasAnyParameter</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ParmVarDecl.html">ParmVarDecl</a>&gt; InnerMatcher</td></tr>
+<tr><td colspan="4" class="doc" id="hasAnyParameter1"><pre>Matches any parameter of a function or ObjC method declaration.
+
+Does not match the 'this' parameter of a method.
+
+Given
+  class X { void f(int x, int y, int z) {} };
+cxxMethodDecl(hasAnyParameter(hasName("y")))
+  matches f(int x, int y, int z) {}
+with hasAnyParameter(...)
+  matching int y
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1OpaqueValueExpr.html">OpaqueValueExpr</a>&gt;</td><td class="name" onclick="toggle('hasSourceExpression1')"><a name="hasSourceExpression1Anchor">hasSourceExpression</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="hasSourceExpression1"><pre>Matches if the cast's source expression
 or opaque value's source expression matches the given matcher.
 
 Example 1: matches "a string"
 (matcher = castExpr(hasSourceExpression(cxxConstructExpr())))
 class URL { URL(string); };
 URL url = "a string";

include/clang/ASTMatchers/ASTMatchers.h

 /// }
 /// \endcode
 /// initListExpr(has(cxxConstructExpr(requiresZeroInitialization()))
 /// will match the implicit array filler for pt[1].
 AST_MATCHER(CXXConstructExpr, requiresZeroInitialization) {
   return Node.requiresZeroInitialization();
 }
 
-/// \brief Matches the n'th parameter of a function declaration.
+/// \brief Matches the n'th parameter of a function or an ObjC method
+/// declaration.
 ///
 /// Given
 /// \code
 ///   class X { void f(int x) {} };
 /// \endcode
 /// cxxMethodDecl(hasParameter(0, hasType(varDecl())))
 ///   matches f(int x) {}
 /// with hasParameter(...)
 ///   matching int x
-AST_MATCHER_P2(FunctionDecl, hasParameter,
-               unsigned, N, internal::Matcher<ParmVarDecl>,
-               InnerMatcher) {
-  return (N < Node.getNumParams() &&
-          InnerMatcher.matches(
-              *Node.getParamDecl(N), Finder, Builder));
+///
+/// For ObjectiveC, given
+/// \code
+///   @interface I - (void) f:(int) y; @end
+/// \endcode
+//
+/// the matcher {@code objcMethodDecl(hasParameter(0, hasName("y")))}
+/// matches the declaration of method {@code f} with {@code hasParameter}
+/// matching {@code y}.
+AST_POLYMORPHIC_MATCHER_P2(hasParameter,
+                           AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+                                                           ObjCMethodDecl),
+                           unsigned, N, internal::Matcher<ParmVarDecl>,
+                           InnerMatcher) {
+  unsigned NumParams;
+  const ParmVarDecl* Decl;
+  if (const auto *FD = dyn_cast<FunctionDecl>(&Node)) {
+    NumParams = FD->getNumParams();
+    Decl = FD->getParamDecl(N);
+  } else {
+    const auto *OM = dyn_cast<ObjCMethodDecl>(&Node);
+    NumParams = OM->param_size();
+    Decl = OM->parameters()[N];
+  }
+  return (N < NumParams && InnerMatcher.matches(*Decl, Finder, Builder));
 }
 
 /// \brief Matches all arguments and their respective ParmVarDecl.
 ///
 /// Given
 /// \code
 ///   void f(int i);
 ///   int y;

       }
     }
     ++ParamIndex;
   }
   *Builder = std::move(Result);
   return Matched;
 }
 
-/// \brief Matches any parameter of a function declaration.
+/// \brief Matches any parameter of a function or ObjC method declaration.
 ///
 /// Does not match the 'this' parameter of a method.
 ///
 /// Given
 /// \code
 ///   class X { void f(int x, int y, int z) {} };
 /// \endcode
 /// cxxMethodDecl(hasAnyParameter(hasName("y")))
 ///   matches f(int x, int y, int z) {}
 /// with hasAnyParameter(...)
 ///   matching int y
-AST_MATCHER_P(FunctionDecl, hasAnyParameter,
-              internal::Matcher<ParmVarDecl>, InnerMatcher) {
+///
+/// For ObjectiveC, given
+/// \code
+///   @interface I - (void) f:(int) y; @end
+/// \endcode
+//
+/// the matcher {@code objcMethodDecl(hasAnyParameter(hasName("y")))}
+/// matches the declaration of method {@code f} with {@code hasParameter}
+/// matching {@code y}.
+AST_POLYMORPHIC_MATCHER_P(hasAnyParameter,
+                          AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+                                                          ObjCMethodDecl),
+                          internal::Matcher<ParmVarDecl>,
+                          InnerMatcher) {
   return matchesFirstInPointerRange(InnerMatcher, Node.param_begin(),
                                     Node.param_end(), Finder, Builder);
 }
 
 /// \brief Matches \c FunctionDecls and \c FunctionProtoTypes that have a
 /// specific parameter count.
 ///
 /// Given

lib/Analysis/BodyFarm.cpp

 /// void call_once(once_flag& o, Callable func) {
 ///   if (!o.__state) {
 ///     func();
 ///   }
 ///   o.__state = 1;
 /// }
 /// \endcode
 static Stmt *create_call_once(ASTContext &C, const FunctionDecl *D) {
+  
   DEBUG(llvm::dbgs() << "Generating body for call_once\n");
 
   // We need at least two parameters.
   if (D->param_size() < 2)
     return nullptr;
 
   ASTMaker M(C);
 

unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

matches("typedef int &int_ref;"
             varDecl(hasType(qualType(hasCanonicalType(referenceType()))))));
 }
 
 TEST(HasParameter, CallsInnerMatcher) {
   EXPECT_TRUE(matches("class X { void x(int) {} };",
                       cxxMethodDecl(hasParameter(0, varDecl()))));
   EXPECT_TRUE(notMatches("class X { void x(int) {} };",
                          cxxMethodDecl(hasParameter(0, hasName("x")))));
+  EXPECT_TRUE(matches("@interface I -(void)f:(int) x; @end",
+                      objcMethodDecl(hasParameter(0, hasName("x")))));
 }
 
 TEST(HasParameter, DoesNotMatchIfIndexOutOfBounds) {
   EXPECT_TRUE(notMatches("class X { void x(int) {} };",
                          cxxMethodDecl(hasParameter(42, varDecl()))));
 }
 
 TEST(HasType, MatchesParameterVariableTypesStrictly) {

 
 TEST(HasAnyParameter, MatchesIndependentlyOfPosition) {
   EXPECT_TRUE(matches(
     "class Y {}; class X { void x(X x, Y y) {} };",
     cxxMethodDecl(hasAnyParameter(hasType(recordDecl(hasName("X")))))));
   EXPECT_TRUE(matches(
     "class Y {}; class X { void x(Y y, X x) {} };",
     cxxMethodDecl(hasAnyParameter(hasType(recordDecl(hasName("X")))))));
+  EXPECT_TRUE(matches("@interface I -(void)f:(int) x; @end",
+                      objcMethodDecl(hasAnyParameter(hasName("x")))));
 }
 
 TEST(Returns, MatchesReturnTypes) {
   EXPECT_TRUE(matches("class Y { int f() { return 1; } };",
                       functionDecl(returns(asString("int")))));
   EXPECT_TRUE(notMatches("class Y { int f() { return 1; } };",
                          functionDecl(returns(asString("float")))));
   EXPECT_TRUE(matches("class Y { Y getMe() { return *this; } };",