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