Re: alloc/init - copy/mutableCopy

[James Knight]

David Ayers wrote on Thu, 06 Feb 2003 22:57:40 +0100:
> Richard Frith-Macdonald wrote:
>
> > Perhaps we should abandon apple compatibility and make the abstract 
> > methods implement shallow copies too?
>
> I'm starting to think we should... Anyone writing custom collection 
> classes that don't implement shallow copies, will be subject to 
> breakage of code using copy/mutableCopy as soon as objects that don't 
> implement NSCopying are contained in the array or the code otherwise 
> relies on standard shallow copying behavior. It's not just an 
> inconvenience for class cluster developers but for the "users" of 
> class cluster objects when the semantics are undefined. It's just not 
> that simple to say "the exact nature of the copy is determined by the 
> class" when the API suggests copy/mutableCopy are to be used to 
> transform between mutable an immutable versions. To not rely in 
> shallow copies for copy/mutableCopy constrains the usefulness of these 
> methods needlessly.
> I think GNUstep can take the lead here, and just maybe Apple will 
> follow suit. But most likely not because we did it, but because 
> hopefully Apple developers will kindly inform them of the > 
> implecations.
>
> Anyway, when we use our own classes explicitly (e.g. GC(Mutable)Array) 
> I think it's perfectly fine to assume copy/mutableCopy to be shallow. 
> We just need to be sure that any arrays passed to us are converted to 
> our classes before we send copy/mutableCopy, which in GDL2 and 
> GC(Mutable)Array should generally already be the case.

I concur with the above. Doing a deep copy in copy/mutableCopy seems 
very unlikely to ever be helpful. I suggest always doing a shallow 
copy, and putting prominantly in the API docs for 
NSArray/NSSet/NSDictionary that if you make your own subclasses, you 
must override copyWithZone to be portable to MacOSX, until such a point 
in time as Apple fixes their implementation as well.

I do consider this to be a bug on Apple's part. Accordingly, I reported 
it - it was assigned Bug #3175688. If anyone else wants to send them 
additional reasoning, sending an email to devbugs@apple.com referencing 
that bug number is probably the easiest way.

Apple bug report follows:

* SUMMARY
The abstract collection classes implement copyWithZone: to do a deep 
copy of the collection. This behavior is fairly non-sensical, as all 
the Cocoa built-in collections classes implement copyWithZone: to do a 
shallow copy. This has the effect of biting unsuspecting programmers in 
the ass when implementing their own subclass of NSArray.

* STEPS TO REPRODUCE
1. Run included test program.

* RESULTS
copyWithZone: and mutableCopyWithZone currently (breaking with past 
OpenStep behavior which was widely held as nonsensical) always do a 
shallow copy on all concrete classes provided with MacOSX. However, the 
abstract base classes have an implementation of copyWithZone which does 
a *deep* copy. It should do a shallow copy as all concrete subclasses 
do, so as to not provide unpleasent surprises to subclass implementors. 
Doing a deep copy is almost certainly not something that anyone expects 
to happen, as it is not mentioned in the documentation that this may 
occur.

This bug most likely appears in the abstract base class for NSArray, 
NSDictionary, and NSSet, although I only tested NSArray.

* REGRESSION
Doubtful, most likely simply inherited behavior from OpenStep.

* NOTES
Test program follows. This program demonstrates that a deep copy occurs 
in the abstract class of NSArray but not in the concrete subclasses. 
The output on my machine currently is:

=========== bugtest.m.output ============
Making new array
Cloning array

Making new mutable array
Cloning array

Making new custom singular array
Cloning array
BAD NEWS! Deep copy occured - Foo copyWithZone called!

=============== bugtest.m ================
#import <Foundation/Foundation.h>
@interface Foo : NSObject
@end

@implementation Foo
- copyWithZone: (NSZone *)zone
{
    puts("BAD NEWS! Deep copy occured - Foo copyWithZone called!");
    return self;
}
@end

@interface SingularArray: NSArray
{
    id obj;
}
- initWithObj: newobj;
@end
@implementation SingularArray
- initWithObj: newobj
{
    obj = newobj;
}
- (unsigned)count
{
    return 1;
}

- objectAtIndex: (unsigned)index
{
    if(index == 0)
        return obj;
    return nil;
}
@end
int main()
{
    NSArray *a;
    Foo *f = [[Foo alloc] init];
    puts("Making new array");
    a = [[NSArray alloc] initWithObjects: f, NULL];
    puts("Cloning array");
    [a copy];
    puts("");

    puts("Making new mutable array");
    a = [[NSMutableArray alloc] initWithObjects: f, NULL];
    puts("Cloning array");
    [a copy];
    puts("");

    puts("Making new custom singular array");
    a = [[SingularArray alloc] initWithObj: f];
    puts("Cloning array");
    [a copy];
    puts("");
}