aaron.ballman added a comment.
In D59806#1447929 <https://reviews.llvm.org/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 thing -- I am only
worried about trying to issue a fix-it in that case, not the diagnostic itself.
================
Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:112
+ << Message->getMethodDecl()
+ << FixItHint::CreateReplacement(Message->getSourceRange(),
+ StringRef("[super init]"));
----------------
stephanemoore wrote:
> stephanemoore wrote:
> > aaron.ballman wrote:
> > > This could be dangerous if the `[super self]` construct is in a macro,
> > > couldn't it? e.g.,
> > > ```
> > > #define DERP self
> > >
> > > [super DERP];
> > > ```
> > Good point. Let me add some test cases and make sure this is handled
> > properly.
> Added some test cases where `[super self]` is expanded from macros.
You missed the test case I was worried about -- where the macro is mixed into
the expression. I don't think we want to try to add a fix-it in that case.
================
Comment at: clang-tools-extra/test/clang-tidy/objc-super-self.m:41
+ INITIALIZER_IMPL();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: suspicious invocation of 'self' in
initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+}
----------------
Are you missing a `CHECK-FIXES` here?
Personally, I don't think we should try to generate a fixit for this case, so I
would expect a CHECK-FIXES that ensures this doesn't get modified.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59806/new/
https://reviews.llvm.org/D59806
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
