[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

This revision was automatically updated to reflect the committed changes. stephanemoore marked an inline comment as done. Closed by commit rL368632: [clang] Update isDerivedFrom to support Objective-C classes  (authored by stephanemoore, committed by ). Herald added a project: LLVM. Herald added

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore marked 3 inline comments as done. stephanemoore added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:820 + llvm::DenseMap> + CompatibleAliases; aaron.ballman wrote: > gribozavr wrote: > > `unordered_set`? > or

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore updated this revision to Diff 214272. stephanemoore added a comment. Use `llvm::SmallPtrSet` to store the compatible aliases instead of `std::set`. Fix a stray unit test failure in `RegistryTest.cpp`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:820 + llvm::DenseMap> + CompatibleAliases; gribozavr wrote: > `unordered_set`? or `SmallPtrSet`? Repository: rG LLVM Github

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:820 + llvm::DenseMap> + CompatibleAliases; `unordered_set`? Repository: rG LLVM

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore updated this revision to Diff 214053. stephanemoore added a comment. Add tests for `isDirectlyDerivedFrom`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60543/new/ https://reviews.llvm.org/D60543 Files:

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore planned changes to this revision. stephanemoore added a comment. Whoops; forgot to add test cases for `isDirectlyDerivedFrom` 臘 Will do that shortly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60543/new/

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore added a comment. I spent some time becoming familiar with how `isDerivedFrom` behaves for C++ classes. I //think// that I have managed to get the behavior for Objective-C classes to mirror that of C++ classes. Please let me know if I overlooked anything.

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore updated this revision to Diff 214035. stephanemoore marked 6 inline comments as done. stephanemoore edited the summary of this revision. stephanemoore added a comment. Update `isDerivedFrom` to match aliased types and compatibility aliases of derived Objective-C classes.

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore planned changes to this revision. stephanemoore added a comment. Thanks for the input! I will get started on making changes accordingly. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2642-2649 + if (const auto *InterfaceDecl = dyn_cast()) { +//

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2638 + // Check if the node is a C++ struct/union/class. + if (const auto *RecordDecl = dyn_cast()) +return Finder->classIsDerivedFrom(RecordDecl, Base, Builder);

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore accepted this revision. stephanemoore added a comment. This revision is now accepted and ready to land. Okay I now have an implementation of Option 2 that //works//. I was hoping to find a more elegant solution but since this is the first working implementation of Option 2 I was

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore updated this revision to Diff 201385. stephanemoore marked an inline comment as done. stephanemoore added a comment. Add missing braces to multi-line if statements. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60543/new/

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore updated this revision to Diff 201384. stephanemoore added a comment. Update isDerivedFrom and related matchers to polymorphic matchers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60543/new/ https://reviews.llvm.org/D60543 Files:

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

gribozavr added a comment. > I prefer Option 2 because it is a cleaner, more understandable design for the > matchers. I agree with Aaron. Clang or Clang Tooling provide no promise of source stability. Of course we should not break things just because we can, but API coherence and

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore planned changes to this revision. stephanemoore added a comment. > Out of curiosity, how invasive is Option 2 within our own code base? I am still working on getting something working but I don't anticipate anything notably invasive. > Does that option require fixing a lot of

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

aaron.ballman added reviewers: klimek, alexfh. aaron.ballman added a comment. Adding some more AST matcher reviewers to see if there are other options that we've not considered. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60543/new/

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

aaron.ballman added a comment. In D60543#1497576 , @stephanemoore wrote: > I did some digging and I believe there are two approaches that we can take to > extend `isDerivedFrom` to support Objective-C classes. > > **Option 1: Match on Common Ancestor

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:566 + notMatchesObjC("@interface Y @end typedef Y X; @interface Z : X @end", + ZIsDerivedFromX)); + EXPECT_TRUE( (note that there are

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 

stephanemoore added a comment. I did some digging and I believe there are two approaches that we can take to extend `isDerivedFrom` to support Objective-C classes. **Option 1: Match on Common Ancestor Declaration Type**: Convert `isDerivedFrom` to match on the common ancestor declaration type,