Your 'Class <Protocol>' Work

[David Ayers]

Ziemowit Laski wrote:
> Took me longer than expected, but here is the alternative patch I 
> promised; no ChangeLog entries for now.  It's running a bit late, so 
> I'll defer my commentary until tomorrow. :-(  As always, feedback is 
> welcome.

Hello Zem,

well I guess we have different views on a gratuitous rewriting of
objc_comptypes :-), but your version seems fine.

The test case you attached fails with the patch:
Excess errors:
[...]/class-protocol-1.m:50: warning: `+doItInstance1' not implemented
by protocol(s)
[...]/class-protocol-1.m:92: warning: no `+doItInstance3' method found
[...]/class-protocol-1.m:133: warning: no `+doItInstance5' method found
[...]/class-protocol-1.m:138: warning: `+doItInstance1' not implemented
by protocol(s)
[...]/class-protocol-1.m:195: warning: no `+doItInstance7' method found
[...]/class-protocol-1.m:198: warning: `+doItInstance7' not implemented
by protocol(s)
[...]/class-protocol-1.m:230: warning: no `+doItInstance8' method found
[...]/class-protocol-1.m:232: warning: no `+doItInstance9' method found


You seem to have omitted the part which is /very/ important to me.
Please! search for instance prototypes in protocol qualified Class
references!  What is your issue with this feature?  It is very important
in my view wrt generating the right code.  I do not understand what is
bothering you about it.  I can live with doing it independent of whether
the protocol is adopted by the root class.  But please do it for those
that are.

I admit that my test cases so far haven't covered that.  So here it is:

/*----------------------------------------*/
/* Prototype mismatch  */

@protocol MyOtherProto1
+(id)doItClass1;
-(id)doItInstance1; /* { dg-warning "using" } */
@end
@interface MyOtherClass1 <MyOtherProto1>
@end

Class <MyOtherProto1> oclsP1;

void
testPrototypeMismatch(void)
{
  id tmp1 = [oclsP1 doItClass1];
  id tmp2 = [oclsP1 doItInstance1]; /* { dg-warning "as class method" } */

  [clsP1 doItClass1];
  [clsP1 doItInstance1]; /* { dg-warning "as class method" } */
}

Please insert right after testInheritedNonRoot().  At least after clsP1
is declared.  Note that this is still not perfect as it only tests
whether "someone" ever emits the "using" if the new protocol and doesn't
really match it up due to the fact that wouldn't know how to tell
dejagnu to do the right thing.  But at least we would know both where
being used.  Alternatively all three warnings could be removed if the
correct prototypes are used w/o emitting the diagnostics as your
modified test case suggests.

Some more comments below...

> Index: gcc/testsuite/objc.dg/class-protocol-1.m
> ===================================================================
> RCS file: gcc/testsuite/objc.dg/class-protocol-1.m
> diff -N gcc/testsuite/objc.dg/class-protocol-1.m
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ gcc/testsuite/objc.dg/class-protocol-1.m	5 Oct 2004 02:18:06 -0000
> @@ -0,0 +1,303 @@
> +/* Check Class <protocol> types */
> +/* Author: David Ayers <d.ayers@inode.at> */
> +/* { dg-do compile } */
> +
> +#include <objc/objc.h>
> +#include <objc/objc-api.h>
> +
> +@protocol MyProto1
> ++(void)doItClass1;
> +-(void)doItInstance1;
> +@end
> +
> +@protocol MyProto2
> ++(void)doItClass2;
> +-(void)doItInstance2;
> +@end
> +
> +@interface MyClass1 <MyProto1>
> +{
> +  Class isa;
> +}
> +@end
> +@implementation MyClass1
> ++(void)doItClass1{}
> +-(void)doItInstance1{}
> +@end
> +
> +@interface MyClass2 : MyClass1 <MyProto2>
> +@end
> +@implementation MyClass2
> ++(void)doItClass2{}
> +-(void)doItInstance2{}
> +@end
> +
> +/*----------------------------------------*/
> +
> +Class cls = 0;
> +Class <MyProto1> clsP1 = 0;
> +Class <MyProto2> clsP2 = 0;
> +
> +void
> +testSimple(void)
> +{
> +  [cls doItClass1];
> +  [cls doItInstance1]; /* Do not warn as root Class declares this.  */
> +  [cls doItClass2];
> +  [cls doItInstance2]; /* { dg-warning "no .\\+doItInstance2. method found" } */
> +
> +  [clsP1 doItClass1];
> +  [clsP1 doItInstance1]; /* Do not warn as root Class declares this.  */

This is failure:
[...]/class-protocol-1.m:50: warning: `+doItInstance1' not implemented
by protocol(s)
-doItInstance1 is part of MyProto1 which is adopted by the root class
MyClass1.
Not warning here is fine by but seems inconsistent with warnings below.
 And didn't you suggest a warning about not finding a class method here?

> +  [clsP1 doItClass2];    /* { dg-warning "not implemented by protocol" } */
> +  [clsP1 doItInstance2]; /* { dg-warning "not implemented by protocol" } */
> +
> +  [clsP2 doItClass1];    /* { dg-warning "not implemented by protocol" } */
> +  [clsP2 doItInstance1]; /* { dg-warning "not implemented by protocol" } */
> +  [clsP2 doItClass2];
> +  [clsP2 doItInstance2]; /* { dg-warning "not implemented by protocol" } */
> +
> +  [MyClass1 doItClass1];
> +  [MyClass1 doItInstance1];
> +  [MyClass1 doItClass2];    /* { dg-warning "may not respond to" } */
> +  [MyClass1 doItInstance2]; /* { dg-warning "may not respond to" } */
> +
> +  [MyClass2 doItClass1];
> +  [MyClass2 doItInstance1];
> +  [MyClass2 doItClass2];
> +  [MyClass2 doItInstance2]; /* { dg-warning "may not respond to" } */
> +

Please add the tests from my last mail:
  [MyClass3 doItClass1];    /* { dg-warning "may not respond to" } */
  [MyClass3 doItInstance1]; /* { dg-warning "may not respond to" } */

  [MyClass4 doItClass1];
  [MyClass4 doItInstance1]; /* { dg-warning "may not respond to" } */

> +}
> +
> +/*----------------------------------------*/
> +/* Protocols declared by categories */
> +
> +@protocol MyProto3
> ++(void)doItClass3;
> +-(void)doItInstance3;
> +@end
> +@protocol MyProto4
> ++(void)doItClass4;
> +-(void)doItInstance4;
> +@end
> +
> +@interface MyClass1 (Category1) <MyProto3>
> +@end
> +@interface MyClass2 (Category2) <MyProto4>
> +@end
> +
> +void
> +testCategory(void)
> +{
> +  [cls doItClass3];
> +  [cls doItInstance3];      /* Do not warn as root Class declares this.  */

This is failure:
[...]/class-protocol-1.m:92: warning: no `+doItInstance3' method found

> +  [cls doItClass4];
> +  [cls doItInstance4];      /* { dg-warning "no .\\+doItInstance4. method found" } */
> +
> +  [MyClass1 doItClass3];
> +  [MyClass1 doItInstance3];
> +  [MyClass1 doItClass4];    /* { dg-warning "may not respond" } */
> +  [MyClass1 doItInstance4]; /* { dg-warning "may not respond" } */
> +
> +  [MyClass2 doItClass3];
> +  [MyClass2 doItInstance3];
> +  [MyClass2 doItClass4];
> +  [MyClass2 doItInstance4]; /* { dg-warning "may not respond" } */
> +
> +}
> +
> +/*----------------------------------------*/
> +/* Inherited protocols declared by categories */
> +
> +@protocol MyProto5 <MyProto1>
> ++(void)doItClass5;
> +-(void)doItInstance5;
> +@end
> +
> +@protocol MyProto6 <MyProto2>
> ++(void)doItClass6;
> +-(void)doItInstance6;
> +@end
> +
> +@interface MyClass1 (Category3) <MyProto5>
> +@end
> +@interface MyClass2 (Category4) <MyProto6>
> +@end
> +
> +Class <MyProto5> clsP5 = 0;
> +Class <MyProto6> clsP6 = 0;
> +
> +void
> +testCategoryInherited(void)
> +{
> +  [cls doItClass5];
> +  [cls doItInstance5]; /* Do not warn as root Class declares this.  */
This failure:
[...]/class-protocol-1.m:133: warning: no `+doItInstance5' method found
> +  [cls doItClass6];
> +  [cls doItInstance6]; /* { dg-warning "no .\\+doItInstance6. method found" } */
> +
> +  [clsP5 doItClass1];
> +  [clsP5 doItInstance1];

This is failure:
[...]/class-protocol-1.m:138: warning: `+doItInstance1' not implemented
by protocol(s)

Pretty much the same comment about doItInstance1 as above, so see below.

> +  [clsP5 doItClass2];    /* { dg-warning "not implemented by protocol" } */
> +  [clsP5 doItInstance2]; /* { dg-warning "not implemented by protocol" } */
> +
> +  [clsP6 doItClass1];    /* { dg-warning "not implemented by protocol" } */
> +  [clsP6 doItInstance1]; /* { dg-warning "not implemented by protocol" } */
> +  [clsP6 doItClass2];
> +  [clsP6 doItInstance2]; /* { dg-warning "not implemented by protocol" } */
> +
> +  [MyClass1 doItClass5];
> +  [MyClass1 doItInstance5]; /* Do not warn as root Class declares this.  */
> +  [MyClass1 doItClass6];    /* { dg-warning "may not respond" } */
> +  [MyClass1 doItInstance6]; /* { dg-warning "may not respond" } */
> +
> +  [MyClass2 doItClass5];
> +  [MyClass2 doItInstance5]; /* Do not warn as root Class declares this.  */
> +  [MyClass2 doItClass6];
> +  [MyClass2 doItInstance6]; /* { dg-warning "may not respond" } */
> +
> +}
> +
> +/*----------------------------------------*/
> +/* Forward declared root protocols */
> +
> +@protocol FwProto;
> +
> +@interface MyClass1 (Forward) <FwProto>
> +@end
> +
> +Class <FwProto> clsP7 = 0;
> +
> +void
> +testForwardeDeclared1(void)
> +{
> +  [cls doItClass7];         /* { dg-warning "no .\\+doItClass7. method found" } */
> +  [cls doItInstance7];      /* { dg-warning "no .\\+doItInstance7. method found" } */
> +
> +  [clsP7 doItClass7];       /* { dg-warning "not implemented by protocol" } */
> +  [clsP7 doItInstance7];    /* { dg-warning "not implemented by protocol" } */
> +
> +  [MyClass1 doItClass7];    /* { dg-warning "may not respond" } */
> +  [MyClass1 doItInstance7]; /* { dg-warning "may not respond" } */
> +
> +  [MyClass2 doItClass7];    /* { dg-warning "may not respond" } */
> +  [MyClass2 doItInstance7]; /* { dg-warning "may not respond" } */
> +
> +}
> +
> +@protocol FwProto
> ++(void)doItClass7;
> +-(void)doItInstance7;
> +@end
> +
> +void
> +testForwardeDeclared2(void)
> +{
> +  [cls doItClass7];
> +  [cls doItInstance7];   /* Do not warn as root Class declares this.  */
This is failure:
[...]/class-protocol-1.m:195: warning: no `+doItInstance7' method found

> +
> +  [clsP7 doItClass7];    
> +  [clsP7 doItInstance7]; /* Do not warn as root Class declares this.  */
This is failure:
[...]/class-protocol-1.m:198: warning: `+doItInstance7' not implemented
by protocol(s)

And again, see below.

> +
> +  [MyClass1 doItClass7];
> +  [MyClass1 doItInstance7];
> +
> +  [MyClass2 doItClass7];
> +  [MyClass2 doItInstance7];
> +}
> +
> +/*----------------------------------------*/
> +/* Inherited non root protocols */
> +
> +@protocol MyProto8
> ++(void)doItClass8;
> +-(void)doItInstance8;
> +@end
> +
> +@protocol MyProto9 <MyProto8>
> ++(void)doItClass9;
> +-(void)doItInstance9;
> +@end
> +
> +@interface MyClass1 (InheritedNonRoot) <MyProto9>
> +@end
> +
> +Class <MyProto8> clsP8 = 0;
> +Class <MyProto9> clsP9 = 0;
> +
> +void
> +testInheritedNonRoot(void)
> +{
> +  [cls doItClass8];
> +  [cls doItInstance8]; /* Do not warn as root Class declares super.  */

This is failure:
[...]/class-protocol-1.m:230: warning: no `+doItInstance8' method found

> +  [cls doItClass9];
> +  [cls doItInstance9]; /* Do not warn as root Class declares this.  */
This is failure:
[...]/class-protocol-1.m:232: warning: no `+doItInstance9' method found

> +
> +  [clsP8 doItClass8];
> +  [clsP8 doItInstance8]; /* { dg-warning "not implemented by protocol" } */


This one is the debatable/pathological case:  The root class MyClass1
adopts MyProto9 which inherits MyProto8.  In that sense we have a good
guess at which prototype to use.  But I'm fine with this yet we should
add a comment the test case.

> +  [clsP8 doItClass9];    /* { dg-warning "not implemented by protocol" } */
> +  [clsP8 doItInstance9]; /* { dg-warning "not implemented by protocol" } */
> +
> +  [clsP9 doItClass8];
> +  [clsP9 doItInstance8]; /* { dg-warning "not implemented by protocol" } */

Here is what I've been referring to about being inconsistent.  MyClass1
is a root class that adopts MyProto9 which inherits MyProto8 and there
for should not warn as mentioned above, or it should only warn that it
doesn't find the class method but is using the prototype of the instance
method.

> +  [clsP9 doItClass9];
> +  [clsP9 doItInstance9]; /* { dg-warning "not implemented by protocol" } */

Again, MyClass1 is a root class that adopts MyProto9 directly and should
not warn as mentioned above.

> +
> +  [MyClass1 doItClass8];
> +  [MyClass1 doItInstance8]; /* Do not warn as root Class declares this.  */
> +  [MyClass1 doItClass9];
> +  [MyClass1 doItInstance9];
> +
> +  [MyClass2 doItClass8];
> +  [MyClass2 doItInstance8]; /* Do not warn as root Class declares this.  */
> +  [MyClass2 doItClass9];
> +  [MyClass2 doItInstance9];
> +  
> +}
> +
> +id obj = nil;
> +id <MyProto1> objP1 = nil;
> +id <MyProto2> objP2 = nil;
> +
> +MyClass1 *mc1 = nil;
> +MyClass2 *mc2 = nil;
> +
> +void
> +testComptypes(void)
> +{
> +  cls == clsP1;
> +  clsP1 == cls;
> +
> +  cls == objP1; /* { dg-warning "lacks a cast" } */
> +  objP1 == cls; /* { dg-warning "lacks a cast" } */
> +
> +  clsP1 == clsP5;
> +  clsP5 == clsP1;
> +
> +  mc1 == clsP1; /* { dg-warning "lacks a cast" } */
> +  clsP1 == mc1; /* { dg-warning "lacks a cast" } */
> +
> +
> +  cls = clsP1; 
> +  clsP1 = cls;
> +
> +  cls = objP1; /* { dg-warning "incompatible" } */
> +  objP1 = cls; /* { dg-warning "incompatible" } */
> +
> +  clsP1 = clsP5;
> +  clsP5 = clsP1; /* { dg-warning "does not conform" } */
> +
> +  mc1 = clsP1; /* { dg-warning "incompatible" } */
> +  clsP1 = mc1; /* { dg-warning "incompatible" } */
> +
> +}
> +
> +int main ()
> +{
> +  testSimple();
> +  testCategory();
> +  testCategoryInherited();
> +  return(0);
> +}
> +
> +/* { dg-warning "Messages without a matching" "" { target *-*-* } 47 } */
> +/* { dg-warning "will be assumed to return" "" { target *-*-* } 47 } */
> +/* { dg-warning "as arguments" "" { target *-*-* } 47 } */