Re: PING: Reflection doesn't work across interfaces

[Bryce McKinlay <mckinlay at redhat dot com>]

Tom Tromey wrote:

> Bryce> I think it would be better to just have the compiler put the interface
> Bryce> method's index in the meth->index field here, rather than have the
> Bryce> runtime do fixups on a hypothetical vtable offset. How does the patch
> Bryce> below look? Otherwise, your patch looks fine to me. Obviously the
> Bryce> invocation code has become a bit of a mess and could really use some
> Bryce> refactoring, for example to clearly separate the lookup and argument
> Bryce> prep/execution stages, but this looks ok for now.
>
> This sounds reasonable to me.  Andrew would have to approve the patch
> though.  Wouldn't we also need (or at least want) to fill in this
> field at runtime for those interfaces laid out then?
>  

Since the dispatch index for each individual interface starts at 1, the 
value is always known at compile time regardless of any inheritance etc.

But actually, the compiler doesn't need to generate the index field at 
all (interface or otherwise) for the BC-ABI - we should drop it when we 
change/optimize the class metadata format for BC. But for now, I figure 
the space is there in _Jv_Method anyway, so we might as well use it.

I have checked in the following patch.

Regards

Bryce

2004-04-14  Andrew Haley  <aph@redhat.com>
            Bryce McKinlay  <mckinlay@redhat.com>

	* java/lang/reflect/natMethod.cc (_Jv_CallAnyMethodA): Use
	_Jv_LookupInterfaceMethodIdx for calls to interfaces.
	* include/jvm.h (_Jv_CallAnyMethodA): Add new `iface' arg.

	* testsuite/libjava.lang/InvokeInterface.java: New file.
	* testsuite/libjava.lang/InvokeInterface.out: New file.
	
2004-04-14  Bryce McKinlay  <mckinlay@redhat.com>

	* class.c (get_interface_method_index): New function. Return dispatch 
	index for interface method.
	(make_method_value): For interface methods, set index field to
	iface dispatch index, not DECL_VINDEX.
	* expr.c (build_invokeinterface): Use get_interface_method_index.

Index: include/jvm.h
===================================================================
RCS file: /cvs/gcc/gcc/libjava/include/jvm.h,v
retrieving revision 1.63
diff -u -r1.63 jvm.h
--- include/jvm.h	1 Feb 2004 20:05:03 -0000	1.63
+++ include/jvm.h	14 Apr 2004 17:36:31 -0000
@@ -422,7 +422,8 @@
 extern jobject _Jv_CallAnyMethodA (jobject obj, jclass return_type,
 				   jmethodID meth, jboolean is_constructor,
 				   JArray<jclass> *parameter_types,
-				   jobjectArray args);
+				   jobjectArray args,
+				   jclass iface = NULL);
 
 union jvalue;
 extern void _Jv_CallAnyMethodA (jobject obj,
@@ -433,7 +434,8 @@
 				JArray<jclass> *parameter_types,
 				jvalue *args,
 				jvalue *result,
-				jboolean is_jni_call = true);
+				jboolean is_jni_call = true,
+				jclass iface = NULL);
 
 extern jobject _Jv_NewMultiArray (jclass, jint ndims, jint* dims)
   __attribute__((__malloc__));
Index: java/lang/reflect/natMethod.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/reflect/natMethod.cc,v
retrieving revision 1.37
diff -u -r1.37 natMethod.cc
--- java/lang/reflect/natMethod.cc	26 Feb 2004 15:22:20 -0000	1.37
+++ java/lang/reflect/natMethod.cc	14 Apr 2004 17:36:31 -0000
@@ -143,14 +143,14 @@
 java::lang::reflect::Method::invoke (jobject obj, jobjectArray args)
 {
   using namespace java::lang::reflect;
+  jclass iface = NULL;
   
   if (parameter_types == NULL)
     getType ();
     
   jmethodID meth = _Jv_FromReflectedMethod (this);
-
   jclass objClass;
-  
+
   if (Modifier::isStatic(meth->accflags))
     {
       // We have to initialize a static class.  It is safe to do this
@@ -188,8 +188,11 @@
 	throw new IllegalAccessException;
     }
 
+  if (declaringClass->isInterface())
+    iface = declaringClass;
+  
   return _Jv_CallAnyMethodA (obj, return_type, meth, false,
-			     parameter_types, args);
+			     parameter_types, args, iface);
 }
 
 jint
@@ -341,7 +344,8 @@
 		    JArray<jclass> *parameter_types,
 		    jvalue *args,
 		    jvalue *result,
-		    jboolean is_jni_call)
+		    jboolean is_jni_call,
+		    jclass iface)
 {
   using namespace java::lang::reflect;
   
@@ -478,7 +482,10 @@
       && (_Jv_ushort)-1 != meth->index)
     {
       _Jv_VTable *vtable = *(_Jv_VTable **) obj;
-      ncode = vtable->get_method (meth->index);
+      if (iface == NULL)
+	ncode = vtable->get_method (meth->index);
+      else
+	ncode = _Jv_LookupInterfaceMethodIdx (vtable->clas, iface, meth->index);
     }
   else
     {
@@ -553,7 +560,8 @@
 		    jmethodID meth,
 		    jboolean is_constructor,
 		    JArray<jclass> *parameter_types,
-		    jobjectArray args)
+		    jobjectArray args,
+		    jclass iface)
 {
   if (parameter_types->length == 0 && args == NULL)
     {
@@ -621,7 +629,7 @@
   _Jv_CallAnyMethodA (obj, return_type, meth, is_constructor,
   		      _Jv_isVirtualMethod (meth),
 		      parameter_types, argvals, &ret_value,
-		      false);
+		      false, iface);
 
   jobject r;
 #define VAL(Wrapper, Field)  (new Wrapper (ret_value.Field))
Index: class.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/class.c,v
retrieving revision 1.180
diff -u -r1.180 class.c
--- class.c	20 Mar 2004 14:03:33 -0000	1.180
+++ class.c	14 Apr 2004 17:39:48 -0000
@@ -1246,10 +1246,15 @@
   tree minit;
   tree index;
   tree code;
+  tree class_decl;
 #define ACC_TRANSLATED          0x4000
   int accflags = get_access_flags_from_decl (mdecl) | ACC_TRANSLATED;
-
-  if (!flag_indirect_dispatch && DECL_VINDEX (mdecl) != NULL_TREE)
+  
+  class_decl = DECL_CONTEXT (mdecl);
+  /* For interfaces, the index field contains the dispatch index. */
+  if (CLASS_INTERFACE (TYPE_NAME (class_decl)))
+    index = build_int_2 (get_interface_method_index (mdecl, class_decl), 0);
+  else if (!flag_indirect_dispatch && DECL_VINDEX (mdecl) != NULL_TREE)
     index = DECL_VINDEX (mdecl);
   else
     index = integer_minus_one_node;
@@ -2131,6 +2136,23 @@
 					method_decl, dtable_count);
 
   TYPE_NVIRTUALS (this_class) = dtable_count;
+}
+
+/* Return the index of METHOD in INTERFACE.  This index begins at 1 and is used as an
+   argument for _Jv_LookupInterfaceMethodIdx(). */
+int
+get_interface_method_index (tree method, tree interface)
+{
+  tree meth;
+  int i = 1;
+
+  for (meth = TYPE_METHODS (interface); ; meth = TREE_CHAIN (meth), i++)
+    {
+      if (meth == method)
+	return i;
+      if (meth == NULL_TREE)
+	abort ();
+    }
 }
 
 /* Lay METHOD_DECL out, returning a possibly new value of
Index: expr.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/expr.c,v
retrieving revision 1.185
diff -u -r1.185 expr.c
--- expr.c	30 Mar 2004 19:19:06 -0000	1.185
+++ expr.c	14 Apr 2004 17:39:49 -0000
@@ -1974,17 +1974,7 @@
     }
   else
     {
-      i = 1;
-      for (meth = TYPE_METHODS (interface); ; meth = TREE_CHAIN (meth), i++)
-	{
-	  if (meth == method)
-            {
-	      idx = build_int_2 (i, 0);
-	      break;
-	    }
-	  if (meth == NULL_TREE)
-	    abort ();
-	}
+      idx = build_int_2 (get_interface_method_index (method, interface), 0);
     }
 
   lookup_arg = tree_cons (NULL_TREE, dtable,
Index: java-tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/java-tree.h,v
retrieving revision 1.197
diff -u -r1.197 java-tree.h
--- java-tree.h	19 Mar 2004 23:10:55 -0000	1.197
+++ java-tree.h	14 Apr 2004 17:39:49 -0000
@@ -1149,6 +1149,7 @@
 extern tree parse_signature_string (const unsigned char *, int);
 extern tree get_type_from_signature (tree);
 extern void layout_class (tree);
+extern int get_interface_method_index (tree, tree);
 extern tree layout_class_method (tree, tree, tree, tree);
 extern void layout_class_methods (tree);
 extern tree build_class_ref (tree);