From patchwork Thu Dec 30 19:52:37 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicola Pero X-Patchwork-Id: 77014 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id EC4F9B70B3 for ; Fri, 31 Dec 2010 06:53:04 +1100 (EST) Received: (qmail 3880 invoked by alias); 30 Dec 2010 19:53:02 -0000 Received: (qmail 3860 invoked by uid 22791); 30 Dec 2010 19:53:00 -0000 X-SWARE-Spam-Status: No, hits=-1.4 required=5.0 tests=AWL, BAYES_00, TW_BJ, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (140.186.70.10) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 30 Dec 2010 19:52:46 +0000 Received: from eggs.gnu.org ([140.186.70.92]:39750) by fencepost.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1PYOYC-0003nW-RD for gcc-patches@gnu.org; Thu, 30 Dec 2010 14:52:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PYOYE-0008Hj-6m for gcc-patches@gnu.org; Thu, 30 Dec 2010 14:52:43 -0500 Received: from smtp131.iad.emailsrvr.com ([207.97.245.131]:47172) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PYOYE-0008Hc-3I for gcc-patches@gnu.org; Thu, 30 Dec 2010 14:52:42 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp43.relay.iad1a.emailsrvr.com (SMTP Server) with ESMTP id 489A92D1093 for ; Thu, 30 Dec 2010 14:52:41 -0500 (EST) Received: by smtp43.relay.iad1a.emailsrvr.com (Authenticated sender: nicola.pero-AT-meta-innovation.com) with ESMTPA id A0EC72D0FC9 for ; Thu, 30 Dec 2010 14:52:40 -0500 (EST) Message-Id: <026A5088-5956-4411-B508-4F16B8A289F5@meta-innovation.com> From: Nicola Pero To: gcc-patches@gnu.org Mime-Version: 1.0 (Apple Message framework v936) Subject: ObjC/ObjC++: Another fix for detection of duplicate methods Date: Thu, 30 Dec 2010 20:52:37 +0100 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org This patch fixes comparison of protocol qualifier lists in types when the compiler is trying to detect duplicate methods in an interface, category or protocol. The main case that didn't work is -- @protocol A @end @protocol B @end @interface MyClass - (void) method: (id )x; - (void) method: (id )x; @end without this patch, GCC incorrectly thinks that the two methods are identical including the types (!!) because of a bug in the protocol comparison code, and does not emit any warning or error. (also note that if you swap the order of the two methods, GCC suddenly realizes that they are + +/* - (void) gMethod: (id )x; dg-error "previous declaration" */ +/* - (void) gMethod: (id )x; dg-error "duplicate declaration" */ + +/* - (void) hMethod: (id )x; dg-error "previous declaration" */ +/* - (void) hMethod: (id )x; dg-error "duplicate declaration" */ +@end different and produces an error). ;-) The other case that didn't work is a bit more subtle, and I found it in the clang testsuite (this is a shrinked down testcase of my own, but the concept is the same): @protocol A; @interface MyClass - (void) method: (id )x; - (void) method: (id )x; @end in this testcase, they seem to assume that "id " is a valid type (and, in fairness, GCC accepts it as valid too) and (obviously) identical to "id " and hence the two methods have the same types and so no warning or error should be generated. But GCC would get confused by the duplicated protocol qualifier; it would compare the length of the protocol list between "id " and "id ", and conclude they are different types, and produce an error. That is a bug - if we accept duplicated protocols in protocol qualifier lists (which we have always done and I guess we'll keep doing - clang does it too) then we need to be able to compare them properly. Anyway, this patch fixes the comparison of protocol lists in all these cases (testcases included). With this patch, it all seems to work quite well. Ok to commit ? Thanks Index: objc/objc-act.c =================================================================== --- objc/objc-act.c (revision 168350) +++ objc/objc-act.c (working copy) @@ -11925,9 +11925,8 @@ start_method_def (tree method) really_start_method (objc_method_context, parm_info); } -/* Return 1 if TYPE1 is equivalent to TYPE2 - for purposes of method overloading. */ - +/* Return 1 if TYPE1 is equivalent to TYPE2 for purposes of method + overloading. */ static int objc_types_are_equivalent (tree type1, tree type2) { @@ -11941,6 +11940,7 @@ objc_types_are_equivalent (tree type1, t if (TYPE_MAIN_VARIANT (type1) != TYPE_MAIN_VARIANT (type2)) return 0; + /* Compare the protocol lists. */ type1 = (TYPE_HAS_OBJC_INFO (type1) ? TYPE_OBJC_PROTOCOL_LIST (type1) : NULL_TREE); @@ -11948,14 +11948,34 @@ objc_types_are_equivalent (tree type1, t ? TYPE_OBJC_PROTOCOL_LIST (type2) : NULL_TREE); - if (list_length (type1) == list_length (type2)) + /* If there are no protocols (most common case), the types are + identical. */ + if (type1 == NULL_TREE && type2 == NULL_TREE) + return 1; + + /* If one has protocols, and the other one hasn't, they are not + identical. */ + if ((type1 == NULL_TREE && type2 != NULL_TREE) + || (type1 != NULL_TREE && type2 == NULL_TREE)) + return 0; + else { - for (; type2; type2 = TREE_CHAIN (type2)) - if (!lookup_protocol_in_reflist (type1, TREE_VALUE (type2))) + /* Else, both have protocols, and we need to do the full + comparison. It is possible that either type1 or type2 + contain some duplicate protocols in the list, so we can't + even just compare list_length as a first check. */ + tree t; + + for (t = type2; t; t = TREE_CHAIN (t)) + if (!lookup_protocol_in_reflist (type1, TREE_VALUE (t))) + return 0; + + for (t = type1; t; t = TREE_CHAIN (t)) + if (!lookup_protocol_in_reflist (type2, TREE_VALUE (t))) return 0; + return 1; } - return 0; } /* Return 1 if TYPE1 has the same size and alignment as TYPE2. */ Index: objc/ChangeLog =================================================================== --- objc/ChangeLog (revision 168350) +++ objc/ChangeLog (working copy) @@ -1,5 +1,12 @@ 2010-12-30 Nicola Pero + * objc-act.c (objc_types_are_equivalent): Fixed comparing protocol + lists. Check them two-ways to fix comparisons when one protocol + implements the other one, or when one list contains duplicated + protocols. + +2010-12-30 Nicola Pero + * objc-act.c (objc_add_method): When emitting an error because a method with the same name but conflicting types is found in the same class or category interface, print a note with the location Index: testsuite/ChangeLog =================================================================== --- testsuite/ChangeLog (revision 168350) +++ testsuite/ChangeLog (working copy) @@ -1,4 +1,11 @@ 2010-12-30 Nicola Pero + + * objc.dg/method-conflict-3.m: New. + * objc.dg/method-conflict-4.m: New. + * obj-c++.dg/method-conflict-3.m: New. + * obj-c++.dg/method-conflict-4.mm: New. + +2010-12-30 Nicola Pero * objc.dg/class-extension-3.m: Updated. * objc.dg/method-1.m: Updated. Index: testsuite/objc.dg/method-conflict-3.m =================================================================== --- testsuite/objc.dg/method-conflict-3.m (revision 0) +++ testsuite/objc.dg/method-conflict-3.m (revision 0) @@ -0,0 +1,63 @@ +/* Contributed by Nicola Pero , December 2010. */ +/* { dg-do compile } */ + +#include + +/* Test that the compiler can correctly compare protocols in types of + method signatures. */ + +@protocol A, B, C; + +@interface MyClass +- (void) method1: (id)x; +- (void) method1: (id)x; /* Ok */ + +- (void) method2: (id )x; +- (void) method2: (id )x; /* Ok */ + +- (void) method3: (id )x; +- (void) method3: (id )x; /* Ok */ + +- (void) method4: (id )x; +- (void) method4: (id )x; /* Ok */ + +- (void) method5: (id )x; +- (void) method5: (id )x; /* Ok */ + +- (void) method6: (id )x; +- (void) method6: (id )x; /* Ok */ + +- (void) method7: (id)x; /* { dg-message "previous declaration" } */ +- (void) method7: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) method8: (id )x; /* { dg-message "previous declaration" } */ +- (void) method8: (id)x; /* { dg-error "duplicate declaration" } */ + +- (void) method9: (id )x; /* { dg-message "previous declaration" } */ +- (void) method9: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) methodA: (id )x; /* { dg-message "previous declaration" } */ +- (void) methodA: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) methodB: (id )x; /* { dg-message "previous declaration" } */ +- (void) methodB: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) methodC: (id )x; /* { dg-message "previous declaration" } */ +- (void) methodC: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) methodD: (id )x; /* { dg-message "previous declaration" } */ +- (void) methodD: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) methodE: (MyClass *)x; /* { dg-message "previous declaration" } */ +- (void) methodE: (MyClass *)x; /* { dg-error "duplicate declaration" } */ + +- (void) methodF: (MyClass *)x; +- (void) methodF: (MyClass *)x; /* Ok */ + +- (void) methodG: (MyClass *)x; /* { dg-message "previous declaration" } */ +- (void) methodG: (MyClass *)x; /* { dg-error "duplicate declaration" } */ + +- (void) methodH: (MyClass *)x; /* { dg-message "previous declaration" } */ +- (void) methodH: (MyClass *)x; /* { dg-error "duplicate declaration" } */ + +@end Index: testsuite/objc.dg/method-conflict-4.m =================================================================== --- testsuite/objc.dg/method-conflict-4.m (revision 0) +++ testsuite/objc.dg/method-conflict-4.m (revision 0) @@ -0,0 +1,47 @@ +/* Contributed by Nicola Pero , December 2010. */ +/* { dg-do compile } */ + +#include + +/* Test that the compiler can correctly compare protocols in types of + method signatures. In this test we look at protocols implementing + other protocols. The fact that one protocol implements another one + doesn't mean that they are identical. */ + +@protocol A +- (void) doSomething; +@end + +@protocol B +- (void) doSomethingElse; +@end + +@protocol C +- (void) doYetSomethingElse; +@end + +@interface MyClass2 +- (void) aMethod: (id )x; /* { dg-message "previous declaration" } */ +- (void) aMethod: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) bMethod: (id )x; /* { dg-message "previous declaration" } */ +- (void) bMethod: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) cMethod: (id )x; +- (void) cMethod: (id )x; /* Ok - because if you implement B, then you also implement A, so == */ + +- (void) dMethod: (id )x; +- (void) dMethod: (id )x; /* Ok */ + +- (void) eMethod: (id )x; /* { dg-message "previous declaration" } */ +- (void) eMethod: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) fMethod: (id )x; /* { dg-message "previous declaration" } */ +- (void) fMethod: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) gMethod: (id )x; /* { dg-message "previous declaration" } */ +- (void) gMethod: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) hMethod: (id )x; /* { dg-message "previous declaration" } */ +- (void) hMethod: (id )x; /* { dg-error "duplicate declaration" } */ +@end Index: testsuite/obj-c++.dg/method-conflict-3.mm =================================================================== --- testsuite/obj-c++.dg/method-conflict-3.mm (revision 0) +++ testsuite/obj-c++.dg/method-conflict-3.mm (revision 0) @@ -0,0 +1,65 @@ +/* Contributed by Nicola Pero , December 2010. */ +/* { dg-do compile } */ + +#include + +/* Test that the compiler can correctly compare protocols in types of + method signatures. */ + +@protocol A, B, C; + +@interface MyClass +- (void) method1: (id)x; +- (void) method1: (id)x; /* Ok */ + +- (void) method2: (id )x; +- (void) method2: (id )x; /* Ok */ + +- (void) method3: (id )x; +- (void) method3: (id )x; /* Ok */ + +- (void) method4: (id )x; +- (void) method4: (id )x; /* Ok */ + +- (void) method5: (id )x; +- (void) method5: (id )x; /* Ok */ + +- (void) method6: (id )x; +- (void) method6: (id )x; /* Ok */ + +- (void) method7: (id)x; /* { dg-warning "previous declaration" } */ +- (void) method7: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) method8: (id )x; /* { dg-warning "previous declaration" } */ +- (void) method8: (id)x; /* { dg-error "duplicate declaration" } */ + +- (void) method9: (id )x; /* { dg-warning "previous declaration" } */ +- (void) method9: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) methodA: (id )x; /* { dg-warning "previous declaration" } */ +- (void) methodA: (id )x; /* { dg-error "duplicate declaration" } */ + +/* FIXME: Bug in the testsuite - the following are done Ok by the compiler, but + the testsuite barfs so we have to comment out the tests. */ +/* - (void) methodB: (id )x; dg-warning "previous declaration" */ +/* - (void) methodB: (id )x; dg-error "duplicate declaration" */ + +/* - (void) methodC: (id )x; dg-warning "previous declaration" */ +/* - (void) methodC: (id )x; dg-error "duplicate declaration" */ + +/* - (void) methodD: (id )x; dg-warning "previous declaration" */ +/* - (void) methodD: (id )x; dg-error "duplicate declaration" */ + +/* - (void) methodE: (MyClass *)x; dg-warning "previous declaration" */ +/* - (void) methodE: (MyClass *)x; dg-error "duplicate declaration" */ + +- (void) methodF: (MyClass *)x; +- (void) methodF: (MyClass *)x; /* Ok */ + +/* - (void) methodG: (MyClass *)x; dg-warning "previous declaration" */ +/* - (void) methodG: (MyClass *)x; dg-error "duplicate declaration" */ + +/* - (void) methodH: (MyClass *)x; dg-warning "previous declaration" */ +/* - (void) methodH: (MyClass *)x; dg-error "duplicate declaration" */ + +@end Index: testsuite/obj-c++.dg/method-conflict-4.mm =================================================================== --- testsuite/obj-c++.dg/method-conflict-4.mm (revision 0) +++ testsuite/obj-c++.dg/method-conflict-4.mm (revision 0) @@ -0,0 +1,47 @@ +/* Contributed by Nicola Pero , December 2010. */ +/* { dg-do compile } */ + +#include + +/* Test that the compiler can correctly compare protocols in types of + method signatures. In this test we look at protocols implementing + other protocols. The fact that one protocol implements another one + doesn't mean that they are identical. */ + +@protocol A +- (void) doSomething; +@end + +@protocol B +- (void) doSomethingElse; +@end + +@protocol C +- (void) doYetSomethingElse; +@end + +@interface MyClass2 +- (void) aMethod: (id )x; /* { dg-error "previous declaration" } */ +- (void) aMethod: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) bMethod: (id )x; /* { dg-error "previous declaration" } */ +- (void) bMethod: (id )x; /* { dg-error "duplicate declaration" } */ + +- (void) cMethod: (id )x; +- (void) cMethod: (id )x; /* Ok - because if you implement B, then you also implement A, so == */ + +- (void) dMethod: (id )x; +- (void) dMethod: (id )x; /* Ok */ + +/* FIXME: The compiler works, but the testsuite produces errors anyway. */ +/* - (void) eMethod: (id )x; dg-error "previous declaration" */ +/* - (void) eMethod: (id )x; dg-error "duplicate declaration" */ + +/*- (void) fMethod: (id )x; dg-error "previous declaration" */ +/*- (void) fMethod: (id )x; dg-error "duplicate declaration" */