[cfe-commits] r53987 - in /cfe/trunk: lib/Analysis/CheckObjCDealloc.cpp test/Analysis/IBOutlet.m test/Analysis/MissingDealloc_SEL.m

[Chris Lattner]

On Jul 24, 2008, at 5:00 PM, Ted Kremenek wrote:
>> Hi Ted,
>>
>> I don't think this is the right fix.  Won't your checker still warn  
>> about other things like "int*" ivars?
>
> Sure.  Without doing more analysis, the question is whether or not  
> in the common case an Objective-C object with an int* owns the  
> memory referenced by the int*.  In such cases -dealloc should be  
> implemented.

Right.  Without that analysis though, it seems worthwhile to avoid the  
f-pos.

>>>   if ((T->isPointerType() || T->isObjCQualifiedIdType()) &&
>>> +        (ID->getAttr<IBOutletAttr>() == 0 && // Skip IBOutlets.
>>> +         !isSEL(T, SelII))) { // Skip SEL ivars.
>>>     containsPointerIvar = true;
>>
>> I think the right check should be what  
>> Sema::isObjCObjectPointerType does.  I think it would make sense to  
>> pull this out and put it on ASTContext or Type.  What do you think?
>
> Agreed.  I moved isObjCObjectPointerType() from Sema to ASTContext.

Awesome, however, you are still allowing any pointers:

+    if ((T->isPointerType() || Ctx.isObjCObjectPointerType(T)) &&
+         (ID->getAttr<IBOutletAttr>() == 0 && // Skip IBOutlets.
+          !isSEL(T, SelII))) { // Skip SEL ivars.

How about just:

   if (Ctx.isObjCObjectPointerType(T) && !ID->getAttr<IBOutletAttr>())
     ...

?

-Chris