Deep copy of dictionaries gives Analyze error in Xcode 4.2

Question

I have the following method in a NSDictionary category, to do a deep copy, which works fine.

I just upgraded from Xcode 4.1 to 4.2, and the Analyze function gives two analyzer warnings for this code, as indicated:

- (id)deepCopy;
{
    id dict = [[NSMutableDictionary alloc] init];
    id copy;

    for (id key in self)
    {
        id object = [self objectForKey:key];

        if ([object respondsToSelector:@selector(deepCopy)])
            copy = [object deepCopy];
        else
            copy = [object copy];

        [dict setObject:copy forKey:key];

        // Both -deepCopy and -copy retain the object, and so does -setObject:forKey:, so need to -release:
        [copy release];  // Xcode 4.2's Analyze says this is an incorrect decrement of the reference count?!
    }

    return dict;  // Xcode 4.2's Analyze says this is a potential leak
}

Are these bugs in Xcode's analyzer, or are there changes I can make to avoid these warnings?

I'm not using ARC yet, though I am interested if there are additional changes needed to support ARC for this method.

Answer 1

Presumably, it is because deepCopy does not begin with the prefix copy.

So you may want to change to something like copyWithDeepCopiedValues (or something like that), and then see if the analyzer flags that.

Update

As Alexsander noted, you can use attributes to denote reference counting intent. This should (IMO) be the exception to the rule, and used rarely, if ever. Personally, I will not use attributes for objc methods because it is fragile.

The only attribute I have used so far has been consume, and every time I use these attributes has been in statically typed contexts (e.g. C functions and C++ functions and methods).

The reasons you should avoid attributes when possible:

1) Stick with conventions for the programmers' sake. The code is clearer and you do not need to refer to the documentation.

2) The approach is fragile. You can still introduce reference count imbalances, and attributes can be used to introduce build errors due to conflicts in attributes.

The following cases are all built with ARC enabled:

Case #1

#import <Foundation/Foundation.h>

@interface MONType : NSObject

- (NSString *)string __attribute__((objc_method_family(copy)));

@end

@implementation MONType

- (NSString *)string
{
    NSMutableString * ret = [NSMutableString new];
    [ret appendString:@"MONType"];
    return ret;
}

@end

int main (int argc, const char * argv[])
{
    @autoreleasepool {
        id obj = nil;
        if (random() % 2U) {
            obj = [[NSAttributedString alloc] initWithString:@"NSAttributedString"];
        }
        else {
            obj = [MONType new];
        }
        NSLog(@"Result: %@, %@", obj, [obj string]);
    }
    /* this tool's name is ARC, dump the leaks: */
    system("leaks ARC");
    return 0;
}

This program produces the following error: error: multiple methods named 'string' found with mismatched result, parameter type or attributes.

Great, the compiler's doing what it can to prevent these issues. What that means is that conflicts in attributes can introduce errors based on the translation. This is bad because when nontrivial codebases are combined and attributes conflict, you will have errors to correct and programs to update. This also means that simply including other libraries in translation units can break existing programs when attributes are used.

Case #2

Header.h

extern id NewObject(void);

Header.m

#import <Foundation/Foundation.h>
#import "Header.h"

@interface MONType : NSObject

- (NSString *)string __attribute__((objc_method_family(copy)));

@end

@implementation MONType

- (NSString *)string
{
    NSMutableString * ret = [NSMutableString new];
    [ret appendString:@"-[MONType string]"];
    return ret;
}

@end


id NewObject(void) {
    id obj = nil;
    if (random() % 2U) {
        obj = [[NSAttributedString alloc] initWithString:@"NSAttributedString"];
    }
    else {
        obj = [MONType new];
    }
    return obj;
}

main.m

#import <Foundation/Foundation.h>
#import "Header.h"

int main (int argc, const char * argv[])
{
    @autoreleasepool {
        for (size_t idx = 0; idx < 8; ++idx) {
            id obj = NewObject();
            NSLog(@"Result: %@, %@", obj, [obj string]);
        }
    }
    /* this tool's name is ARC, dump the leaks: */
    system("leaks ARC");
    return 0;
}

Ok. This is just bad. We've introduced leaks because the necessary information was not available in the translation unit. Here's the leaks report:

leaks Report Version:  2.0
Process 7778: 1230 nodes malloced for 210 KB
Process 7778: 4 leaks for 192 total leaked bytes.
Leak: 0x1005001f0  size=64  zone: DefaultMallocZone_0x100003000   __NSCFString  ObjC  CoreFoundation  mutable non-inline:  "-[MONType string]"
Leak: 0x100500320  size=64  zone: DefaultMallocZone_0x100003000   __NSCFString  ObjC  CoreFoundation  mutable non-inline:  "-[MONType string]"
Leak: 0x100500230  size=32  zone: DefaultMallocZone_0x100003000  has-length-byte:  "-[MONType string]"
Leak: 0x100500390  size=32  zone: DefaultMallocZone_0x100003000  has-length-byte:  "-[MONType string]"

note: the count may differ because we used random()

This means that because MONType is not visible to main(), the compiler bound the ARC properties to methods which were visible to the current TU (that is, string from declarations in Foundation, all of which follow convention). As a result, the compiler got it wrong and we were able to introduce leaks into our program.

Case 3

Using a similar approach, I was also able to introduce negative reference count imbalances (premature releases, or a messaged zombie).

note: Code not provided because Case #2 already illustrates how one can accomplish a reference count imbalance.

Conclusion

You can avoid all these problems and improve readability and maintainability by sticking with convention, rather than using attributes.

Bringing the conversation back to non-ARC code: Using attributes makes manual memory management more difficult for programmers' readability, and for the tools which are there to help you (e.g. compiler, static analysis). If the program is suitably complex such that the tools can't detect such errors, then you should reconsider your design, because it will be equally complex for you or somebody else to debug these issues.

Answer 2

Adding onto @Justin's answer, you can tell the compiler that -deepCopy returns a retained object by appending the NS_RETURNS_RETAINED attribute to the method's declaration like so:

- (id) deepCopy NS_RETURNED_RETAINED;

Alternatively, you can use explicitly control the method's "family" using the objc_method_family attribute like so:

- (id) deepCopy __attribute__((objc_method_family(copy)));

If you do this, the compiler will know that this method is in the copy family and returns a copied value.