diff mbox

Patch for PR libobjc/50428 ("Consider changing semantics of +initialize so that it is inherited")

Message ID 6285556D-C008-4F35-A5B0-8717EE3F378A@meta-innovation.com
State New
Headers show

Commit Message

Nicola Pero Oct. 8, 2011, 5:53 p.m. UTC 
This patch implements the change discussed in libobjc/50428.  Traditionally, the Apple runtime "inherits" +initialize,
meaning that if a class doesn't implement it, but the superclass does, the superclass's implementation gets executed
the first time a method of the class is invoked.  Instead, the GNU runtime traditionally doesn't "inherit" +initialize,
meaning in that situation, no +initialize is executed.  The idea is that with the GNU runtime you wouldn't have to worry
about protecting your +initialize against multiple executions.  But, in practice, people seem to protect +initialize methods all
the same even with the GNU runtime, and they supply the following two explanations of why:

 * It is generally considered good practice to protect +initialize methods against multiple exclusions because it allows
the code to run without changes with the Apple/NeXT runtime.

 * The runtime doesn't prevent programmers from invoking +initialize directly.  You are not supposed to do it, but some beginners
do, which is why a number of frameworks (including GNUstep) protect all +initialize methods against multiple executions all the
same even when using the GNU runtime and even ignoring the existence of the Apple/NeXT runtime (!!).

Because of these two reasons, it seems that everyone's really protecting their +initialize methods against multiple executions
all the same!  So the difference in behaviour between the GNU runtime and the Apple/NeXT one is not really helping anyone;
but it makes it harder to port code written for the Apple/NeXT runtime to the GNU runtime.  Programmers writing code for the
Apple/NeXT runtime assume that +initialize methods are "inherited", so they may write code which expects such methods to
be called multiple times, once per subclass.  That code would compile on the GNU runtime but not work.  Fixing the code so
that it works with both runtimes is not necessarily trivial in some situations, for example if the classes are dynamically
created at runtime.  So the difference in behaviour is not really helping anyone, but it's making life hard for some people who
are porting software to run on the GNU runtime.

This patch (originally submitted by Richard Frith-Macdonald in a private email, and revised by me) changes the behaviour of the
GNU runtime to match the one of the Apple/NeXT one.  This makes it easier to port software to use the GNU runtime.

A testcase is included; I tested the patch on an Apple Mac OS X 10.6.8, making sure that the testcase behaves in the same way
with the Apple/NeXT and GNU runtimes.  Committed to trunk.

Thanks

PS: This change absolutely deserves a mention in the GCC 4.7 release notes.  But I'll work on the release notes separately,
once we are in stage 2 or so. :-)
diff mbox

Patch

Index: gcc/doc/objc.texi
===================================================================
--- gcc/doc/objc.texi   (revision 179710)
+++ gcc/doc/objc.texi   (working copy)
@@ -635,7 +635,8 @@  following class does this:
 
 + (void)initialize
 @{
-  class_ivar_set_gcinvisible (self, "weakPointer", YES);
+  if (self == objc_lookUpClass ("WeakPointer"))
+    class_ivar_set_gcinvisible (self, "weakPointer", YES);
 @}
 
 - initWithPointer:(const void*)p
Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog       (revision 179710)
+++ gcc/ChangeLog       (working copy)
@@ -1,3 +1,9 @@ 
+2011-10-08  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR libobjc/50428
+       * doc/objc.texi (Garbage Collection): Updated example to protect
+       +initialize against execution in subclasses.
+
 2011-10-07  Richard Henderson  <rth@redhat.com>
 
        * doc/extend.texi (__builtin_shuffle): Improve the description to
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog     (revision 179710)
+++ gcc/testsuite/ChangeLog     (working copy)
@@ -1,3 +1,8 @@ 
+2011-10-08  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR libobjc/50428        
+       * objc/execute/initialize-1.m: New test.
+
 2011-10-08  Paul Thomas  <pault@gcc.gnu.org>
 
        PR fortran/47844
Index: gcc/testsuite/objc/execute/initialize-1.m
===================================================================
--- gcc/testsuite/objc/execute/initialize-1.m   (revision 0)
+++ gcc/testsuite/objc/execute/initialize-1.m   (revision 0)
@@ -0,0 +1,47 @@ 
+/* Contributed by Nicola Pero - Sat  8 Oct 2011 16:47:48 BST */
+#include <objc/objc.h>
+
+/* Test that if a class has no +initialize method, the superclass
+   implementation is called.  */
+
+static int class_variable = 0;
+
+@interface TestClass
+{
+  Class isa;
+}
++ (void) initialize;
++ (int) classVariable;
+@end
+
+@implementation TestClass
++ (void) initialize
+{
+  class_variable++;
+}
++ (int) classVariable
+{
+  return class_variable;
+}
+@end
+
+@interface TestSubClass : TestClass
+@end
+
+@implementation TestSubClass
+@end
+
+int main (void)
+{
+  if ([TestClass classVariable] != 1)
+    {
+      abort ();
+    }
+
+  if ([TestSubClass classVariable] != 2)
+    {
+      abort ();
+    }
+
+  return 0;
+}
Index: libobjc/sendmsg.c
===================================================================
--- libobjc/sendmsg.c   (revision 179710)
+++ libobjc/sendmsg.c   (working copy)
@@ -516,34 +516,13 @@  __objc_send_initialize (Class class)
 
       {
        SEL op = sel_registerName ("initialize");
-       IMP imp = 0;
-        struct objc_method_list * method_list = class->class_pointer->methods;
-       
-        while (method_list)
+        struct objc_method *method = search_for_method_in_hierarchy (class->class_pointer, 
+                                                                    op);
+
+       if (method)
          {
-           int i;
-           struct objc_method * method;
-           
-           for (i = 0; i < method_list->method_count; i++)
-             {
-               method = &(method_list->method_list[i]);
-               if (method->method_name
-                   && method->method_name->sel_id == op->sel_id)
-                 {
-                   imp = method->method_imp;
-                   break;
-                 }
-             }
-           
-           if (imp)
-             break;
-           
-           method_list = method_list->method_next;
-         }
-       if (imp)
-         {
            DEBUG_PRINTF (" begin of [%s +initialize]\n", class->name);
-           (*imp) ((id) class, op);
+           (*method->method_imp) ((id)class, op);
            DEBUG_PRINTF (" end of [%s +initialize]\n", class->name);
          }
 #ifdef DEBUG
Index: libobjc/ChangeLog
===================================================================
--- libobjc/ChangeLog   (revision 179710)
+++ libobjc/ChangeLog   (working copy)
@@ -1,3 +1,14 @@ 
+2011-10-08  Richard Frith-Macdonald <rfm@gnu.org>
+            Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR libobjc/50428
+       * sendmsg.c (__objc_send_initialize): If a class does not have an
+       +initialize method, search for an +initialize method in the
+       superclass and in the ancestor classes and execute the first one
+       that is found.  This makes the GNU runtime behave in the same way
+       as the Apple/NeXT runtime with respect to +initialize methods and
+       subclassing.
+
 2011-08-06  Nicola Pero  <nicola.pero@meta-innovation.com>
 
        PR libobjc/50002