[ASTMatchers] Add ObjC matchers for fundamental decls

[kastiglione]

Summary

This adds matchers for ObjCProtocolDecl, ObjCCategoryDecl, ObjCMethodDecl, ObjCIvarDecl, and
ObjCPropertyDecl. These matchers complement the existing matcher for ObjCInterfaceDecl.

[kastiglione]

I'd like to add more ObjC specific matcher functionality in follow up diffs. This diff is to to start somewhere, and get feedback.

[malcolm.parsons]

LGTM!

[kastiglione]

Thanks @malcolm.parsons. I don't have commit access, will you be able to commit this?

[aaron.ballman]

unittests/ASTMatchers/ASTMatchersNodeTest.cpp

1501

These changes are unrelated and should be in a separate commit.

1546

Instead of using a pragma for this, I think it would make more sense to just modify matchesObjC() to disable the diagnostic. This is only intended to test the dynamic AST matchers, so the diagnostics are not useful in that case anyway.

unittests/ASTMatchers/ASTMatchersTest.h

134–137

Can you explain why this change is required?

[kastiglione]

unittests/ASTMatchers/ASTMatchersNodeTest.cpp

1501

They're related to the use of either -fobjc-runtime= or fobjc-nonfragile-abi.

1546

matchesConditionally() accepts only one compiler arg, so putting the diagnostics here was a smaller change than refactoring that function. Do you think it would be better to refactor matchesConditionally()?

unittests/ASTMatchers/ASTMatchersTest.h

134–137

Code was not being evaluated as Objective-C 2, which resulted in warnings and errors for the test this diff introduces. Setting the runtime was the first approach I tried, and it worked so I went with it without looking into why it was necessary. Now that you've asked, I stepped through and found that the i386-unknown-unknown triple is resulting in the use of an ELF toolchain and GCC objc runtime.

It can be changed to -fobjc-nonfragile-abi, which seems better than a specific runtime, do you agree? Is there any reason to not have Objective-C 2 be the default?

[kastiglione]

unittests/ASTMatchers/ASTMatchersNodeTest.cpp

1546

I notice that many other tests have warnings. Should these tests just allow the warnings to be emitted?

[kastiglione]

Use -fobjc-nonfragile-abi

[aaron.ballman]

unittests/ASTMatchers/ASTMatchersNodeTest.cpp

1546

We generally let the warnings go -- it's not harmful to have them. However, if this is a warning that's likely to trigger on most tests, there's no harm in suppressing it either.

unittests/ASTMatchers/ASTMatchersTest.h

134–137

I think -fobjc-nonfragile-abi may be fine, but I guess I'm surprised that ObjC1 didn't require any specific runtime and ObjC2 requires one or else you get errors (warnings are fine, however -- we have plenty of those in these tests).

Perhaps it's time to fix the FIXME in matchesConditionally() so that we don't need to specify the triple at all, and then you won't need to specify the runtime? I don't think that should hold up this patch, however.

[kastiglione]

unittests/ASTMatchers/ASTMatchersNodeTest.cpp

1546

Sounds good, I'm for suppressing them. Should I refactor matchesConditionally() to allow multiple compile args, and disable these warnings from matchesObjC()?

unittests/ASTMatchers/ASTMatchersTest.h

134–137

I'm surprised that ObjC1 didn't require any specific runtime and ObjC2 requires one or else you get errors

I think this is because the existing ObjC in this test was small in size and coverage of syntax/features. Given the variable name Objc1String, it was probably written to avoid ObjC2 specific abi/features.

[aaron.ballman]

unittests/ASTMatchers/ASTMatchersNodeTest.cpp

1546

Yes, I think that's the way to go.

unittests/ASTMatchers/ASTMatchersTest.h

134–137

Fair enough

[kastiglione]

Overload matchesConditionally() for multiple compiler args

[aaron.ballman]

unittests/ASTMatchers/ASTMatchersTest.h

64

I think this might be better as an llvm::ArrayRef<llvm::StringRef>.

114

This could then use makeArrayRef(CompileArg).

[kastiglione]

unittests/ASTMatchers/ASTMatchersTest.h

64

Ok. It needs to be a std::vector<std::string> for runToolOnCodeWithArgs(). I don't see any built in way to do that conversion, so this function will have to manually do that I guess?

[aaron.ballman]

unittests/ASTMatchers/ASTMatchersTest.h

64

Can you do:

std::vector<std::string> LocalArgs(Args.begin(), Args.end());
LocalArgs.insert(LocalArgs.end(), {"everything else"});

below?

[kastiglione]

Use ArrayRef<StringRef>

unittests/ASTMatchers/ASTMatchersTest.h

64

right, thanks!

[aaron.ballman]

LGTM! I'll go ahead and commit for you.

unittests/ASTMatchers/ASTMatchersTest.h

113

The formatting of this line is incorrect, but I can fix it when I commit.

[aaron.ballman]

I've commit in r297882, thanks!

[kastiglione]

@aaron.ballman @malcolm.parsons Thanks for the reviews and the commit. I plan to do follow ups for more ObjC ASTMatcher support.

docs/LibASTMatchersReference.html

 
 Given
   template <typename T, int N> struct C {};
 nonTypeTemplateParmDecl()
   matches 'N', but not 'T'.
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>&gt;</td><td class="name" onclick="toggle('objcCategoryDecl0')"><a name="objcCategoryDecl0Anchor">objcCategoryDecl</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ObjCCategoryDecl.html">ObjCCategoryDecl</a>&gt;...</td></tr>
+<tr><td colspan="4" class="doc" id="objcCategoryDecl0"><pre>Matches Objective-C category declarations.
+
+Example matches Foo (Additions)
+  @interface Foo (Additions)
+  @end
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>&gt;</td><td class="name" onclick="toggle('objcInterfaceDecl0')"><a name="objcInterfaceDecl0Anchor">objcInterfaceDecl</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ObjCInterfaceDecl.html">ObjCInterfaceDecl</a>&gt;...</td></tr>
 <tr><td colspan="4" class="doc" id="objcInterfaceDecl0"><pre>Matches Objective-C interface declarations.
 
 Example matches Foo
   @interface Foo
   @end
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>&gt;</td><td class="name" onclick="toggle('objcIvarDecl0')"><a name="objcIvarDecl0Anchor">objcIvarDecl</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ObjCIvarDecl.html">ObjCIvarDecl</a>&gt;...</td></tr>
+<tr><td colspan="4" class="doc" id="objcIvarDecl0"><pre>Matches Objective-C instance variable declarations.
+
+Example matches _enabled
+  @implementation Foo {
+    BOOL _enabled;
+  }
+  @end
+</pre></td></tr>
+
+
+<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>&gt;</td><td class="name" onclick="toggle('objcMethodDecl0')"><a name="objcMethodDecl0Anchor">objcMethodDecl</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ObjCMethodDecl.html">ObjCMethodDecl</a>&gt;...</td></tr>
+<tr><td colspan="4" class="doc" id="objcMethodDecl0"><pre>Matches Objective-C method declarations.
+
+Example matches both declaration and definition of -[Foo method]
+  @interface Foo
+  - (void)method;
+  @end
+
+  @implementation Foo
+  - (void)method {}
+  @end
+</pre></td></tr>
+
+
+<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>&gt;</td><td class="name" onclick="toggle('objcPropertyDecl0')"><a name="objcPropertyDecl0Anchor">objcPropertyDecl</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ObjCPropertyDecl.html">ObjCPropertyDecl</a>&gt;...</td></tr>
+<tr><td colspan="4" class="doc" id="objcPropertyDecl0"><pre>Matches Objective-C property declarations.
+
+Example matches enabled
+  @interface Foo
+  @property BOOL enabled;
+  @end
+</pre></td></tr>
+
+
+<tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>&gt;</td><td class="name" onclick="toggle('objcProtocolDecl0')"><a name="objcProtocolDecl0Anchor">objcProtocolDecl</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ObjCProtocolDecl.html">ObjCProtocolDecl</a>&gt;...</td></tr>
+<tr><td colspan="4" class="doc" id="objcProtocolDecl0"><pre>Matches Objective-C protocol declarations.
+
+Example matches FooDelegate
+  @protocol FooDelegate
+  @end
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>&gt;</td><td class="name" onclick="toggle('parmVarDecl0')"><a name="parmVarDecl0Anchor">parmVarDecl</a></td><td>Matcher&lt;<a href="http://clang.llvm.org/doxygen/classclang_1_1ParmVarDecl.html">ParmVarDecl</a>&gt;...</td></tr>
 <tr><td colspan="4" class="doc" id="parmVarDecl0"><pre>Matches parameter variable declarations.
 
 Given
   void f(int x);
 parmVarDecl()
   matches int x.
 </pre></td></tr>

include/clang/ASTMatchers/ASTMatchers.h

 /// \code
 ///   @interface Foo
 ///   @end
 /// \endcode
 const internal::VariadicDynCastAllOfMatcher<
   Decl,
   ObjCInterfaceDecl> objcInterfaceDecl;
 
+/// \brief Matches Objective-C protocol declarations.
+///
+/// Example matches FooDelegate
+/// \code
+///   @protocol FooDelegate
+///   @end
+/// \endcode
+const internal::VariadicDynCastAllOfMatcher<
+  Decl,
+  ObjCProtocolDecl> objcProtocolDecl;
+
+/// \brief Matches Objective-C category declarations.
+///
+/// Example matches Foo (Additions)
+/// \code
+///   @interface Foo (Additions)
+///   @end
+/// \endcode
+const internal::VariadicDynCastAllOfMatcher<
+  Decl,
+  ObjCCategoryDecl> objcCategoryDecl;
+
+/// \brief Matches Objective-C method declarations.
+///
+/// Example matches both declaration and definition of -[Foo method]
+/// \code
+///   @interface Foo
+///   - (void)method;
+///   @end
+///
+///   @implementation Foo
+///   - (void)method {}
+///   @end
+/// \endcode
+const internal::VariadicDynCastAllOfMatcher<
+  Decl,
+  ObjCMethodDecl> objcMethodDecl;
+
+/// \brief Matches Objective-C instance variable declarations.
+///
+/// Example matches _enabled
+/// \code
+///   @implementation Foo {
+///     BOOL _enabled;
+///   }
+///   @end
+/// \endcode
+const internal::VariadicDynCastAllOfMatcher<
+  Decl,
+  ObjCIvarDecl> objcIvarDecl;
+
+/// \brief Matches Objective-C property declarations.
+///
+/// Example matches enabled
+/// \code
+///   @interface Foo
+///   @property BOOL enabled;
+///   @end
+/// \endcode
+const internal::VariadicDynCastAllOfMatcher<
+  Decl,
+  ObjCPropertyDecl> objcPropertyDecl;
+
 /// \brief Matches expressions that introduce cleanups to be run at the end
 /// of the sub-expression's evaluation.
 ///
 /// Example matches std::string()
 /// \code
 ///   const std::string str = std::string();
 /// \endcode
 const internal::VariadicDynCastAllOfMatcher<

lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(namesType);
   REGISTER_MATCHER(nestedNameSpecifier);
   REGISTER_MATCHER(nestedNameSpecifierLoc);
   REGISTER_MATCHER(nullPointerConstant);
   REGISTER_MATCHER(nullStmt);
   REGISTER_MATCHER(numSelectorArgs);
   REGISTER_MATCHER(ofClass);
   REGISTER_MATCHER(objcInterfaceDecl);
+  REGISTER_MATCHER(objcProtocolDecl);
+  REGISTER_MATCHER(objcCategoryDecl);
+  REGISTER_MATCHER(objcMethodDecl);
+  REGISTER_MATCHER(objcIvarDecl);
+  REGISTER_MATCHER(objcPropertyDecl);
   REGISTER_MATCHER(objcMessageExpr);
   REGISTER_MATCHER(objcObjectPointerType);
   REGISTER_MATCHER(on);
   REGISTER_MATCHER(onImplicitObjectArgument);
   REGISTER_MATCHER(opaqueValueExpr);
   REGISTER_MATCHER(parameterCountIs);
   REGISTER_MATCHER(parenExpr);
   REGISTER_MATCHER(parenListExpr);

unittests/ASTMatchers/ASTMatchersNodeTest.cpp

TEST(TypedefNameDeclMatcher, Match) {
   EXPECT_TRUE(matches("using typedefNameDeclTest2 = int;",
                       typedefNameDecl(hasName("typedefNameDeclTest2"))));
 }
 
 TEST(ObjCMessageExprMatcher, SimpleExprs) {
   // don't find ObjCMessageExpr where none are present
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything())));
 
   std::string Objc1String =
     "@interface Str "
-      " - (Str *)uppercaseString:(Str *)str;"
+      " - (Str *)uppercaseString;"
       "@end "
       "@interface foo "
+      "- (void)contents;"
       "- (void)meth:(Str *)text;"
       "@end "
       " "
       "@implementation foo "
       "- (void) meth:(Str *)text { "
       "  [self contents];"
       "  Str *up = [text uppercaseString];"
       "} "
EXPECT_TRUE(matchesObjC(
     objcMessageExpr(hasSelector("contents"), numSelectorArgs(0))));
   EXPECT_TRUE(matchesObjC(
     Objc1String,
     objcMessageExpr(matchesSelector("uppercase*"),
                     argumentCountIs(0)
     )));
 }
 
+TEST(ObjCDeclMacher, CoreDecls) {
+  std::string ObjCString =
+    "@protocol Proto "
+    "- (void)protoDidThing; "
+    "@end "
+    "@interface Thing "
+    "@property int enabled; "
+    "@end "
+    "@interface Thing (ABC) "
+    "- (void)abc_doThing; "
+    "@end "
+    "@implementation Thing "
+    "{ id _ivar; } "
+    "- (void)anything {} "
+    "@end "
+    ;
+
+  EXPECT_TRUE(matchesObjC(
+    ObjCString,
+    objcProtocolDecl(hasName("Proto"))));
+  EXPECT_TRUE(matchesObjC(
+    ObjCString,
+    objcCategoryDecl(hasName("ABC"))));
+  EXPECT_TRUE(matchesObjC(
+    ObjCString,
+    objcMethodDecl(hasName("protoDidThing"))));
+  EXPECT_TRUE(matchesObjC(
+    ObjCString,
+    objcMethodDecl(hasName("abc_doThing"))));
+  EXPECT_TRUE(matchesObjC(
+    ObjCString,
+    objcMethodDecl(hasName("anything"))));
+  EXPECT_TRUE(matchesObjC(
+    ObjCString,
+    objcIvarDecl(hasName("_ivar"))));
+  EXPECT_TRUE(matchesObjC(
+    ObjCString,
+    objcPropertyDecl(hasName("enabled"))));
+}
+
 } // namespace ast_matchers
 } // namespace clang

unittests/ASTMatchers/ASTMatchersTest.h

 private:
   bool *const Verified;
   const std::unique_ptr<BoundNodesCallback> FindResultReviewer;
 };
 
 template <typename T>
 testing::AssertionResult matchesConditionally(
     const std::string &Code, const T &AMatcher, bool ExpectMatch,
-    llvm::StringRef CompileArg,
+    std::vector<std::string> Args,
     const FileContentMappings &VirtualMappedFiles = FileContentMappings(),
     const std::string &Filename = "input.cc") {
   bool Found = false, DynamicFound = false;
   MatchFinder Finder;
   VerifyMatch VerifyFound(nullptr, &Found);
   Finder.addMatcher(AMatcher, &VerifyFound);
   VerifyMatch VerifyDynamicFound(nullptr, &DynamicFound);
   if (!Finder.addDynamicMatcher(AMatcher, &VerifyDynamicFound))
     return testing::AssertionFailure() << "Could not add dynamic matcher";
   std::unique_ptr<FrontendActionFactory> Factory(
       newFrontendActionFactory(&Finder));
   // Some tests need rtti/exceptions on.  Use an unknown-unknown triple so we
   // don't instantiate the full system toolchain.  On Linux, instantiating the
   // toolchain involves stat'ing large portions of /usr/lib, and this slows down
   // not only this test, but all other tests, via contention in the kernel.
   //
   // FIXME: This is a hack to work around the fact that there's no way to do the
   // equivalent of runToolOnCodeWithArgs without instantiating a full Driver.
   // We should consider having a function, at least for tests, that invokes cc1.
-  std::vector<std::string> Args = {CompileArg, "-frtti", "-fexceptions",
-                                   "-target", "i386-unknown-unknown"};
+  Args.insert(Args.end(), {"-frtti", "-fexceptions",
+                           "-target", "i386-unknown-unknown"});
   if (!runToolOnCodeWithArgs(
           Factory->create(), Code, Args, Filename, "clang-tool",
           std::make_shared<PCHContainerOperations>(), VirtualMappedFiles)) {
     return testing::AssertionFailure() << "Parsing error in \"" << Code << "\"";
   }
   if (Found != DynamicFound) {
     return testing::AssertionFailure() << "Dynamic match result ("
                                        << DynamicFound
                                        << ") does not match static result ("
                                        << Found << ")";
   }
   if (!Found && ExpectMatch) {
     return testing::AssertionFailure()
       << "Could not find match in \"" << Code << "\"";
   } else if (Found && !ExpectMatch) {
     return testing::AssertionFailure()
       << "Found unexpected match in \"" << Code << "\"";
   }
   return testing::AssertionSuccess();
 }
 
 template <typename T>
+testing::AssertionResult matchesConditionally(
+    const std::string &Code, const T &AMatcher, bool ExpectMatch,
+    llvm::StringRef CompileArg,
+    const FileContentMappings &VirtualMappedFiles = FileContentMappings(),
+    const std::string &Filename = "input.cc") {
+  return matchesConditionally(
+    Code, AMatcher, ExpectMatch, std::vector<std::string>{CompileArg},
+    VirtualMappedFiles, Filename);
+}
+
+template <typename T>
 testing::AssertionResult matches(const std::string &Code, const T &AMatcher) {
   return matchesConditionally(Code, AMatcher, true, "-std=c++11");
 }
 
 template <typename T>
 testing::AssertionResult notMatches(const std::string &Code,
                                     const T &AMatcher) {
   return matchesConditionally(Code, AMatcher, false, "-std=c++11");
 }
 
 template <typename T>
 testing::AssertionResult matchesObjC(const std::string &Code,
-                                     const T &AMatcher) {
+                                     const T &AMatcher,
+                                     bool ExpectMatch = true) {
   return matchesConditionally(
-    Code, AMatcher, true,
-    "", FileContentMappings(), "input.m");
+    Code, AMatcher, ExpectMatch,
+    {"-fobjc-nonfragile-abi",
+     "-Wno-objc-root-class", "-Wno-incomplete-implementation"},
+    FileContentMappings(), "input.m");
 }
 
 template <typename T>
 testing::AssertionResult matchesC(const std::string &Code, const T &AMatcher) {
   return matchesConditionally(Code, AMatcher, true, "", FileContentMappings(),
                               "input.c");
 }
 
 template <typename T>
 testing::AssertionResult matchesC99(const std::string &Code,
                                     const T &AMatcher) {
   return matchesConditionally(Code, AMatcher, true, "-std=c99",
                               FileContentMappings(), "input.c");
 }
 
 template <typename T>
 testing::AssertionResult notMatchesC(const std::string &Code,
                                      const T &AMatcher) {
   return matchesConditionally(Code, AMatcher, false, "", FileContentMappings(),
                               "input.c");
 }
 
 template <typename T>
 testing::AssertionResult notMatchesObjC(const std::string &Code,
                                         const T &AMatcher) {
-  return matchesConditionally(
-    Code, AMatcher, false,
-    "", FileContentMappings(), "input.m");
+  return matchesObjC(Code, AMatcher, false);
 }
 
 
 // Function based on matchesConditionally with "-x cuda" argument added and
 // small CUDA header prepended to the code string.
 template <typename T>
 testing::AssertionResult matchesConditionallyWithCuda(
     const std::string &Code, const T &AMatcher, bool ExpectMatch,