From patchwork Thu Dec 23 05:32:07 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: =?UTF-8?Q?libobjc:=20fix=20class=5FaddMethod()=20to=20return=20NO=20if?= =?UTF-8?Q?=20the=20method=20already=20exists=20in=20the=20class?= Date: Wed, 22 Dec 2010 19:32:07 -0000 From: Nicola Pero X-Patchwork-Id: 76477 Message-Id: <1293082327.57225869@192.168.2.228> To: "gcc-patches@gnu.org" This patch fixes an obvious bug in the new Objective-C runtime API implementation. Testcases both for classes in constructions and for classes already constructed are included. While the bug is obvious once you know where to look, it was crashing gnustep-base in the initial (complicated) +initialize phases and it took time to track down. With this patch applied (and using the latest gnustep-base that I've patched to recognize the new ObjC runtime API and use it), I can now use GCC 4.6 to compile GNUstep Objective-C programs that use all the new Objective-C 2.0 features and which actually work. :-) Committed to trunk. Thanks Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (revision 168198) +++ gcc/testsuite/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2010-12-23 Nicola Pero + + * obj-c.dg/gnu-api-2-class.m: Test that class_addMethod() returns + NO if the method is already implemented in the class. + * obj-c++.dg/gnu-api-2-class.mm: Same change. + 2010-12-22 Sebastian Pop PR tree-optimization/47019 Index: gcc/testsuite/objc.dg/gnu-api-2-class.m =================================================================== --- gcc/testsuite/objc.dg/gnu-api-2-class.m (revision 168198) +++ gcc/testsuite/objc.dg/gnu-api-2-class.m (working copy) @@ -140,6 +140,12 @@ int main(int argc, void **args) method_getTypeEncoding (method2))) abort (); + /* Test that if the method already exists in the class, + class_addMethod() returns NO. */ + if (class_addMethod (new_class, @selector (variable), method_getImplementation (method2), + method_getTypeEncoding (method2))) + abort (); + objc_registerClassPair (new_class); /* Now, MySubClass2 is basically the same as MySubClass! We'll @@ -152,6 +158,15 @@ int main(int argc, void **args) if ([o variable] != o) abort (); } + + /* Now, try that if you take an existing class and try to add an + already existing method, class_addMethod returns NO. This is + subtly different from before, when 'new_class' was still in + construction. Now it's a real class and the libobjc internals + differ between the two cases. */ + if (class_addMethod (new_class, @selector (variable), method_getImplementation (method2), + method_getTypeEncoding (method2))) + abort (); } printf ("Testing class_addProtocol ()...\n"); Index: gcc/testsuite/obj-c++.dg/gnu-api-2-class.mm =================================================================== --- gcc/testsuite/obj-c++.dg/gnu-api-2-class.mm (revision 168198) +++ gcc/testsuite/obj-c++.dg/gnu-api-2-class.mm (working copy) @@ -140,6 +140,12 @@ int main () method_getTypeEncoding (method2))) abort (); + /* Test that if the method already exists in the class, + class_addMethod() returns NO. */ + if (class_addMethod (new_class, @selector (variable), method_getImplementation (method2), + method_getTypeEncoding (method2))) + abort (); + objc_registerClassPair (new_class); /* Now, MySubClass2 is basically the same as MySubClass! We'll @@ -152,6 +158,15 @@ int main () if ([o variable] != o) abort (); } + + /* Now, try that if you take an existing class and try to add an + already existing method, class_addMethod returns NO. This is + subtly different from before, when 'new_class' was still in + construction. Now it's a real class and the libobjc internals + differ between the two cases. */ + if (class_addMethod (new_class, @selector (variable), method_getImplementation (method2), + method_getTypeEncoding (method2))) + abort (); } std::cout << "Testing class_addProtocol ()...\n"; Index: libobjc/sendmsg.c =================================================================== --- libobjc/sendmsg.c (revision 168198) +++ libobjc/sendmsg.c (working copy) @@ -759,6 +759,45 @@ class_addMethod (Class class_, SEL selector, IMP i if (method_name == NULL) return NO; + /* If the method already exists in the class, return NO. It is fine + if the method already exists in the superclass; in that case, we + are overriding it. */ + if (CLS_IS_IN_CONSTRUCTION (class_)) + { + /* The class only contains a list of methods; they have not been + registered yet, ie, the method_name of each of them is still + a string, not a selector. Iterate manually over them to + check if we have already added the method. */ + struct objc_method_list * method_list = class_->methods; + while (method_list) + { + int i; + + /* Search the method list. */ + for (i = 0; i < method_list->method_count; ++i) + { + struct objc_method * method = &method_list->method_list[i]; + + if (method->method_name + && strcmp ((char *)method->method_name, method_name) == 0) + return NO; + } + + /* The method wasn't found. Follow the link to the next list of + methods. */ + method_list = method_list->method_next; + } + /* The method wasn't found. It's a new one. Go ahead and add + it. */ + } + else + { + /* Do the standard lookup. This assumes the selectors are + mapped. */ + if (search_for_method_in_list (class_->methods, selector)) + return NO; + } + method_list = (struct objc_method_list *)objc_calloc (1, sizeof (struct objc_method_list)); method_list->method_count = 1; Index: libobjc/ChangeLog =================================================================== --- libobjc/ChangeLog (revision 168198) +++ libobjc/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2010-12-23 Nicola Pero + + * sendmsg.c (class_addMethod): Return NO if the method already + exists in the class. + 2010-12-22 Nicola Pero * init.c (duplicate_classes): New.