[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

This revision was automatically updated to reflect the committed changes. Closed by commit rL358620: [clang-tidy] Add a check for [super self] in initializers  (authored by stephanemoore, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore added a comment. In D59806#1469257 , @aaron.ballman wrote: > LGTM! You can either land this now and refactor after the AST matcher lands, > or you can wait until the AST matcher lands and land this patch after -- your > call. I will

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! You can either land this now and refactor after the AST matcher lands, or you can wait until the AST matcher lands and land this patch after -- your call. Repository:

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:57 +/// \endcode +AST_MATCHER_P(ObjCImplementationDecl, isSubclassOf, + ast_matchers::internal::Matcher, stephanemoore wrote: > aaron.ballman

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 194998. stephanemoore added a comment. Fix some formatting issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59806/new/ https://reviews.llvm.org/D59806 Files:

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 194997. stephanemoore marked 4 inline comments as done. stephanemoore added a comment. Check if either the receiver or selector are in macro locations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 194996. stephanemoore marked 3 inline comments as done. stephanemoore added a comment. Add `CHECK-FIXES` to verify code is preserved for scenarios that should not have fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 194995. stephanemoore added a comment. Update check to avoid emitting a fix if the expression is in a macro. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59806/new/ https://reviews.llvm.org/D59806

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore planned changes to this revision. stephanemoore marked an inline comment as done. stephanemoore added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:112 + << Message->getMethodDecl() + <<

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:112 + << Message->getMethodDecl() + << FixItHint::CreateReplacement(Message->getSourceRange(), + StringRef("[super init]"));

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:112 + << Message->getMethodDecl() + << FixItHint::CreateReplacement(Message->getSourceRange(), + StringRef("[super init]"));

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 193340. stephanemoore marked an inline comment as done. stephanemoore added a comment. Add a test case where a macro emits just `self` in the message expression. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

aaron.ballman added a comment. In D59806#1447929 , @jordan_rose wrote: > I don't think there's ever a reason to call `[super self]`, and doing so > through a macro could easily indicate a bug. Thank you for the verification! And agreed about the macro

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

rjmccall added a comment. In D59806#1448537 , @stephanemoore wrote: > In D59806#1447929 , @jordan_rose > wrote: > > > I don't think there's ever a reason to call `[super self]`, and doing so > > through a macro

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore marked 3 inline comments as done. stephanemoore added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:112 + << Message->getMethodDecl() + << FixItHint::CreateReplacement(Message->getSourceRange(), +

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 192938. stephanemoore added a comment. Add test cases with `[super self]` expanded from macros. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59806/new/ https://reviews.llvm.org/D59806 Files:

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

jordan_rose added a comment. Ooh, I should have remembered "designated initializer". I guess it doesn't matter so much either way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59806/new/ https://reviews.llvm.org/D59806

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore added a comment. In D59806#1447929 , @jordan_rose wrote: > I don't think there's ever a reason to call `[super self]`, and doing so > through a macro could easily indicate a bug. Agreed. The only relatively common usage of -[NSObject

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

jordan_rose added a comment. I don't think there's ever a reason to call `[super self]`, and doing so through a macro could easily indicate a bug. Diagnostic nitpick: the Objective-C term is "init method", not "initializer". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

aaron.ballman added reviewers: jordan_rose, rjmccall. aaron.ballman added a comment. I think this basically LGTM, but I'd appreciate hearing from someone more well-versed in ObjC before landing this. My primary question is: are there situations where `[super self]` is sensible (if so, how

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 192383. stephanemoore added a comment. Update tests to match updated diagnostic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59806/new/ https://reviews.llvm.org/D59806 Files:

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore marked 2 inline comments as done. stephanemoore added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:110 + "invoke a superclass initializer?") + << Message->getMethodDecl() + <<

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore marked 2 inline comments as done. stephanemoore added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:113 + + Finds invocations of `-self` on super instances in initializers of subclasses + of `NSObject` and recommends calling a superclass

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 192373. stephanemoore added a comment. Use double backticks rather than single backticks for symbols in documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59806/new/

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:113 + + Finds invocations of `-self` on super instances in initializers of subclasses + of `NSObject` and recommends calling a superclass initializer instead. Please use

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:57 +/// \endcode +AST_MATCHER_P(ObjCImplementationDecl, isSubclassOf, + ast_matchers::internal::Matcher, aaron.ballman wrote: > This matcher seems

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 192317. stephanemoore added a comment. Enclose -self in backticks in check documentation (overlooked in previous attempt). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59806/new/

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 192316. stephanemoore marked an inline comment as done. stephanemoore added a comment. Enclose -self in backticks in release notes and check documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 192315. stephanemoore marked 5 inline comments as done. stephanemoore added a comment. Fix diagnostic format string to actually use the message's method declaration. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore updated this revision to Diff 192312. stephanemoore added a comment. Removed usage of `auto` with a nonobvious type in isSubclassOf matcher. Added backticks around `NSObject` in docs. Synchronized check description in release notes and check documentation. Repository: rG LLVM

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:57 +/// \endcode +AST_MATCHER_P(ObjCImplementationDecl, isSubclassOf, + ast_matchers::internal::Matcher, This matcher seems like it would be

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:61 + // Check if any of the superclasses of the class match. + for (const auto *SuperClass = Node.getClassInterface()->getSuperClass(); + SuperClass != nullptr;

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

stephanemoore created this revision. Herald added subscribers: cfe-commits, jdoerfert, xazax.hun, mgorny. Herald added a project: clang. stephanemoore edited the summary of this revision. This check aims to address a relatively common benign error where Objective-C subclass initializers call