Patch for correctly intializing interface class on Method.invoke() (Was: Line number support for interpreter)

[Bryce McKinlay]

Mark Wielaard wrote:

>The following patch adds explicit initialization of interface classes to
>Method.invoke() and also does the test whether the declaringClass is an
>interface only when the method involved isn't a static method since
>interfaces cannot contain static methods.
>  
>

If other implementations initialize interfaces during Method.invoke(), 
which apparently from our discussions on IRC, they do, then we should do 
the same for compatibility - so your patch is fine. However, an 
interface should not have to be initialized in order to call 
Method.invoke() on it, assuming the method being invoked is not static. 
So, I think the real bug here is that we layout the interface methods at 
the wrong time, ie during initialization when it should be done during 
linking/preparation. So, I suggest the following patch (untested):

Index: java/lang/natClass.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/natClass.cc,v
retrieving revision 1.77
diff -u -r1.77 natClass.cc
--- java/lang/natClass.cc       21 Apr 2004 19:26:22 -0000      1.77
+++ java/lang/natClass.cc       20 May 2004 14:18:25 -0000
@@ -790,9 +790,6 @@
        }
     }
 
-  if (isInterface ())
-    _Jv_LayoutInterfaceMethods (this);
-
   _Jv_PrepareConstantTimeTables (this);
 
   if (vtable == NULL)
Index: java/lang/natClassLoader.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/natClassLoader.cc,v
retrieving revision 1.65
diff -u -r1.65 natClassLoader.cc
--- java/lang/natClassLoader.cc 20 Apr 2004 01:38:46 -0000      1.65
+++ java/lang/natClassLoader.cc 20 May 2004 14:18:25 -0000
@@ -170,6 +170,9 @@
     }
 #endif /* INTERPRETER */
 
+  if (klass->isInterface ())
+    _Jv_LayoutInterfaceMethods (klass);
+
   klass->notifyAll ();
 
   _Jv_PushClass (klass);

>2004-05-20  Mark Wielaard  <mark@klomp.org>
>
>       * java/lang/reflect/natMethod.cc (invoke): Only check isInterface()
>       and set iface when method is non-static. Always initialize interface
>       classes.
>
>There is now a gnu.testlet.java.lang.Class.init test in Mauve.
>gij-3.3 gives a couple of failures since it initializes the interface
>class much too early. gij-3.4 (and gij CVS) crashes as soon as invoke()
>is called on the interface method (but correctly delay initializing the
>interface class). And with this patch gij CVS gives all PASSes for this
>test. All regression tests in libjava/testsuite also still PASS.
>
>OK to commit?
>  
>

OK, but perhaps shorten & change the comment to indicate that this is 
for compatibility with other implementations. It may also be worth 
updating the javadoc for invoke() to indicate what & when it initializes.

Thanks

Bryce

>Index: java/lang/reflect/natMethod.cc
>===================================================================
>RCS file: /cvs/gcc/gcc/libjava/java/lang/reflect/natMethod.cc,v
>retrieving revision 1.39
>diff -u -r1.39 natMethod.cc
>--- java/lang/reflect/natMethod.cc	20 Apr 2004 01:38:46 -0000	1.39
>+++ java/lang/reflect/natMethod.cc	20 May 2004 09:20:47 -0000
>@@ -165,6 +165,18 @@
>      
>       if (! _Jv_IsAssignableFrom (declaringClass, objClass))
>         throw new java::lang::IllegalArgumentException;
>+
>+      // If the declaring class of the method is an interface we will
>+      // have to explicitly initialize it.  Neither
>+      // Class.get(Declared)Method nor creating an Object of a class
>+      // that implements that interface has to initialize the
>+      // interface class.  So this may be the first active use of the
>+      // interface as class.
>+      if (declaringClass->isInterface ())
>+	{
>+	  _Jv_InitClass (declaringClass);
>+	  iface = declaringClass;
>+	}
>     }
> 
>   // Check accessibility, if required.
>@@ -188,9 +200,6 @@
> 	throw new IllegalAccessException;
>     }
> 
>-  if (declaringClass->isInterface())
>-    iface = declaringClass;
>-  
>   return _Jv_CallAnyMethodA (obj, return_type, meth, false,
> 			     parameter_types, args, iface);
> }
>  
>