Re: [objc-improvements] Message passing breaks when receiver type doesn't match promoted sender type (Was: How is it looking?)

[Alexander Malmberg]

Ziemowit Laski wrote:
> Ah, good catch!  Thanks. :-)
> 
> The reason I did what I did is that, when constructing ObjC message
> sends, esp. to receivers of type 'id' or 'Class', there may be multiple
> method declarations floating around such that their selectors are the
> same but their arguments/return types differ. :-(  This is a very real
> phenomenon in our (i.e., Apple) frameworks.

I don't recall any instance where this has caused problems in normal
calls in GNUstep; name collisions are fairly rare. There have been
issues with forwarding in user code related to this, though.

> I once timidly proposed
> making ObjC more strict so that all methods sharing a selector also
> agree on argument/return types, but was shot down by our resident ObjC
> deities. :-)

That's understandable. :) OTOH, testing with POC shows that it warns if
there are two selectors with the same name but different types.

> Of course, I do see the nature of your problem -- my patch has
> essentially disabled implicit argument conversions when calling ObjC
> methods.  Without it, though, you often get idiotic warnings or
> unneeded/erroneous conversions.

Not all warnings or conversions or unneeded, though. Thinking more about
this, this seems like a rather deep issue. In particular, the return
value is just as important as the arguments, so any solution needs to
deal with that as well.

> (Perhaps I'd need to add a test case
> that shows what happens when the compiler chooses a "wrong" method to
> cast to.).  It seems we have several possible options here, none
> particularly elegant:
> 
> (1) Leave my patch as is, and force call sites to explicitly cast their
> arguments when needed. :-(
> (2) Conditionalize my patch with -fnext-runtime.  This fix is political
> rather than technical in nature, and is probably the least desirable of
> the three...
> (3) Take your patch, but modify it slightly so that a method_prototype
> is used only if there is exactly one.  If there is more than one, we
> fall back to my method and issue a warning (I believe the warning is
> already issued, at least in some cases).
> 
> What do you think?  I tend to prefer (3)...

Well, stepping back from the implementation for a while and looking at
it from a higher level, it seems we have the following cases:

a. "Type" information for the receiver gives exactly one method
prototype.
b. "Type" information for the receiver gives more than one method
prototype.
c. "Type" information for the receiver gives no method prototype, and
there is more than one method prototype visible.
d. "Type" information for the receiver gives no method prototype, and
there is exactly one method prototype visible.
e. The selector is completely unknown.


(For class objects, replace id with Class.)

Current/old behavior in gcc seems to be:

a. Use it.
b. Pick one randomly. Warn, but only if both prototypes are in the same
@interface/@protocol.
c. If receiver is "id", pick one randomly. Otherwise, assume id return
and ... for arguments. Warn.
d. Use it. Warn if receiver isn't "id".
e. Warn. Assume id return and ... for arguments.


With (1), the warnings remain the same, and argument/return type
becomes:

a-d. ... for arguments. Return type picked from a "random" prototype.
e. As before. ... for arguments, id for return.

Aside from many not so technical issues I have with this, the return
type handling is a big flaw. The elegance of this solution is, I think,
that method prototypes wouldn't matter, and thus the issue of picking a
prototype is avoided. However, this doesn't work for return types, so
the solution fails to avoid and fails to address the core issue.


I also grabbed a copy of POC (3.1.32) and tested. Since there doesn't
seem to be any way of typing an object in POC, ie. all objects are of
type "id", a. and b. don't apply. For the others, I got:

c. Warn. Pick one randomly.
d. Use it.
e. Warn. The warning incorrectly claims that arguments are assumed to be
of type id; they're actually assumed to be ..., and the return type id.


Anyway, that's for reference (and may be slightly inaccurate; there are
many possible ways of triggering each case, and I'm assuming that
they're treated the same). As for what we want the new behavior to be, I
hope we can agree that in a., we should use the prototype. Also, as
guidelines, I propose:

1. If a prototype is used, it's used completely. Thus, if the return
type is taken from a prototype, that argument types are taken from the
same prototype.

2. If a prototype isn't used, the return type is assumed to be id and
the arguments ....

Given this and formalizing (3) a bit, I get:

a. Use it.

b. Warn and pick one at random. This is really a pathological case; the
user is claiming that the receiver has two different prototypes for the
same method. Picking one at random seems like the best option, and would
(arguably) be the Right Thing for minor mismatches (eg. signed vs.
unsigned). I'm not picky about the behavior in this case, though. The
user should simply remove the incorrect prototype.

c. Warn. Use id for return, ... for arguments.

d. Use it. Warn if receiver isn't id. Optionally warn even if receiver
is id (for strict typing fanatics :).

e. Warn. Use id for return, ... for arguments.


I think d. is the hardest case. Using the prototype is nice if it's
correct, but if it isn't, the only way to get around it is to declare a
dummy protocol or something with another prototype to get case c.
However, I think that the single prototype will be correct more often
than not, so it seems reasonable to use it. I can't think of any really
good reason either way, though.

In all cases where we use ... for arguments, it seems like it might be
better to hack things to avoid the type promotion (eg. by constructing a
prototype from the types of the arguments before promotion; I think this
would be possible). Otherwise, it'd be impossible to call methods with
float arguments in c. and e.

We'd probably want to look closely at the warnings if we do this to make
them as consistent and useful as possible.

Comments? Does this seem acceptable?

In the source, I suggest moving method_prototype handling out of
build_objc_method_call(). Instead, finish_message_expr() should always
give build_objc_method_call() a valid prototype (looked up or
constructed).

- Alexander Malmberg