[cfe-dev] Missing dealloc analysis

[David Chisnall]

On 3 Jul 2008, at 18:18, Jean-Daniel Dupas wrote:

>
> Le 3 juil. 08 à 18:28, Ted Kremenek a écrit :
>
>> Hi Nikita,
>>
>> On Jul 2, 2008, at 10:35 PM, Nikita Zhuk wrote:
>>
>>> I noticed the addition of missing dealloc analysis in r53075. It's a
>>> good idea to check that dealloc is implemented and that it always
>>> calls [super dealloc], but there're couple of points I would like to
>>> mention:
>>>
>>> 1. It should be disabled in GC environment, because dealloc methods
>>> are not called under GC
>>
>> Done:
>>
>>  http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20080630/006371.html
>>
>>> 2. In non-GC environment, the primary function of dealloc method  
>>> is to
>>> update retain counts of variables the object being dealloced is
>>> pointing to. So that usually means that dealloc must be implemented
>>> when an object has some ivars. Dealloc is not always required, and
>>> there're lot of classes which don't need it (e.g. singleton  
>>> classes).
>>> Currently missing dealloc analysis requires every class to implement
>>> dealloc and it's causing a lot of false positives.
>>>
>>> I'm not completely sure if absence of ivars should be the only  
>>> factor
>>> which disables missing dealloc analysis, but at least in my case it
>>> would suppress a lot of false positives.
>>
>> Done:
>>
>>  http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20080630/006373.html
>>
>> Both commits are now rolled into checker-55 on the Clang website.
>>
>> These are both great suggestions.  I remembered the GC/non-GC issue
>> this morning before I read your email, and I can easily see how not
>> checking to see if a class contains any ivars would lead to many  
>> false
>> positives.  I also believe that checking for ivars should also handle
>> the Singleton design pattern (please correct me if I am wrong).
>
> Why do you think singleton class need a special case ?
>
> in obj-c a singleton class is usually implemented using a class  
> method that return a shared instance:
>
> + (id)sharedInstance {
> 	static Foo *sharedInstance = nil;
> 	if (!sharedInstance) {
> 		sharedInstance = [[Foo alloc] init];
> 	}
> 	return sharedInstance;
> }
>
> optionaly you can also override +alloc, -init, -retain, -release to  
> prevent creation of other instance, and destruction of the shared  
> instance, but this is not recommanded (except if you have a good  
> reason to do so).
>
> It's a good practice to release ivars in a singleton dealloc, even  
> if dealloc may never be call (like that, if needed, you can create  
> other instance of this class).
>
> So I don't think it need a special case, but I maybe wrong too.

This is especially true when you consider subclassing.  A singleton  
can not guarantee that its subclasses will also be singletons, and so  
should release all of its ivars when it is -dealloc'd, or risk causing  
leaks if it is subclassed.

If a singleton needs to have shared resources that are also shared  
with subclasses then it should store them in file statics, no ivars.

David