This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Your 'Class <Protocol>' Work



On 30 Sep 2004, at 5.25, David Ayers wrote:


I apologize for not realizing this earlier; I kept thinking of when
to issue the warning 'Class <Proto1, Proto2> may not respond to
"bar"' and when to suppress it. A brief lecture of objc-act.c now
reminded me that such a warning should _never_ be issued for
receivers of type 'id' or 'Class', whether protocol-qualified or
not.

OK, now we either have a fundamental disagreement or a fundamental misunderstanding. If I send a message to a protocol qualified 'id' which is not declared by the protocol, I expect a warning. Why else should I bother qualifying it? Or what do you mean with "that such a warning should _never_ be issued for receivers of type 'id' or 'Class', _whether protocol-qualified or not_"?

Hopefully this is just a misunderstanding, and not even a fundamental one. :-)
When we say "warning", we are in fact talking about two different warnings:


   (1)  'Foo' may not respond to 'bar'
   (2)  'bar' not implemented by protocol(s)

In the paragraph you quoted, I was saying that warning (1) should never
be issued, but warning (2) should and is.  The distinction between these
two warnings is crucial, since they have different semantics and impose
different requirements on the implementation to produce them.  I seem to
have completely forgotten about this distinction, until my epiphany.


For the 'Class' case the situation a bit more hairy, due to the semantics of instance methods in root classes. What I'm trying to achieve is that:

Class <NSObject> clsP1;
Class <NSCoding> clsP2;

[clsP1 retain]; /* doesn't warn */

Unless the NSObject protocol realy does provide +retain, then the compiler _should_ warn in this case.

[clsP2 encodeWithCoder: coder]; /* warns */

Now if you are saying that you think it's OK, to warn in the +retain
case, (which is what I  understand from the second to last paragraph)
then, yes, we wouldn't need to mark root protocols at all.

Exactly.


But as the
correct retrieval of that prototype and the suppression of that warning
is _exactly_ what I'm trying to achieve, I have written the code to mark
those protocols and to test that mark when searching for protocols.

Ok, I guess we have a disagreement after all. :-( As you said above,
the very reason you'd want to protocol-qualify 'id' or 'Class' in the
first place is so that the compiler could warn you if you're invoking
a method _not_ listed in those protocols. The machinery you're trying
to put in place will defeat this mechanism; why would you want to do this?


If you do not want to see a warning in this case, you can use either
(1) an unadorned 'Class' or (2) a concrete ObjC class (e.g., 'NSObject')
as the receiver. In both cases, the compiler will look at instance methods
as a last resort.


This is especially important when the prototypes start mismatching which
was part of the original trigger for me to do this at all.

I don't understand this last sentence at all. :-( Can you show an example of when "prototypes start mismatching"? Perhaps this will help me understand your motivation a bit better.

As you suggested, I think I will whip up a separate patch that incorporates
(and probably modifies) your testcases to show you what I mean.


Thanks,

--Zem



In summation, I believe that the only necessary changes to objc_finish_message_expr() are the following (I've attached it inline to preserve tabulation):



---------------------------------------------------------------------- --

Index: gcc/objc/objc-act.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/objc/objc-act.c,v
retrieving revision 1.248
diff -u -3 -p -r1.248 objc-act.c
--- gcc/objc/objc-act.c	24 Sep 2004 23:15:33 -0000	1.248
+++ gcc/objc/objc-act.c	30 Sep 2004 01:13:57 -0000
@@ -5570,7 +5570,8 @@ objc_finish_message_expr (tree receiver,
       else
 	{
 	  class_tree = objc_class_name;
-	  OBJC_SET_TYPE_NAME (rtype, class_tree);
+	  rprotos = TYPE_PROTOCOL_LIST (rtype);
+	  rtype = NULL_TREE;
 	}

       if (rprotos)
@@ -5650,10 +5651,19 @@ objc_finish_message_expr (tree receiver,
 		 IDENTIFIER_POINTER (OBJC_TYPE_NAME (rtype)),
 		 (class_tree ? '+' : '-'),
 		 IDENTIFIER_POINTER (sel_name));
+      /* If we are messaging an unadorned 'id' or 'Class' object,
+	 then we have failed to find _any_ instance or class method,
+	 respectively.  */
+      else if (!rprotos)
+	warning ("no `%c%s' method found",
+		 (class_tree ? '+' : '-'),
+		 IDENTIFIER_POINTER (sel_name));
+
       if (rprotos)
 	warning ("`%c%s' not implemented by protocol(s)",
 		 (class_tree ? '+' : '-'),
 		 IDENTIFIER_POINTER (sel_name));
+		
       if (!warn_missing_methods)
 	{
 	  warning ("(Messages without a matching method signature");


---------------------------------------------------------------------- --

I tried it anyway and it fails to find the prototypes. Since you have
the patch and test case maybe you can play a bit, to show me what you mean.


At this point, I'm wondering whether (1)
PROTOCOL_ADOPTED_BY_ROOT_CLASS is needed at all and (2) if instance
methods of protocols adopted by root classes should be placed in the
class method hash table. Doing so will make no difference whatsoever
when messaging 'Class <ProtoList>', but may help out when messaging
an unadorned 'Class'. What do you think? I have yet to develop a
strong opinion here... :-)

As noted above, it does make a difference. Or I'm totally missing your point.

Perhaps instead of 'incorporated', we should consistently say
'inherited'?

Actually "incorporated" is the term used in the "Objective-C" book.


http://developer.apple.com/documentation/Cocoa/Conceptual/ObjectiveC/
 LanguageSummary/chapter_5_section_8.html And the term "inherited"
is a bit misleading. The concept doesn't really match protocols.


Why not? If you think of protocols as abstract interfaces, then
"inherit" fits the bill perfectly, I would think... Personally, I
find "incorporate" a bit vague, since it could mean that you're
manually repeating the method declarations from the other
protocol...

Well I'm not going to argue heavily about this. I think "repeating the method declarations" is exactly what it does, but then again "inheritance" repeats instance variables, method declarations and implementation. There is a difference to Objective-C class inheritance in that you can declare "multiple inheritance" wrt to protocol declarations. I just had the feeling that the term when applied to protocols was causing confusion. I'm still partial to term "incorporated" now, but if you'd like me to revert my documentation to speak of inheritance, I will.

BTW, :-) If you feel strongly about conceptually representing protocols
as a second set of inheritance hierarchies, then maybe we should
reconsider whether:

@protocol MyProto8
+(void)doItClass8;
-(void)doItInstance8;
@end
@protocol MyProto9 <MyProto8>
+(void)doItClass9;
-(void)doItInstance9;
@end

where MyProto9 is a "subprotocol" of MyProto8

@interface SomeRoot <MyProto9>
@end

Class <MyProto8> clsP8 = 0;

[clsP8 doItInstance8];

should warn, or not, as strictly speaking it's only the "sub protocol"
that has been declared by a root class and not MyProto8. But like I
said the case is extreme and we save ourselves a lot of complexity by
not handling it. I just mentioned it again to show the potential
expectation if you view protocols as (mulitple inheritance) hierarchies,
as I also did, which lead to my first implementation.


...
[sniped sly remark]
You're not being constructive here. :-(

Sorry, it was intended to be a little pinch, not a slap.


If, as you say, the term "inherited" is misleading, then it
definitely _is_ interesting to see what term the Java-ites settled on
 in order to maintain this distinction and avoid confusion. Just as
Java "inherited" (pun intended) the notion of protocols/interfaces
from ObjC, so we can definitely borrow their terminology if we find
it useful.

<joke>I do Objective-C, I don't believe in multiple inheritance. (not implying that Java has it) :-) Well in this case it would even be "recursive".</joke>

No, seriously, if the Java folks have a term for this, let's hear it.

Hmm... I guess my comment was motivated by the belief that 'Class
 <Proto>' is virtually identical to 'id <Proto>' as far as type
comparisons are concerned, and the few differences (if any) could
easily be special-cased. Do you disagree?

Well yes, I agree that they are virtually identical, which is why it was so easy to "write" objc_comptypes_proto_proto. Maybe I should have mentioned that this is a verbatim copy if the previuos handling in the "Protocol" = "Protocol" case, plus the assertion that both parameters must either be 'id' or both be 'Class'.


OK, but then objc_comptypes_proto_proto() is nothing more than a
gratuitous rewrite of objc_comptypes(), which I thought we agreed
should wait until later... :-)


Actually, objc_comptypes_proto_proto is merely an extraction of a part
of objc_comptypes to avoid code duplication and avoid rearranging the
flow control of objc_comptypes. There is no change in the real code, it
just cut & paste and reformatted to take the nesting level into account.
That doesn't fight my definition of a gratuitous rewrite.


I'm curious... if you were just to augment the definition of
IS_PROTOCOL_QUALIFIED_ID to encompass 'Class <Proto>' constructs, but
left objc_comptypes() in its original form, which comparisons would
break as a result?

objc_comptypes is currently structured like:


if (lhs is pointer to structure && rhs is pointer to structure)
  {
    if (lhs is protocol qualified 'id')
      {
        if (rhs is protocol qualified 'id')
          { Handle "id <protocol> = id <protocol>" case. }
        else if (rhs is concrete class reference)
          { Handle "id <protocol> = SomeClass *" case. }
        else if (rhs is 'id')
          { Handle "id <protocol> = id" case. }
        else if (rhs is 'Class')
          { Handle "id <protocol> = Class" case. }
        else
          { Defer handling to comptypes. }
      }
    else if (rhs is protocol qualified 'id')
      {
        if (rhs is concrete class reference)
          { Handle "SomeClass * = id <protocol>" case. }
        else if (rhs is 'id')
          { Handle "id = id <protocol>" case. }
        else if (rhs is 'Class')
          { Handle "Class = id <protocol>" case. }
        else
          { Defer handling to comptypes. }
       }
     else
       { strip pointer from lhs and rhs }
  }

...
What I've done is:

if (lhs is pointer to structure && rhs is pointer to structure)
{
if (lhs is protocol qualified 'id')
{
if (rhs is protocol qualified 'id')
* { Handle "id <protocol> = id <protocol>" case. }
else if (rhs is concrete class reference)
{ Handle "id <protocol> = SomeClass *" case. }
else if (rhs is 'id')
{ Handle "id <protocol> = id" case. }
else if (rhs is 'Class')
{ Handle "id <protocol> = Class (& Class <protocol>)" case. }
else
{ Defer handling to comptypes. }
}
else if (rhs is protocol qualified 'id')
{
if (rhs is concrete class reference)
{ Handle "SomeClass * = id <protocol>" case. }
else if (rhs is 'id')
{ Handle "id = id <protocol>" case. }
else if (rhs is 'Class')
{ Handle "Class (& Class <protocol>) = id <protocol> " case. }
else
{ Defer handling to comptypes. }
}
+ else if (lhs is protocol qualified 'Class')
+ {
+ if (rhs is protocol qualified 'Class')
+* { Handle "Class <protocol> = Class <protocol>" case. }
+ else if (rhs is concrete class reference)
+ { Handle "Class <protocol> = SomeClass *" case. }
+ else if (rhs is 'id')
+ { Handle "Class <protocol> = id" case. }
+ else if (rhs is 'Class')
+ { Handle "Class <protocol> = Class" case. }
+ else
+ { Defer handling to comptypes. }
+ }
+ else if (rhs is protocol qualified 'Class')
+ {
+ if (rhs is concrete class reference)
+ { Handle "SomeClass * = Class <protocol>" case. }
+ else if (rhs is 'id')
+ { Handle "id = Class <protocol>" case. }
+ else if (rhs is 'Class')
+ { Handle "Class = Class <protocol>" case. }
+ else
+ { Defer handling to comptypes. }
+ }
else
{ strip pointer from lhs and rhs }
}


with the * being the extracted code in objc_comptypes_proto_proto.

What I /think/ you are asking me to do is:

if (lhs is pointer to structure && rhs is pointer to structure)
  {
    if (lhs is protocol qualified 'id' or 'Class')
      {
        if (rhs is protocol qualified 'id' or 'Class')
(1)       { Handle "id/Class <protocol> = id/Class <protocol>" case. }
        else if (rhs is concrete class reference)
(2)       { Handle "id/Class <protocol> = SomeClass *" case. }
        else if (rhs is 'id')
          { Handle "id/Class <protocol> = id" case. }
        else if (rhs is 'Class')
(3)       { Handle "id/Class <protocol> = Class" case. }
        else
          { Defer handling to comptypes. }
      }
    else if (rhs is protocol qualified 'id' or 'Class')
      {
        if (rhs is concrete class reference)
(4)       { Handle "SomeClass * = id/Class <protocol>" case. }
        else if (rhs is 'id')
          { Handle "id = id/Class <protocol>" case. }
        else if (rhs is 'Class')
(5)        { Handle "Class = id/Class <protocol>" case. }
        else
          { Defer handling to comptypes. }
       }
     else
       { strip pointer from lhs and rhs }
  }

(1) Here we would either have to special case:
Class <protocol> = id <protocol> (and the inverse)
to returns 0 (not compatible) without the need to checking protocol
conformance.

(2) Here we would either have to special case:
Class <protocol> = SomeClass *
To return 0 (not compatible) instead of looking for the protocol in the
class.

(3) Here we would either have to special case:
Class = id <protocol>
which returns 0 (not compatible)
Class = Class <protocol>
should return 1.

(4) Inversion of (2)
(5) Inversion of (3)

And it's this special casing that would disrupt the flow, at least IMO.

Actually, if my hunch above is correct (and I hope it is :-), you
 shouldn't need to modify lookup_method_in_protocol_list() at
all, and you will not need instance_prototype_of_root_protocol()
either since lookup_method_in_hash_lists() should find the
instance methods for you. Hopefully, the sole use of the
PROTOCOL_ADOPTED_BY_ROOT_CLASS flag will henceforth be to
indicate that a protocol's instance methods were put into the
global class method hash table.

I don't think it worth loosing that type information. We need to account for several root classes each possibly adopting different protocols.


But lookup_method_static() has access to this information as is; see
below.

@protocol MyProto8
+(void)doItClass8;
-(void)doItInstance8;
@end

@protocol MyProto9 <MyProto8>
+(void)doItClass9;
-(void)doItInstance9;
@end

@interface MyClass1 <MyProto9>
@end

Class <MyProto8> clsP8 = 0;

...
 [clsP8 doItInstance8]; /* { dg-warning "not implemented" } */

Now, does the fact that a root class adopts MyProto9, which
incorporates MyProto8, make MyProto8 a "root" protocol in the
sense that we should search for instance methods when sending
messages to Class <Proto8> ?
I admit this is a corner case that may not warrant the extra
complexity. This updated patch would make MyProto8 a full
fledged "root" protocol.

I agree; since MyProto9 is a "root protocol", then so is MyProto8, and so the warning above should not be generated. Yes, this is overkill, since we don't know if 'clsP8' points at a 'MyClass1' or not, but this is the same overkill as we already have with root classes themselves, as mentioned above.

Well just because a class conforms to MyProto8 does not mean it is a class who's root class conforms to MyProto9 so we are being rather lax but the case can be a bit pathalogical and the complexity not worth the trouble.


Yes, I humbly reverse my previous opinion; see above.

Umm, I'm lost. So is your opinion that MyProto "shouldn't" be a "root"
class? Or are you just suggesting, that with the above assertion, that
marking root protocols is superfluous? Which I hope I have shown that
it isn't, that it is in effect very important wrt to "prototype hygiene"
to quote you :-).


I did add a flag to lookup_method_in_protocol_list to indicate
whether instance methods of protocols adopted by root classes
should be searched, so the caller can decide. This is necessary
as we only want to use this feature if we actually have
'Class', not when we have a "concrete" MyClass4 that is not a
root class but implements a protocol that is a root class of a
different hierarchy:

@protocol MyProto1
@end

You mean


  @protocol MyProto1
  -doItInstance1;
  @end

right?

Indeed :-) This was from the test case.



@interface MyClass1 <MyProto1> /* root */
@end
@interface MyClass2 : MyClass1
@end
@interface MyClass3 /* New root */
@end
@interface MyClass4 : MyClass3 <MyProto1>
@end

[MyClass2 doItInstance1]; /* Do not warn */
...
[MyClass4 doItInstance1]; /* { dg-warning "may not respond to" } */



In this case, the lookup should be performed by lookup_method_static(), which I believe already does what it should. I agree with you that the warning _is_ needed in the second case, but lookup_method_static() should _not_ be able to stumble upon MyProto1 while searching for +doItInstance1 in MyClass4; if it does somehow find it, then I think it's buggy. :-( So again, I don't think you need to do anything beyond what's already being done to handle this case.

Yet lookup_method_static uses lookup_method_in_protocol_list, so that is exactly the place where I need to tell it to act differently :-)


I think, though, that when lookup_method_static() reaches the top of the
hierarchy when searching for class methods, it will search for instance
methods in the root class _and_ its associated protocols as a last gasp,
no?

Exactly! But it is _not_ supposed to look into the protocols adopted by
unrelated root classes, this what that flag suppresses here.


Cheers,
David



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]