The static analyzer should warn when +initialize doesn't call [super initialize]

[Quuxplusone]

Bugzilla Link	PR9144
Status	NEW
Importance	P enhancement
Reported by	Ken Case (kc@omnigroup.com)
Reported on	2011-02-04 18:52:44 -0800
Last modified on	2013-12-19 10:09:27 -0800
Version	unspecified
Hardware	Macintosh MacOS X
CC	kc@omnigroup.com, keith@33software.com, kremenek@apple.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Overview:

The static analyzer should warn when +initialize doesn't call [super initialize], since that can lead to inconsistent and unreliable behavior with respect to initializing a class.

Discussion:

One should always call [super initialize] because that's what happens if you don't implement +initialize--and you shouldn't change a subclass' behavior when you implement a subclassed method.

In Objective C, a class' +initialize is supposed to be called for itself and for every subclass which is initialized.  This is the implemented and documented behavior of the runtime.

This is an important feature of the language:  the superclass may need to register some information for each of its subclasses, e.g. to set up class-specific mapping dictionaries, or to register CoreData change notifications for dependent keys.   The superclass doesn't and shouldn't know which subclasses are non-implementing and the subclasses shouldn't need to know what initialization the superclass needs to do, the superclass should be able to do class-specific initialization for each of its subclasses.

I can think of no benefit to not calling [super initialize] except to avoid some slight overhead--the same benefit one might achieve by not calling [super init] (which also seems like terrible general advice).  And there would be a big downside in terms of consistency of behavior between classes which do or don't implement +initialize and consistency of implementation between the way you might implement +initialize and the way you would implement other methods.

Instead, as a general rule, one should know that any time they subclass any method they should call the superclass' implementation (unless they are intentionally trying to bypass/replace the logic provided by the superclass).  This is just as true for +initialize as it is for any other method.

[Quuxplusone]

Is this part of the Cocoa conventions?

[Quuxplusone]

(In reply to comment #1)
> Is this part of the Cocoa conventions?

Yes.  I'm including some sample code which demonstrates the convention (run
with "cc -o /tmp/InitializeTest InitializeTest.m -framework Foundation &&
/tmp/InitializeTest").  Note that AnySubclass, MyGoodSubclass, and
MyBlissfulSubclass all log invocations from [MySuperclass initialize], while
MyForgetfulSubclass does not.  (Since [AnySubclass self] is called first, it
actually logs twice: once with self == MySuperclass and once with self ==
AnySubclass.)

I believe this has been the convention since 1989; or at least since 1993, when
the behavior was documented in "Object-Oriented Programming and the Objective C
Language" (by NeXT Publications):  The runtime generates a single +initialize
for each class before sending it any other messages.  The +initialize message
is sent whether or not a subclass implements it, so when implementing
+initialize you can expect to be called once for each subclass.  (Unless, of
course, the subclass fails to call [super initialize]!)  The 1993 documentation
for +initialize included two techniques for protecting code that really only
wants to be called once (testing self to see if it's being called for the main
class rather than a subclass; or testing a static initialization variable).

That said, I should note that the current NSObject class documentation has
edited the description of +initialize slightly since that 1993 publication:  it
shows just one technique for guarding code you want to run once (rather than
two)—and unfortunately it now also suggests that +initialize shouldn't
typically call [super initialize] "since the runtime sends appropriate
initialize messages automatically." I'm not quite sure when this advice was
introduced into the documentation; in 2007 I filed RADAR 5480173 pointing out
that that's a departure from the actual convention and suggesting they correct
the advice.  (That suggestion was introduced relatively recently; the
aforementioned 1993 documentation for +initialize reads largely the same
otherwise.)

Failing to call [super initialize] leads to inconsistent and sometimes
difficult to track down results where a subclass will stop working when someone
introduces a new +initialize method which doesn't call [super initialize].
(This is much the same problem as when a subclass fails to call [super init].)
My hope is that adding a warning will help prevent many of those bugs!

                Thanks!
                Ken

#import <Foundation/Foundation.h>

@interface MySuperclass : NSObject
@end

@interface AnySubclass : MySuperclass
@end

@interface MyGoodSubclass : MySuperclass
@end

@interface MyForgetfulSubclass : MySuperclass
@end

@interface MyBlissfulSubclass : MySuperclass
@end

@implementation MySuperclass

+ (void)initialize;
{
    NSLog(@"MySuperclass: +[%@ %@] begin", NSStringFromClass(self), NSStringFromSelector(_cmd));
    [super initialize];
    NSLog(@"MySuperclass: +[%@ %@] end", NSStringFromClass(self), NSStringFromSelector(_cmd));
}

@end

@implementation AnySubclass
@end

@implementation MyGoodSubclass

+ (void)initialize;
{
    NSLog(@"MyGoodSubclass: +[%@ %@] begin", NSStringFromClass(self), NSStringFromSelector(_cmd));
    [super initialize];
    NSLog(@"MyGoodSubclass: +[%@ %@] end", NSStringFromClass(self), NSStringFromSelector(_cmd));
}

@end

@implementation MyForgetfulSubclass

+ (void)initialize;
{
    NSLog(@"MyForgetfulSubclass +[%@ %@] begin", NSStringFromClass(self), NSStringFromSelector(_cmd));
    NSLog(@"MyForgetfulSubclass +[%@ %@] end", NSStringFromClass(self), NSStringFromSelector(_cmd));
}

@end

@implementation MyBlissfulSubclass
@end

int main(int argc, const char *argv[])
{
    NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
    [AnySubclass self];
    [MyGoodSubclass self];
    [MyForgetfulSubclass self];
    [MyBlissfulSubclass self];
    [pool release];
    return 0;
}

[Quuxplusone]

P.S. — To be clear, I'm not trying to stir up anything; I've always thought that calling [super initialize] was as non-controversial as calling [super init]. My documentation RADAR hasn't seen any feedback, but I've been assuming that just means it's a low priority. If there /is/ controversy over this convention, I'm happy to participate in dialog about that in a more appropriate forum (if I can figure out where that should be!).

[Quuxplusone]

Not being a big Objective-C programmer myself, I was mainly wondering if this
*was* documented somewhere.  I can readily loop in the AppKit guys to confirm
the convention, and discuss it with them.

You mentioned a documentation radar; do you have the number on hand?

[Quuxplusone]

(In reply to comment #4)
> Not being a big Objective-C programmer myself, I was mainly wondering if this
> *was* documented somewhere.  I can readily loop in the AppKit guys to confirm
> the convention, and discuss it with them.

That sounds great.  Thank you!

> You mentioned a documentation radar; do you have the number on hand?

Yes, it's <rdar://5480173>.

                Thanks again,
                Ken

[Quuxplusone]

cloned to rdar://problem/10502477

[Quuxplusone]

I just received notification that rdar://problem/5480173 has finally been fixed: the documentation for +initialize now indicates that it's appropriate to call [super initialize].

It would be great if the static analyzer would add an option to warn developers when they forget to call [super initialize] from +initialize, since this can be a source of subtle errors!