Objective-C really strong type checking

Hi All

Is there a check implemented in Clang Checker (maybe not released for XCode yet) that would find errors like in example:

@interface SomeClass : NSObject {
NSMutableDictionary *_someObjects;
}
@end

@implementation SomeClass

  • (id)init {
    if ((self = [super init])) {
    _someObject = [NSMutableArray new];
    }
    return self;
    }
    @end

Methods ‘new’ or ‘init’ return object of type ‘id’ by convention, but as I experienced it myself it can be unsafe.

Cheers
Marcin

This isn't implemented, but it would be a great check and would be really easy to do.

Hi All

Is there a check implemented in Clang Checker (maybe not released for XCode yet) that would find errors like in example:

@interface SomeClass : NSObject {
NSMutableDictionary *_someObjects;
}
@end

@implementation SomeClass
- (id)init {
if ((self = [super init])) {
   _someObject = [NSMutableArray new];
}
return self;
}
@end

Methods 'new' or 'init' return object of type 'id' by convention, but as I experienced it myself it can be unsafe.

Cheers
Marcin

This isn't implemented, but it would be a great check and would be really easy to do.

Problem with this is that this is very common form of assignments in objc and checker will get very noisy.

- Fariborz

2010/9/8 Fariborz Jahanian <fjahanian@apple.com>

No it won't. Assignments to a distinct Objective-C type are very rare. The issue here is that +new (along with (+alloc, +allocWithZone:, -init, and so on) is declared on NSObject and not overridden. When you send a +alloc message to almost any class, you are calling the implementation in NSObject, with the class as self. This implementation then allocates enough space for an instance of the receiver, initalizes the isa pointer, and returns.

The problem, from an analysis point of view, is that the Objective-C type system is not expressive enough to tell you what is really going on here. The +new method always returns an instance of the receiver (or, in some special cases such as class clusters, a subclass of the receiver).

Although it would be possible to add a specific hack for +new into the analyser, it would be cleaner to solve this in the general case by providing two annotations, one saying that the return type of a class method is an instance of the receiver and another saying that the return type of an instance method is the receiver.

The latter would also be useful for -retain, -autorelease, and all of the various -initWith{something}: methods in Cocoa. For example, the initialisers in NSArray return a mutable array when the receiver is an instance of NSMutableArray (or a subclass), while the constructors return a mutable array when the receiver is NSMutableArray.

David

-- Sent from my STANTEC-ZEBRA

2010/9/8 Fariborz Jahanian <fjahanian@apple.com>

Hi All

Is there a check implemented in Clang Checker (maybe not released for XCode yet) that would find errors like in example:

@interface SomeClass : NSObject {
NSMutableDictionary *_someObjects;
}
@end

@implementation SomeClass

  • (id)init {
    if ((self = [super init])) {
    _someObject = [NSMutableArray new];
    }
    return self;
    }
    @end

Methods ‘new’ or ‘init’ return object of type ‘id’ by convention, but as I experienced it myself it can be unsafe.

Cheers
Marcin

This isn’t implemented, but it would be a great check and would be really easy to do.

Problem with this is that this is very common form of assignments in objc and checker will get very noisy.

  • Fariborz

Maybe you missed the point: there’s NSMutableArray object assigned to attribute of NSMutableDictionary type, and those types have completly different interfaces. If the programmer would realy want this, he could use attribute of type ‘id’ and live with possibility of runtime errors.

So, this is more than just warn on assigning an ‘id’ expression to a static type. Checker should treat [NSMutableArray new] specially and see how it gets used.
Thanks for the explanation.

  • Fariborz

I think this case is probably useful enough that we'd go ahead and
special-case based on naming conventions for -init..., just like the
retain-release checking. The analyzer doesn't care about
implementations, since for non-class methods it can't know about them
anyway. (And after all, if someone wanted to be pathological they could
switch out the implementation of the +allocWithZone: method at runtime.)

On the other hand, factory methods like +[NSArray arrayWithObject:]
pose a bigger problem, since there's not really a checkable convention
for the factory method name. So for completeness' sake, we might want
such an annotation, just like attribute((ns_returns_retained)). But it'd
be nice to have this check automagically work for init methods, even
without annotations (again like ns_returns_retained).

As for the "this method returns the receiver" annotation, this is
important for -retain and -autorelease, but not very common in Cocoa
otherwise. Admittedly, there are libraries other than Cocoa, and the
annotation could be shared with C++ as well. But again, we might want to
special-case -retain and -autorelease (and IIRC we are) since they show
up so often, even when the annotation is "missing".

Of course, right now the handling of -retain and -autorelease is bound
up in CFRefCount.cpp, which needs to be broken down and rewritten to use
the regular Checker interface. I think Ted's planning this eventual
rewrite/refactor.

Jordy

Maybe you missed the point: there's NSMutableArray object assigned to
attribute of NSMutableDictionary type, and those types have completly
different interfaces. If the programmer would realy want this, he could use
attribute of type 'id' and live with possibility of runtime errors.

So, this is more than just warn on assigning an 'id' expression to a static
type. Checker should treat [NSMutableArray new] specially and see how it
gets used.

I think it's about assigning a specified object type (which +new and
-init... are by convention) to a non-id variable that's not derivative
of the object type. NSMutableArray "is-not-a" NSMutableDictionary, but
by convention +new and -init... return id to mean an instance of this
type or a derived type (or possibly something that proxies to either).

/Jesper
[accidentally sent this to Fariborz and not to the list]