stephanemoore added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:112
+ << Message->getMethodDecl()
+ << FixItHint::CreateReplacement(Message->getSourceRange(),
+ StringRef("[super init]"));
----------------
aaron.ballman wrote:
> 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.
Added a test case though at the moment it generates a fixit.
Before I investigate modifying the check to avoid generating a fixit in this
case, I think it would be helpful for me to understand your concerns in that
scenario better. Is your concern that a proper fix might involve fixing the
macro itself rather than the message expression?
```
#if FLAG
#define INVOCATION self
#else
#define INVOCATION init
#endif
- (instancetype)init {
return [super INVOCATION];
}
```
================
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]
+}
----------------
aaron.ballman wrote:
> 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.
No fix is currently generated for this case which is why there is no
`CHECK-FIXES`. I also agree that no fix should be generated for this case.
I must confess that I have yet to fully understand why the fix for this case is
discarded (though I am grateful for the behavior). Let me dig around to try to
better understand why no fixit is generated for this case and assess adding a
condition for emitting the fixit.
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
