[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione added a comment. @aaron.ballman @malcolm.parsons Thanks for the reviews and the commit. I plan to do follow ups for more ObjC ASTMatcher support. https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
aaron.ballman closed this revision. aaron.ballman added a comment. I've commit in r297882, thanks! https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
aaron.ballman accepted this revision.
aaron.ballman added a comment.
LGTM! I'll go ahead and commit for you.
Comment at: unittests/ASTMatchers/ASTMatchersTest.h:114
+const std::string = "input.cc") {
+ return matchesConditionally(
+Code, AMatcher, ExpectMatch, llvm::makeArrayRef(CompileArg),
The formatting of this line is incorrect, but I can fix it when I commit.
https://reviews.llvm.org/D30854
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione added inline comments.
Comment at: unittests/ASTMatchers/ASTMatchersTest.h:64
const std::string , const T , bool ExpectMatch,
-llvm::StringRef CompileArg,
+std::vector Args,
const FileContentMappings = FileContentMappings(),
aaron.ballman wrote:
> kastiglione wrote:
> > aaron.ballman wrote:
> > > I think this might be better as an `llvm::ArrayRef`.
> > Ok. It needs to be a `std::vector` 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?
> Can you do:
> ```
> std::vector LocalArgs(Args.begin(), Args.end());
> LocalArgs.insert(LocalArgs.end(), {"everything else"});
> ```
> below?
right, thanks!
https://reviews.llvm.org/D30854
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione updated this revision to Diff 91914.
kastiglione added a comment.
Use ArrayRef
https://reviews.llvm.org/D30854
Files:
docs/LibASTMatchersReference.html
include/clang/ASTMatchers/ASTMatchers.h
lib/ASTMatchers/Dynamic/Registry.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
unittests/ASTMatchers/ASTMatchersTest.h
Index: unittests/ASTMatchers/ASTMatchersTest.h
===
--- unittests/ASTMatchers/ASTMatchersTest.h
+++ unittests/ASTMatchers/ASTMatchersTest.h
@@ -61,7 +61,7 @@
template
testing::AssertionResult matchesConditionally(
const std::string , const T , bool ExpectMatch,
-llvm::StringRef CompileArg,
+llvm::ArrayRef CompileArgs,
const FileContentMappings = FileContentMappings(),
const std::string = "input.cc") {
bool Found = false, DynamicFound = false;
@@ -81,8 +81,9 @@
// 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 Args = {CompileArg, "-frtti", "-fexceptions",
- "-target", "i386-unknown-unknown"};
+ std::vector Args(CompileArgs.begin(), CompileArgs.end());
+ Args.insert(Args.end(), {"-frtti", "-fexceptions",
+ "-target", "i386-unknown-unknown"});
if (!runToolOnCodeWithArgs(
Factory->create(), Code, Args, Filename, "clang-tool",
std::make_shared(), VirtualMappedFiles)) {
@@ -105,6 +106,17 @@
}
template
+testing::AssertionResult matchesConditionally(
+const std::string , const T , bool ExpectMatch,
+llvm::StringRef CompileArg,
+const FileContentMappings = FileContentMappings(),
+const std::string = "input.cc") {
+ return matchesConditionally(
+Code, AMatcher, ExpectMatch, llvm::makeArrayRef(CompileArg),
+VirtualMappedFiles, Filename);
+}
+
+template
testing::AssertionResult matches(const std::string , const T ) {
return matchesConditionally(Code, AMatcher, true, "-std=c++11");
}
@@ -117,10 +129,13 @@
template
testing::AssertionResult matchesObjC(const std::string ,
- const T ) {
+ const T ,
+ 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
@@ -145,10 +160,8 @@
template
testing::AssertionResult notMatchesObjC(const std::string ,
- const T ) {
- return matchesConditionally(
-Code, AMatcher, false,
-"", FileContentMappings(), "input.m");
+const T ) {
+ return matchesObjC(Code, AMatcher, false);
}
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1500,9 +1500,10 @@
std::string Objc1String =
"@interface Str "
- " - (Str *)uppercaseString:(Str *)str;"
+ " - (Str *)uppercaseString;"
"@end "
"@interface foo "
+ "- (void)contents;"
"- (void)meth:(Str *)text;"
"@end "
" "
@@ -1540,5 +1541,45 @@
)));
}
+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
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -360,6 +360,11 @@
REGISTER_MATCHER(numSelectorArgs);
REGISTER_MATCHER(ofClass);
REGISTER_MATCHER(objcInterfaceDecl);
+
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
aaron.ballman added inline comments.
Comment at: unittests/ASTMatchers/ASTMatchersTest.h:64
const std::string , const T , bool ExpectMatch,
-llvm::StringRef CompileArg,
+std::vector Args,
const FileContentMappings = FileContentMappings(),
kastiglione wrote:
> aaron.ballman wrote:
> > I think this might be better as an `llvm::ArrayRef`.
> Ok. It needs to be a `std::vector` 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?
Can you do:
```
std::vector LocalArgs(Args.begin(), Args.end());
LocalArgs.insert(LocalArgs.end(), {"everything else"});
```
below?
https://reviews.llvm.org/D30854
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersTest.h:64 const std::string , const T , bool ExpectMatch, -llvm::StringRef CompileArg, +std::vector Args, const FileContentMappings = FileContentMappings(), aaron.ballman wrote: > I think this might be better as an `llvm::ArrayRef`. Ok. It needs to be a `std::vector` 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? https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
aaron.ballman added inline comments.
Comment at: unittests/ASTMatchers/ASTMatchersTest.h:64
const std::string , const T , bool ExpectMatch,
-llvm::StringRef CompileArg,
+std::vector Args,
const FileContentMappings = FileContentMappings(),
I think this might be better as an `llvm::ArrayRef`.
Comment at: unittests/ASTMatchers/ASTMatchersTest.h:114
+ return matchesConditionally(
+Code, AMatcher, ExpectMatch, std::vector{CompileArg},
+VirtualMappedFiles, Filename);
This could then use `makeArrayRef(CompileArg)`.
https://reviews.llvm.org/D30854
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione updated this revision to Diff 91801.
kastiglione added a comment.
Overload matchesConditionally() for multiple compiler args
https://reviews.llvm.org/D30854
Files:
docs/LibASTMatchersReference.html
include/clang/ASTMatchers/ASTMatchers.h
lib/ASTMatchers/Dynamic/Registry.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
unittests/ASTMatchers/ASTMatchersTest.h
Index: unittests/ASTMatchers/ASTMatchersTest.h
===
--- unittests/ASTMatchers/ASTMatchersTest.h
+++ unittests/ASTMatchers/ASTMatchersTest.h
@@ -61,7 +61,7 @@
template
testing::AssertionResult matchesConditionally(
const std::string , const T , bool ExpectMatch,
-llvm::StringRef CompileArg,
+std::vector Args,
const FileContentMappings = FileContentMappings(),
const std::string = "input.cc") {
bool Found = false, DynamicFound = false;
@@ -81,8 +81,8 @@
// 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 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(), VirtualMappedFiles)) {
@@ -105,6 +105,17 @@
}
template
+testing::AssertionResult matchesConditionally(
+const std::string , const T , bool ExpectMatch,
+llvm::StringRef CompileArg,
+const FileContentMappings = FileContentMappings(),
+const std::string = "input.cc") {
+ return matchesConditionally(
+Code, AMatcher, ExpectMatch, std::vector{CompileArg},
+VirtualMappedFiles, Filename);
+}
+
+template
testing::AssertionResult matches(const std::string , const T ) {
return matchesConditionally(Code, AMatcher, true, "-std=c++11");
}
@@ -117,10 +128,13 @@
template
testing::AssertionResult matchesObjC(const std::string ,
- const T ) {
+ const T ,
+ 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
@@ -145,10 +159,8 @@
template
testing::AssertionResult notMatchesObjC(const std::string ,
- const T ) {
- return matchesConditionally(
-Code, AMatcher, false,
-"", FileContentMappings(), "input.m");
+const T ) {
+ return matchesObjC(Code, AMatcher, false);
}
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1500,9 +1500,10 @@
std::string Objc1String =
"@interface Str "
- " - (Str *)uppercaseString:(Str *)str;"
+ " - (Str *)uppercaseString;"
"@end "
"@interface foo "
+ "- (void)contents;"
"- (void)meth:(Str *)text;"
"@end "
" "
@@ -1540,5 +1541,45 @@
)));
}
+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
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -360,6 +360,11 @@
REGISTER_MATCHER(numSelectorArgs);
REGISTER_MATCHER(ofClass);
REGISTER_MATCHER(objcInterfaceDecl);
+ REGISTER_MATCHER(objcProtocolDecl);
+
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
aaron.ballman added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:1547 + std::string ObjCString = +"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n" +"@protocol Proto " kastiglione wrote: > aaron.ballman wrote: > > kastiglione wrote: > > > kastiglione wrote: > > > > aaron.ballman wrote: > > > > > 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. > > > > `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()`? > > > I notice that many other tests have warnings. Should these tests just > > > allow the warnings to be emitted? > > 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. > Sounds good, I'm for suppressing them. Should I refactor > `matchesConditionally()` to allow multiple compile args, and disable these > warnings from `matchesObjC()`? Yes, I think that's the way to go. Comment at: unittests/ASTMatchers/ASTMatchersTest.h:123 Code, AMatcher, true, -"", FileContentMappings(), "input.m"); +"-fobjc-runtime=macosx", FileContentMappings(), "input.m"); } kastiglione wrote: > aaron.ballman wrote: > > kastiglione wrote: > > > aaron.ballman wrote: > > > > Can you explain why this change is required? > > > `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? > > 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. > > 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. Fair enough https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:1547 + std::string ObjCString = +"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n" +"@protocol Proto " aaron.ballman wrote: > kastiglione wrote: > > kastiglione wrote: > > > aaron.ballman wrote: > > > > 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. > > > `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()`? > > I notice that many other tests have warnings. Should these tests just allow > > the warnings to be emitted? > 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. Sounds good, I'm for suppressing them. Should I refactor `matchesConditionally()` to allow multiple compile args, and disable these warnings from `matchesObjC()`? Comment at: unittests/ASTMatchers/ASTMatchersTest.h:123 Code, AMatcher, true, -"", FileContentMappings(), "input.m"); +"-fobjc-runtime=macosx", FileContentMappings(), "input.m"); } aaron.ballman wrote: > kastiglione wrote: > > aaron.ballman wrote: > > > Can you explain why this change is required? > > `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? > 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. > 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. https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
aaron.ballman added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:1547 + std::string ObjCString = +"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n" +"@protocol Proto " kastiglione wrote: > kastiglione wrote: > > aaron.ballman wrote: > > > 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. > > `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()`? > I notice that many other tests have warnings. Should these tests just allow > the warnings to be emitted? 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. Comment at: unittests/ASTMatchers/ASTMatchersTest.h:123 Code, AMatcher, true, -"", FileContentMappings(), "input.m"); +"-fobjc-runtime=macosx", FileContentMappings(), "input.m"); } kastiglione wrote: > aaron.ballman wrote: > > Can you explain why this change is required? > `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? 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. https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione updated this revision to Diff 91740.
kastiglione added a comment.
Use -fobjc-nonfragile-abi
https://reviews.llvm.org/D30854
Files:
docs/LibASTMatchersReference.html
include/clang/ASTMatchers/ASTMatchers.h
lib/ASTMatchers/Dynamic/Registry.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
unittests/ASTMatchers/ASTMatchersTest.h
Index: unittests/ASTMatchers/ASTMatchersTest.h
===
--- unittests/ASTMatchers/ASTMatchersTest.h
+++ unittests/ASTMatchers/ASTMatchersTest.h
@@ -120,7 +120,7 @@
const T ) {
return matchesConditionally(
Code, AMatcher, true,
-"", FileContentMappings(), "input.m");
+"-fobjc-nonfragile-abi", FileContentMappings(), "input.m");
}
template
@@ -148,7 +148,7 @@
const T ) {
return matchesConditionally(
Code, AMatcher, false,
-"", FileContentMappings(), "input.m");
+"-fobjc-nonfragile-abi", FileContentMappings(), "input.m");
}
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1498,7 +1498,9 @@
// don't find ObjCMessageExpr where none are present
EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
- std::string Objc1String =
+ std::string ObjCString =
+"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n"
+"#pragma clang diagnostic ignored \"-Wobjc-method-access\"\n"
"@interface Str "
" - (Str *)uppercaseString:(Str *)str;"
"@end "
@@ -1513,32 +1515,73 @@
"} "
"@end ";
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(anything(;
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(hasSelector("contents";
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(matchesSelector("cont*";
EXPECT_FALSE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(matchesSelector("?cont*";
EXPECT_TRUE(notMatchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(hasSelector("contents"), hasNullSelector(;
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(hasSelector("contents"), hasUnarySelector(;
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(hasSelector("contents"), numSelectorArgs(0;
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(matchesSelector("uppercase*"),
argumentCountIs(0)
)));
}
+TEST(ObjCDeclMacher, CoreDecls) {
+ std::string ObjCString =
+"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n"
+"@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
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -360,6 +360,11 @@
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);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -1118,6 +1118,69 @@
Decl,
ObjCInterfaceDecl> objcInterfaceDecl;
+/// \brief Matches Objective-C protocol declarations.
+///
+/// Example matches FooDelegate
+/// \code
+/// @protocol FooDelegate
+/// @end
+/// \endcode
+const internal::VariadicDynCastAllOfMatcher<
+ Decl,
+
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:1547 + std::string ObjCString = +"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n" +"@protocol Proto " kastiglione wrote: > aaron.ballman wrote: > > 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. > `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()`? I notice that many other tests have warnings. Should these tests just allow the warnings to be emitted? https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:1501 - std::string Objc1String = + std::string ObjCString = +"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n" aaron.ballman wrote: > These changes are unrelated and should be in a separate commit. They're related to the use of either `-fobjc-runtime=` or `fobjc-nonfragile-abi`. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:1547 + std::string ObjCString = +"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n" +"@protocol Proto " aaron.ballman wrote: > 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. `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()`? Comment at: unittests/ASTMatchers/ASTMatchersTest.h:123 Code, AMatcher, true, -"", FileContentMappings(), "input.m"); +"-fobjc-runtime=macosx", FileContentMappings(), "input.m"); } aaron.ballman wrote: > Can you explain why this change is required? `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? https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
aaron.ballman added inline comments. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:1501 - std::string Objc1String = + std::string ObjCString = +"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n" These changes are unrelated and should be in a separate commit. Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:1547 + std::string ObjCString = +"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n" +"@protocol Proto " 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. Comment at: unittests/ASTMatchers/ASTMatchersTest.h:123 Code, AMatcher, true, -"", FileContentMappings(), "input.m"); +"-fobjc-runtime=macosx", FileContentMappings(), "input.m"); } Can you explain why this change is required? https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione added a comment. Thanks @malcolm.parsons. I don't have commit access, will you be able to commit this? https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
malcolm.parsons accepted this revision. malcolm.parsons added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione added a comment. I'd like to add more ObjC specific matcher functionality in follow up diffs. This diff is to to start somewhere, and get feedback. https://reviews.llvm.org/D30854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30854: [ASTMatchers] Add ObjC matchers for fundamental decls
kastiglione created this revision.
This adds matchers for `ObjCProtocolDecl`, `ObjCCategoryDecl`,
`ObjCMethodDecl`, `ObjCIvarDecl`, and
`ObjCPropertyDecl`. These matchers complement the existing matcher for
`ObjCInterfaceDecl`.
https://reviews.llvm.org/D30854
Files:
docs/LibASTMatchersReference.html
include/clang/ASTMatchers/ASTMatchers.h
lib/ASTMatchers/Dynamic/Registry.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
unittests/ASTMatchers/ASTMatchersTest.h
Index: unittests/ASTMatchers/ASTMatchersTest.h
===
--- unittests/ASTMatchers/ASTMatchersTest.h
+++ unittests/ASTMatchers/ASTMatchersTest.h
@@ -120,7 +120,7 @@
const T ) {
return matchesConditionally(
Code, AMatcher, true,
-"", FileContentMappings(), "input.m");
+"-fobjc-runtime=macosx", FileContentMappings(), "input.m");
}
template
@@ -148,7 +148,7 @@
const T ) {
return matchesConditionally(
Code, AMatcher, false,
-"", FileContentMappings(), "input.m");
+"-fobjc-runtime=macosx", FileContentMappings(), "input.m");
}
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1498,7 +1498,9 @@
// don't find ObjCMessageExpr where none are present
EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
- std::string Objc1String =
+ std::string ObjCString =
+"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n"
+"#pragma clang diagnostic ignored \"-Wobjc-method-access\"\n"
"@interface Str "
" - (Str *)uppercaseString:(Str *)str;"
"@end "
@@ -1513,32 +1515,73 @@
"} "
"@end ";
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(anything(;
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(hasSelector("contents";
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(matchesSelector("cont*";
EXPECT_FALSE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(matchesSelector("?cont*";
EXPECT_TRUE(notMatchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(hasSelector("contents"), hasNullSelector(;
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(hasSelector("contents"), hasUnarySelector(;
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(hasSelector("contents"), numSelectorArgs(0;
EXPECT_TRUE(matchesObjC(
-Objc1String,
+ObjCString,
objcMessageExpr(matchesSelector("uppercase*"),
argumentCountIs(0)
)));
}
+TEST(ObjCDeclMacher, CoreDecls) {
+ std::string ObjCString =
+"#pragma clang diagnostic ignored \"-Wobjc-root-class\"\n"
+"@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
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -360,6 +360,11 @@
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);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -1118,6 +1118,69 @@
Decl,
ObjCInterfaceDecl> objcInterfaceDecl;
+/// \brief Matches Objective-C protocol declarations.
+///
+/// Example matches FooDelegate
+///
