Fix for PR objc/48539 (Missing warning when messaging a forward-declared class)

2011-06-02 Thread Nicola Pero
This patch fixes PR objc/48539 (Missing warning when messaging a 
forward-declared class).

The problem occurs when using @class, and then messaging class or instance 
objects of the class.
It can happen both with class and instance methods.

--

An example with class methods is

 @class MyClass;

 [MyClass method];

In this example, the compiler has no information on 'MyClass' and on what 
methods it responds to.
So, there is no way to determine if MyClass responds to the method +method, 
and what the method
prototype is (in this example it doesn't matter, but if there are arguments or 
return values, it could
matter a lot, including potential causing a crash at runtime if the wrong 
method prototype is used).
Clang emits a warning there, which seems very appropriate, and with this patch, 
GCC 4.7.0 emits a warning
there too. :-)

--

Then there is then the issue of what to do about instance methods, as in the 
following example --

 @class MyClass;

 MyClass *x;
 [x method];

This is almost identical to the case above, and this patch adds a similar 
warning. ;-)

Note that in this case, the current behaviour of the compiler is substandard; 
the compiler silently throws away
the MyClass * type, silently casts x to id, and proceeds to accept for it 
to be used as a receiver
of any possible method of any class, without any warning (!!).  clang does the 
same by the way.

We clearly do want to emit a warning there, instead.  The fact that the 
programmer has explicitly declared x to be
of type MyClass * instead of id means she is expecting the compiler to use 
that information to do the
standard method lookup/check based on the class.  If the @interface is missing, 
it is most likely an error / slip
in the program, which is worthwhile for the compiler to warn about (in the same 
way as we warn above for class
methods!). ;-)

So this patch changes this behaviour and adds a warning here; if the programmer 
doesn't want the warning and
is happy with x being treated as an id, she can simply add a cast to id 
to clarify her mind, and the
warning will go away.  Ie. if you do

 @class MyClass;

 MyClass *x;
 [(id)x method];

you don't get any warning as you explicitly disabled type-based checks by 
casting to id.  But if you leave x to be
of type MyClass *, then you're asking for type-based checks / method lookup, 
the compiler will try to do the method lookup,
and if the @interface of MyClass was not found, will emit a warning because 
it can't do the requested type-based checks /
method lookup. ;-)

I tried this patch with gnustep core and it did find a number of slips in the 
code, which is good.  No major
bugs, but all cases where someone had forgotten to #include the header with the 
@interface of a class and so
where the compiler couldn't do the proper checks, but nobody would notice 
because of the current silent
behaviour where the missing @interface causes the variable to magically and 
silently become of type id.

In fact, looking at the examples is very convincing that we need this warning.  
Without it, @class NSArray; basically
makes NSArray * a typedef for id when doing method invocations.  So, in 
your code you may have methods or functions
taking (NSArray *) arguments, and then you call methods on these arguments, 
expecting the compiler to check that the methods
are appropriate for an NSArray.  Instead, the compiler is silently treating 
NSArray * as identical to id, and performing
no checks whatsoever, and not bothering to tell you anything about the fact! ;-)

--

Finally, there's the additional complication of deciding what to do when this 
is mixed with protocols, as in --

 @class MyClass;
 @protocol MyProtocol
 - (void) method;
 @end

 MyClass MyProtocol *x;
 [x method];

This is a weird/rare case, and I don't expect to see it much in practice, but 
it needs to be sorted out, and GCC already
has at least two existing testcases for it in the GCC testsuite.  At the 
moment, the compiler does the equivalent of
silently converting MyClass MyProtocol * to id MyProtocol.  That's not 
great, but I pondered about this for
a long time, tried a few variations, and then decided to make no changes. :-)

The reason is that if the method being called is part of the protocol, then the 
compiler can find the method prototype
(and do the type-checking) without needing any more information on the actual 
class.  Complaining about the @interface not
being available seems pointless nit-picking, since it's not required to do the 
type-based method lookup. ;-)

If the method being called is not part of the protocol, the compiler already 
emits a warning that the method could not be
found in the protocol.  I thought about adding a second warning about the 
@interface not being found, and for a while had
it in the patch, but in practice it seemed overkill and I removed it from the 
final version.

--

The warning message that I chose for GCC is --

  method-lookup-1.m:42:3: warning: @interface of class ‘NotKnown’ 

Re: Fix for PR objc/48539 (Missing warning when messaging a forward-declared class)

2011-06-02 Thread Mike Stump
On Jun 2, 2011, at 11:29 AM, Nicola Pero wrote:
 This patch fixes PR objc/48539 (Missing warning when messaging a 
 forward-declared class).

 Ok to commit to trunk ?

Ok.