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

Reply via email to