stephanemoore accepted this revision.
stephanemoore added inline comments.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/objc-nsinvocation-argument-lifetime.m:64-69
+- (void)processInvocation:(NSInvocation *)Invocation {
+ [Invocation getArgument:&Argument atIndex:2];
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: NSInvocation
'-getArgument:atIndex:' should only pass pointers to objects with ownership
__unsafe_unretained [objc-nsinvocation-argument-lifetime]
+ [Invocation getArgument:&self->Argument atIndex:2];
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: NSInvocation
'-getArgument:atIndex:' should only pass pointers to objects with ownership
__unsafe_unretained [objc-nsinvocation-argument-lifetime]
+}
----------------
mwyman wrote:
> mwyman wrote:
> > stephanemoore wrote:
> > > stephanemoore wrote:
> > > > How about a test case where we take the address of something on an ivar?
> > > >
> > > > For example, I think the code below should not produce a diagnostic
> > > > finding, right?
> > > > ```
> > > > struct Example {
> > > > __unsafe_unretained id Field;
> > > > };
> > > >
> > > > @interface TestClass : NSObject {
> > > > struct Example ExampleIvar;
> > > > }
> > > >
> > > > @end
> > > >
> > > > @implementation TestClass
> > > >
> > > > - (void)processInvocation:(NSInvocation *)Invocation {
> > > > [Invocation getArgument:&(self->ExampleIvar.Field) atIndex:2];
> > > > }
> > > >
> > > > @end
> > > > ```
> > > Maybe worth adding an ivar case that doesn't produce a diagnostic?
> > This required checking MemberRefExprs too. Added both a matching and
> > non-matching case here.
> Added a non-matching ivar case, and in doing so discovered it was often
> actually matching the "self" reference, which is an ImplicitParamDecl, itself
> a type of VarDecl, which matched the declRefExpr(), leading to even cases
> that shouldn't have produced diagnostics to produce them.
>
> Have fixed this by excluding the self case (by ensuring the declRefExpr's
> parent is not an implicitCastExpr, which only appears in the AST that I've
> seen around the self—or other dereferenced object—reference).
It might be good to also have a case using the address of a field of a struct
local variable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77571/new/
https://reviews.llvm.org/D77571
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
