aaron.ballman added reviewers: jordan_rose, rjmccall. aaron.ballman added a comment.
Adding some ObjC experts in case they'd like to weigh in on the name `isDerivedFrom` in this context. ================ Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479 +/// \endcode +AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface, + internal::Matcher<ObjCInterfaceDecl>, ---------------- stephanemoore wrote: > aaron.ballman wrote: > > stephanemoore wrote: > > > I am still uncertain about the naming. > > > > > > `isSubclassOf` seemed too generic as that could apply to C++ classes. > > > `objcIsSubclassOf` seemed unconventional as a matcher name. > > > `isSubclassOfObjCInterface` and `isSubclassOfObjCClass` seemed awkwardly > > > lengthy. > > > Creating a new namespace `clang::ast_matchers::objc` seemed unprecedented. > > > > > > I am happy to change the name if you think another name would be more > > > appropriate. > > Does ObjC use the term "derived" by any chance? We already have > > `isDerivedFrom`, so I'm wondering if we can use that to also match on an > > `ObjCInterfaceDecl`? > Objective-C doesn't generally use the term "derived" (for example, see > archive of [Programming With Objective-C > Defining > Classes](https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/DefiningClasses/DefiningClasses.html#//apple_ref/doc/uid/TP40011210-CH3-SW1)). > With that said, I don't think it's unreasonable or incorrect to use the term > "derived" to describe inheritance in Objective-C. The behavior of this > matcher is also consistent with the behavior of `isDerivedFrom`. In order to > change `isDerivedFrom`, I would also need to update `isSameOrDerivedFrom`. > That would probably be a good thing so that derivation matching feature set > is consistent for C++ and Objective-C language variants. > > Let me take a crack at merging this into `isDerivedFrom`. I agree that if we go with `isDerivedFrom`, you should update `isSameOrDerivedFrom` at the same time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60543/new/ https://reviews.llvm.org/D60543 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
