diff mbox

ObjC/ObjC++: extend fix of PR objc/47076 ("Protocol referenced in @interface declarations should be defined")

Message ID B20A289A-0CC2-4D01-976C-B5E329872160@meta-innovation.com
State New
Headers show

Commit Message

Nicola Pero Dec. 31, 2010, 11:15 a.m. UTC 
This patch is a sophistication of the fix to PR objc/47076, and fixes  
a slightly more complicated case.

In objc/47076, we fixed the detection that a protocol was only forward- 
declared, ie, the testcase

@protocol A;

@interface MyClass <A>
@end

this testcase should cause a warning, because protocol A is forward- 
declared, ie, it has been declared
that A is a protocol without providing the list of methods required/ 
optional for it; but when A is used in the
declaration of MyClass, the compiler needs the list of methods to  
properly process MyClass to do all the
checks and things required.  So, the compiler does (now) correctly  
warn in this case.

What we didn't cover was the complication

@protocol A;

@protocol B <A>
- (void)method;
@end

@interface MyClass <B>
@end

In this case, protocol B, which is directly referenced by MyClass, is  
a normal, declared protocol with
a list of methods.  But, it is also specified that B conforms to A,  
which is another protocol, this time
forward-declared.  So, again, the compiler won't have the full list of  
methods required/optional for the
protocols that MyClass is declared to implement, and won't be able to  
perform all the expected checks
and stuff on MyClass.  It should, again, warn.  But our fix to objc/ 
47076 didn't cover this case; it only
covered protocols directly referenced by the declaration.

This patch extends the fix to these cases by doing the recursive check  
required, so we now warn
in this case too.  The warning/checks actually become much more useful  
in practice too :-).  Testcases
included.

Ok to commit ?

Thanks

Comments

Mike Stump Dec. 31, 2010, 6:58 p.m. UTC | #1 
On Dec 31, 2010, at 3:15 AM, Nicola Pero wrote:
> This patch is a sophistication of the fix to PR objc/47076, and fixes a slightly more complicated case.

> Ok to commit ?

Ok.
diff mbox

Patch

Index: objc/objc-act.c
===================================================================
--- objc/objc-act.c     (revision 168362)
+++ objc/objc-act.c     (working copy)
@@ -10929,11 +10929,30 @@  add_protocol (tree protocol)
    return protocol_chain;
  }

+/* Check that a protocol is defined, and, recursively, that all
+   protocols that this protocol conforms to are defined too.  */
+static void
+check_that_protocol_is_defined (tree protocol)
+{
+  if (!PROTOCOL_DEFINED (protocol))
+    warning (0, "definition of protocol %qE not found",
+            PROTOCOL_NAME (protocol));
+
+  /* If the protocol itself conforms to other protocols, check them
+     too, recursively.  */
+  if (PROTOCOL_LIST (protocol))
+    {
+      tree p;
+
+      for (p = PROTOCOL_LIST (p); p; p = TREE_CHAIN (p))
+       check_that_protocol_is_defined (TREE_VALUE (p));
+    }
+}
+
  /* Looks up a protocol.  If 'warn_if_deprecated' is true, a warning is
     emitted if the protocol is deprecated.  If 'definition_required' is
     true, a warning is emitted if a full @protocol definition has not
     been seen.  */
-
  static tree
  lookup_protocol (tree ident, bool warn_if_deprecated, bool  
definition_required)
  {
@@ -10951,9 +10970,8 @@  lookup_protocol (tree ident, bool warn_i
                      PROTOCOL_NAME (chain));
           }

-       if (definition_required && !PROTOCOL_DEFINED (chain))
-         warning (0, "definition of protocol %qE not found",
-                  PROTOCOL_NAME (chain));
+       if (definition_required)
+         check_that_protocol_is_defined (chain);

         return chain;
        }
Index: objc/ChangeLog
===================================================================
--- objc/ChangeLog      (revision 168362)
+++ objc/ChangeLog      (working copy)
@@ -1,3 +1,8 @@ 
+2010-12-31  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc-act.c (check_that_protocol_is_defined): New.
+       (lookup_protocol): Call check_that_protocol_is_defined.
+
  2010-12-30  Nicola Pero  <nicola.pero@meta-innovation.com>

         * objc-act.c (objc_types_are_equivalent): Fixed comparing  
protocol
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 168362)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,10 @@ 
+2010-12-31  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc.dg/protocol-forward-1.m: Removed TODO.
+       * objc.dg/protocol-forward-2.m: New.
+       * obj-c++.dg/protocol-forward-2.mm: Removed TODO.
+       * obj-c++.dg/protocol-forward-2.mm: New.
+
  2010-12-30  Nicola Pero  <nicola.pero@meta-innovation.com>

         * objc.dg/method-conflict-3.m: New.
Index: testsuite/objc.dg/protocol-forward-2.m
===================================================================
--- testsuite/objc.dg/protocol-forward-2.m      (revision 0)
+++ testsuite/objc.dg/protocol-forward-2.m      (revision 0)
@@ -0,0 +1,95 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>,  
December 2010.  */
+/* { dg-do compile } */
+
+/* Test that all protocols appearing in @interface declarations are
+   real (ie, we saw a full @protocol definition with list of methods),
+   and not just forward-references (ie, "@protocol NSObject;").  This
+   test checks protocols implemented by other protocols.  */
+
+#include <objc/objc.h>
+
+@protocol MyProtocol;
+
+@interface MyClass <MyProtocol> /* { dg-warning "definition of  
protocol .MyProtocol. not found" } */
+@end
+
+
+@protocol MyProtocol2 <MyProtocol>
+- (int)method2;
+@end
+
+@interface MyClass2 <MyProtocol2> /* { dg-warning "definition of  
protocol .MyProtocol. not found" } */
+- (int)method2;
+@end
+
+
+@protocol MyProtocol3 <MyProtocol2>
+- (int)method3;
+@end
+
+@interface MyClass3 <MyProtocol3>  /* { dg-warning "definition of  
protocol .MyProtocol. not found" } */
+- (int)method2;
+- (int)method3;
+@end
+
+
+@protocol MyProtocol4 <MyProtocol3, MyProtocol2>
+- (int)method4;
+@end
+
+@interface MyClass4 <MyProtocol4>  /* { dg-warning "definition of  
protocol .MyProtocol. not found" } */
+- (int)method2;
+- (int)method3;
+- (int)method4;
+@end
+
+
+@protocol MyProtocol5
+- (int)method5;
+@end
+
+@interface MyClass5 <MyProtocol5> /* Ok */
+- (int)method5;
+@end
+
+
+@protocol MyProtocol6 <MyProtocol5>
+- (int)method6;
+@end
+
+@interface MyClass6 <MyProtocol6> /* Ok */
+- (int)method5;
+- (int)method6;
+@end
+
+
+@protocol MyProtocol7 <MyProtocol5, MyProtocol4>
+- (int)method7;
+@end
+
+@interface MyClass7 <MyProtocol7> /* { dg-warning "definition of  
protocol .MyProtocol. not found" } */
+- (int)method2;
+- (int)method3;
+- (int)method4;
+- (int)method5;
+- (int)method7;
+@end
+
+
+/* Now test that if we finally define MyProtocol, the warnings go  
away.  */
+@protocol MyProtocol
+- (int)method;
+@end
+
+@protocol MyProtocol8 <MyProtocol5, MyProtocol4>
+- (int)method8;
+@end
+@interface MyClass8 <MyProtocol8> /* Ok */
+- (int)method;
+- (int)method2;
+- (int)method3;
+- (int)method4;
+- (int)method5;
+- (int)method8;
+@end
Index: testsuite/objc.dg/protocol-forward-1.m
===================================================================
--- testsuite/objc.dg/protocol-forward-1.m      (revision 168362)
+++ testsuite/objc.dg/protocol-forward-1.m      (working copy)
@@ -24,7 +24,3 @@ 

  @protocol MyProtocol3 <MyProtocol> /* Ok */
  @end
-
-/* TODO: I guess if MyProtocol3 is now used in an @interface, we
-   should check that all the protocols it references are defined
-   too ?  */
Index: testsuite/obj-c++.dg/protocol-forward-2.mm
===================================================================
--- testsuite/obj-c++.dg/protocol-forward-2.mm  (revision 0)
+++ testsuite/obj-c++.dg/protocol-forward-2.mm  (revision 0)
@@ -0,0 +1,95 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>,  
December 2010.  */
+/* { dg-do compile } */
+
+/* Test that all protocols appearing in @interface declarations are
+   real (ie, we saw a full @protocol definition with list of methods),
+   and not just forward-references (ie, "@protocol NSObject;").  This
+   test checks protocols implemented by other protocols.  */
+
+#include <objc/objc.h>
+
+@protocol MyProtocol;
+
+@interface MyClass <MyProtocol> /* { dg-warning "definition of  
protocol .MyProtocol. not found" } */
+@end
+
+
+@protocol MyProtocol2 <MyProtocol>
+- (int)method2;
+@end
+
+@interface MyClass2 <MyProtocol2> /* { dg-warning "definition of  
protocol .MyProtocol. not found" } */
+- (int)method2;
+@end
+
+
+@protocol MyProtocol3 <MyProtocol2>
+- (int)method3;
+@end
+
+@interface MyClass3 <MyProtocol3>  /* { dg-warning "definition of  
protocol .MyProtocol. not found" } */
+- (int)method2;
+- (int)method3;
+@end
+
+
+@protocol MyProtocol4 <MyProtocol3, MyProtocol2>
+- (int)method4;
+@end
+
+@interface MyClass4 <MyProtocol4>  /* { dg-warning "definition of  
protocol .MyProtocol. not found" } */
+- (int)method2;
+- (int)method3;
+- (int)method4;
+@end
+
+
+@protocol MyProtocol5
+- (int)method5;
+@end
+
+@interface MyClass5 <MyProtocol5> /* Ok */
+- (int)method5;
+@end
+
+
+@protocol MyProtocol6 <MyProtocol5>
+- (int)method6;
+@end
+
+@interface MyClass6 <MyProtocol6> /* Ok */
+- (int)method5;
+- (int)method6;
+@end
+
+
+@protocol MyProtocol7 <MyProtocol5, MyProtocol4>
+- (int)method7;
+@end
+
+@interface MyClass7 <MyProtocol7> /* { dg-warning "definition of  
protocol .MyProtocol. not found" } */
+- (int)method2;
+- (int)method3;
+- (int)method4;
+- (int)method5;
+- (int)method7;
+@end
+
+
+/* Now test that if we finally define MyProtocol, the warnings go  
away.  */
+@protocol MyProtocol
+- (int)method;
+@end
+
+@protocol MyProtocol8 <MyProtocol5, MyProtocol4>
+- (int)method8;
+@end
+
+@interface MyClass8 <MyProtocol8> /* Ok */
+- (int)method;
+- (int)method2;
+- (int)method3;
+- (int)method4;
+- (int)method5;
+- (int)method8;
+@end
Index: testsuite/obj-c++.dg/protocol-forward-1.mm
===================================================================
--- testsuite/obj-c++.dg/protocol-forward-1.mm  (revision 168362)
+++ testsuite/obj-c++.dg/protocol-forward-1.mm  (working copy)
@@ -25,6 +25,3 @@ 
  @protocol MyProtocol3 <MyProtocol> /* Ok */
  @end

-/* TODO: I guess if MyProtocol3 is now used in an @interface, we
-   should check that all the protocols it references are defined
-   too ?  */