Patchwork ObjC/ObjC++ - detect when protocol qualifier used with non-Objective-C object type

login
register
mail settings
Submitter Nicola Pero
Date Nov. 12, 2010, 7:39 p.m.
Message ID <1289590762.980826762@192.168.2.227>
Download mbox | patch
Permalink /patch/71010/
State New
Headers show

Comments

Nicola Pero - Nov. 12, 2010, 7:39 p.m.
This patch adds a missing check for invalid usage of Objective-C protocol qualifiers, making
our compiler consistent with the Apple one in rejecting protocol qualifiers used with non-Objective-C
object types.

The master example of this is the following Objective-C snippet:

  @protocol MyProtocol;
  typedef int Integer;
  Integer <MyProtocol> *variable;

This code is invalid - "<MyProtocol>" is the "protocol qualifier" syntax used in Objective-C
to specify a type of an Objective-C object or class which, in addition to the specified class, conforms
to the protocol MyProtocol.  But "variable" is an "int *", which is not an Objective-C object (you can't
use it for ObjC messaging), hence it makes no sense to say that it conforms to a @protocol (since conforming
to a @protocol means you can use it for ObjC messaging with the ObjC methods defined in the @protocol).

But without this patch, GCC accepts that invalid code and doesn't even warn!  It just silently discards
the <MyProtocol> bit.  And by the way, it also ignores the actual protocol name, so if you put a non-existing
protocol name in there, it still compiles it and does not even emit a warning ;-)

This patch fixes this case and has GCC detect such cases, and generate an error for them.  The error 
I chose for this is

invalid-type-1.m:21:1: error: only Objective-C object types can be qualified with a protocol

which I hope is clear enough for users, while still being reasonably correct.

Unfortunately, there is a complication, which is that our own FSF GCC Objective-C testsuite had a testcase 
(objc/compile/20060406-1.m) that contained precisely the invalid syntax that this patch is meant
to detect and reject (!!). (PS: The Apple llvm-gcc testsuite does not have that testcase).  I suspect
the invalid syntax was used in there in an attempt to write the testcase without including objc/objc.h.
Anyway I changed the testcase removing the invalid syntax and replacing it with a few valid combinations 
of typedefs and protocol qualifiers which I felt were worth testing instead. :-)

I'd like to mention that Apple llvm-gcc happens to have the exact opposite testcase, which, like my new
testcases, tests precisely that you can't use a protocol qualifier with a non-Objective-C object type.  So 
it's not just my interpretation - it is consistent with the Apple interpretation :-)

Ok to commit to trunk ?

Thanks
Mike Stump - Nov. 13, 2010, 3:05 p.m.
On Nov 12, 2010, at 11:39 AM, "Nicola Pero" <nicola.pero@meta-innovation.com> wrote:
> This patch adds a missing check for invalid usage of Objective-C protocol qualifiers, making
> our compiler consistent with the Apple one in rejecting protocol qualifiers used with non-Objective-C
> object types.

> Ok to commit to trunk ?

Ok.
>

Patch

Index: objc/objc-act.c
===================================================================
--- objc/objc-act.c     (revision 166670)
+++ objc/objc-act.c     (working copy)
@@ -2530,7 +2530,22 @@  objc_get_protocol_qualified_type (tree interface,
                  : xref_tag (RECORD_TYPE, type));
        }
       else
-        return interface;
+       {
+         /* This case happens when we are given an 'interface' which
+            is not a valid class name.  For example if a typedef was
+            used, and 'interface' really is the identifier of the
+            typedef, but when you resolve it you don't get an
+            Objective-C class, but something else, such as 'int'.
+            This is an error; protocols make no sense unless you use
+            them with Objective-C objects.  */
+         error_at (input_location, "only Objective-C object types can be qualified with a protocol");
+
+         /* Try to recover.  Ignore the invalid class name, and treat
+            the object as an 'id' to silence further warnings about
+            the class.  */
+         type = objc_object_type;
+         is_ptr = true;
+       }
     }
 
   if (protocols)
Index: objc/ChangeLog
===================================================================
--- objc/ChangeLog      (revision 166670)
+++ objc/ChangeLog      (working copy)
@@ -1,3 +1,9 @@ 
+2010-11-12  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc-act.c (objc_get_protocol_qualified_type): detect cases
+       where we are asked to attach a protocol to something which is not
+       an Objective-C object type, and produce an error.
+
 2010-11-11  Nicola Pero  <nicola.pero@meta-innovation.com>
 
        * objc-act.c (objc_add_property_declaration): Check that the type
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 166670)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,12 @@ 
+2010-11-12  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc/compile/20060406-1.m: Fixed testcase not to try to qualify
+       a pointer to an arbitrary C struct with an Objective-C protocol.
+       Test various valid uses of typedef with Objective-C objects and
+       protocols instead.
+       * objc.dg/invalid-type-1.m: New.
+       * obj-c++.dg/invalid-type-1.m: New.     
+       
 2010-11-12  James Dennett <jdennett@google.com>
 
        PR c++/39415
@@ -114,6 +123,7 @@ 
        * gfortran.dg/proc_decl_24.f90: New.
 
 2010-11-11  Nicola Pero  <nicola.pero@meta-innovation.com>
+
        * objc.dg/property/at-property-20.m: New.
        * objc.dg/property/synthesize-8.m: New. 
        * obj-c++.dg/property/at-property-20.m: New.
Index: testsuite/objc.dg/invalid-type-1.m
===================================================================
--- testsuite/objc.dg/invalid-type-1.m  (revision 0)
+++ testsuite/objc.dg/invalid-type-1.m  (revision 0)
@@ -0,0 +1,24 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, November 2010.  */
+/* { dg-do compile } */
+#include <objc/objc.h>
+
+typedef int Integer;
+
+@class MyClass;
+
+typedef MyClass AClass;
+
+@protocol MyProtocol
+- (void) method;
+@end
+
+Class <MyProtocol> class_object; /* This is fine.  */
+
+id <MyProtocol> object; /* This is fine.  */
+
+AClass <MyProtocol> *object1; /* This is fine.  */
+
+Integer <MyProtocol> *object2; /* { dg-error "only Objective-C object types can be qualified with a protocol" } */
+
+Integer <NonExistingProtocol> *object3; /* { dg-error "only Objective-C object types can be qualified with a protocol" } */
+/* { dg-error "cannot find protocol" "" { target *-*-* } 23 } */
Index: testsuite/obj-c++.dg/invalid-type-1.mm
===================================================================
--- testsuite/obj-c++.dg/invalid-type-1.mm      (revision 0)
+++ testsuite/obj-c++.dg/invalid-type-1.mm      (revision 0)
@@ -0,0 +1,25 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, November 2010.  */
+/* { dg-do compile } */
+#include <objc/objc.h>
+
+typedef int Integer;
+
+@class MyClass;
+
+typedef MyClass AClass;
+
+@protocol MyProtocol
+- (void) method;
+@end
+
+Class <MyProtocol> class_object; /* This is fine.  */
+
+id <MyProtocol> object; /* This is fine.  */
+
+AClass <MyProtocol> *object1; /* This is fine.  */
+
+Integer <MyProtocol> *object2; /* { dg-error ".Integer. is not a template" } */
+/* { dg-error ".MyProtocol. was not declared in this scope" "" { target *-*-* } 21 } */
+
+Integer <NonExistingProtocol> *object3; /* { dg-error ".Integer. is not a template" } */
+/* { dg-error ".NonExistingProtocol. was not declared in this scope" "" { target *-*-* } 24 } */
Index: testsuite/objc/compile/20060406-1.m
===================================================================
--- testsuite/objc/compile/20060406-1.m (revision 166670)
+++ testsuite/objc/compile/20060406-1.m (working copy)
@@ -1,21 +1,65 @@ 
-typedef struct
-{
-  void *p;
-} *S;
+/* This test tests typedefs and protocol qualifiers.  */
 
 @protocol O
 - (unsigned)j;
 @end
 
+@interface T
+@end
+
+
+/* First test.  */
+typedef T<O> *S;
+
 @interface I
-+ (unsigned char)T:(S<O>[2])p v:(S<O>)h;
++ (unsigned char)T:(S[2])p
+                 v:(S)h;
 @end
 
 @implementation I
-+ (unsigned char)T:(S<O>[2])p v:(S<O>)h
++ (unsigned char)T:(S[2])p
+                 v:(S)h
 {
   p[0] = (S) 0;
   p[1] = (S) 0;
   return 0;
 }
 @end
+
+
+/* Second test.  */
+typedef T<O> S1;
+
+@interface I1
++ (unsigned char)T1:(S1*[2])p 
+                 v1:(S1*)h;
+@end
+
+@implementation I1
++ (unsigned char)T1:(S1*[2])p
+                 v1:(S1*)h
+{
+  p[0] = (S1*) 0;
+  p[1] = (S1*) 0;
+  return 0;
+}
+@end
+
+
+/* Third test.  */
+typedef T S2;
+
+@interface I2
++ (unsigned char)T1:(S2<O>*[2])p 
+                 v1:(S2<O>*)h;
+@end
+
+@implementation I2
++ (unsigned char)T1:(S2<O>*[2])p
+                 v1:(S2<O>*)h
+{
+  p[0] = (S2<O>*) 0;
+  p[1] = (S2<O>*) 0;
+  return 0;
+}
+@end