stephanemoore requested changes to this revision. stephanemoore added inline comments. This revision now requires changes to proceed.
================ Comment at: clang-tools-extra/clang-tidy/objc/DeallocInCategoriesCheck.cpp:21 + Finder->addMatcher( + objcMethodDecl(hasName("dealloc"), hasDeclContext(objcCategoryImplDecl())) + .bind("dealloc"), ---------------- Add `isInstanceMethod()` within the `objcMethodDecl`? ================ Comment at: clang-tools-extra/clang-tidy/objc/DeallocInCategoriesCheck.h:24 +/// http://clang.llvm.org/extra/clang-tidy/checks/objc-dealloc-in-categories.html +class DeallocInCategoriesCheck : public ClangTidyCheck { +public: ---------------- What do you think of the name `DeallocInCategoryCheck`? ================ Comment at: clang-tools-extra/test/clang-tidy/checkers/objc-dealloc-in-categories.m:23 + +@implementation Bar (Category) +- (void)dealloc { ---------------- Please add a class method `+dealloc` and make sure that the check does not trigger on that. ================ Comment at: clang-tools-extra/test/clang-tidy/checkers/objc-dealloc-in-categories.m:35 +// overriding implementation, and should not generate a warning. +- (void)dealloc; +@end ---------------- Perhaps add an `@implementation` for `Baz` (not a category implementation) that contains `-dealloc` to ensure that a declaration in a category with a definition in the implementation does not trigger? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72876/new/ https://reviews.llvm.org/D72876 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits