[ecj] Patch: FYI: misc. annotation bug fixes

[Tom Tromey <tromey at redhat dot com>]

I'm checking this in on the gcj-eclipse branch.

This fixes a number of bugs I found in annotations.

* It overrides getAnnotation() in Method, Field, and Constructor.
  (This patch could be pushed into Classpath.  And perhaps the code
  could be put into AccessibleObject.)

* It handles 'c' and 'e' values better, and fixes a bug with my
  earlier patch for 's' values.

* It fixes a couple bugs in defineclass.

* It actually creates annotations at runtime.  I fixed a couple little
  things in AnnotationInvocationHandler; I'll push this back into
  Classpath soon.

Now the short test case Andrew sent me today works fine.

I've marked known problems with FIXME.  Mostly these are related to
properly emitting exceptions -- the AnnotationInvocationHandler code
doesn't handle this, and we'd need to do a little experimentation to
see what is correct.  I consider this relatively minor.  Also there is
the issue that annotation inheritance is not working properly; the
code from Classpath is just wrong.  (There's a PR for this one.)

Tom

Index: ChangeLog
from  Tom Tromey  <tromey@redhat.com>
	* defineclass.cc (handleMemberAnnotations): Write member index
	after 'kind'.
	(handleAnnotation): Call prepare_pool_entry.
	* java/lang/reflect/natMethod.cc (getDeclaredAnnotationsInternal):
	Removed unused variable.
	(getParameterAnnotationsInternal): Likewise.
	* java/lang/reflect/natField.cc (getDeclaredAnnotationsInternal):
	Removed unused variable.
	* java/lang/reflect/natConstructor.cc
	(getDeclaredAnnotationsInternal): Removed unused variable.
	(getParameterAnnotationsInternal): Likewise.
	* java/lang/natClass.cc (parseAnnotation): Create annotation.
	(parseAnnotationElement): Handle 'c' and 'e' cases more
	correctly.
	(getMethodDefaultValue): Fixed variable names.
	(parseAnnotationElement): Create String for 's' entry.
	* java/lang/reflect/Constructor.java (getAnnotation): New method.
	* java/lang/reflect/Field.java (getAnnotation): New method.
	* java/lang/reflect/Method.java (getAnnotation): New method.

Index: classpath/ChangeLog.gcj
from  Tom Tromey  <tromey@redhat.com>

	* sun/reflect/annotation/AnnotationInvocationHandler.java
	(invoke): Clone array values before return.
	(create): New method.
	(arrayClone): Likewise.

Index: java/lang/natClass.cc
===================================================================
--- java/lang/natClass.cc	(revision 117830)
+++ java/lang/natClass.cc	(working copy)
@@ -68,6 +68,9 @@
 #include <java/lang/Boolean.h>
 #include <java/lang/annotation/Annotation.h>
 #include <java/util/HashMap.h>
+#include <java/util/Map.h>
+#include <sun/reflect/annotation/AnnotationInvocationHandler.h>
+#include <java/lang/Enum.h>
 
 #include <java-cpool.h>
 #include <java-interp.h>
@@ -1145,30 +1148,39 @@
 	// Despite what the JVM spec says, compilers generate a Utf8
 	// constant here, not a String.
 	check_constant (pool, cindex, JV_CONSTANT_Utf8);
-	result = _Jv_Linker::resolve_pool_entry (klass, cindex).string;
+	result = pool->data[cindex].utf8->toString();
       }
       break;
     case 'e':
       {
-	// FIXME
-// 	uint16 type_name_index = JCF_readu2 (jcf);
-// 	uint16 const_name_index = JCF_readu2 (jcf);
+	int type_name_index = read_u2 (bytes, last);
+	check_constant (pool, type_name_index, JV_CONSTANT_Utf8);
+ 	int const_name_index = read_u2 (bytes, last);
+	check_constant (pool, const_name_index, JV_CONSTANT_Utf8);
+
+	_Jv_Utf8Const *u_name = pool->data[type_name_index].utf8;
+	_Jv_Utf8Const *e_name = pool->data[const_name_index].utf8;
+
+	// FIXME: throw correct exceptions at the correct times.
+	jclass e_class = _Jv_FindClassFromSignature(u_name->chars(),
+						    klass->getClassLoaderInternal());
+	result = ::java::lang::Enum::valueOf(e_class, e_name->toString());
       }
       break;
     case 'c':
       {
-	// FIXME: should be a Utf8 here!
 	int cindex = read_u2 (bytes, last);
-	check_constant (pool, cindex, JV_CONSTANT_Class);
-	try
-	  {
-	    result
-	      = _Jv_Linker::resolve_pool_entry (klass, cindex, false).clazz;
-	  }
-	catch (NoClassDefFoundError *err)
-	  {
-	    throw new TypeNotPresentException(err->getMessage(), err);
-	  }
+	check_constant (pool, cindex, JV_CONSTANT_Utf8);
+	_Jv_Utf8Const *u_name = pool->data[cindex].utf8;
+	jclass anno_class
+	  = _Jv_FindClassFromSignatureNoException(u_name->chars(),
+						  klass->getClassLoaderInternal());
+	// FIXME: not correct: we should lazily do this when trying to
+	// read the element.  This means that
+	// AnnotationInvocationHandler needs to have a special case.
+	if (! anno_class)
+	  // FIXME: original exception...
+	  throw new TypeNotPresentException(u_name->toString(), NULL);
       }
       break;
     case '@':
@@ -1197,8 +1209,12 @@
 {
   int type_index = read_u2 (bytes, last);
   check_constant (pool, type_index, JV_CONSTANT_Utf8);
-  jclass anno_class = NULL; // FIXME ... and share code with 'c' case
 
+  _Jv_Utf8Const *u_name = pool->data[type_index].utf8;
+  jclass anno_class = _Jv_FindClassFromSignatureNoException(u_name->chars(),
+							    klass->getClassLoaderInternal());
+  // FIXME: what to do if anno_class==NULL?
+
   ::java::util::HashMap *hmap = new ::java::util::HashMap();
   int npairs = read_u2 (bytes, last);
   for (int i = 0; i < npairs; ++i)
@@ -1210,9 +1226,9 @@
       // FIXME: any checks needed for name?
       hmap->put(name, value);
     }
-  // FIXME: use AnnotationInvocationHandler here.
-  // FIXME: add a static method to it for creation, that handles defaults.
-  return NULL;
+  using namespace ::sun::reflect::annotation;
+  return AnnotationInvocationHandler::create (anno_class,
+					      (::java::util::Map *) hmap);
 }
 
 static jobjectArray
@@ -1259,18 +1275,18 @@
 
   while (true)
     {
-      int kind = read_u1 (bytes);
-      if (kind == JV_DONE_ATTR)
+      int type = read_u1 (bytes);
+      if (type == JV_DONE_ATTR)
 	return NULL;
       int len = read_4 (bytes);
       unsigned char *next = bytes + len;
-      if (kind != JV_METHOD_ATTR)
+      if (type != JV_METHOD_ATTR)
 	{
 	  bytes = next;
 	  continue;
 	}
-      int type = read_u1 (bytes, next);
-      if (type != JV_ANNOTATION_DEFAULT_KIND)
+      int kind = read_u1 (bytes, next);
+      if (kind != JV_ANNOTATION_DEFAULT_KIND)
 	{
 	  bytes = next;
 	  continue;
Index: java/lang/reflect/Method.java
===================================================================
--- java/lang/reflect/Method.java	(revision 117705)
+++ java/lang/reflect/Method.java	(working copy)
@@ -414,6 +414,15 @@
    */
   public native Object getDefaultValue();
 
+  public <T extends Annotation> T getAnnotation(Class<T> annoClass)
+  {
+    Annotation[] annos = getDeclaredAnnotations();
+    for (int i = 0; i < annos.length; ++i)
+      if (annos[i].annotationType() == annoClass)
+	return (T) annos[i];
+    return null;
+  }
+
   public Annotation[] getDeclaredAnnotations()
   {
     Annotation[] result = getDeclaredAnnotationsInternal();
Index: java/lang/reflect/Field.java
===================================================================
--- java/lang/reflect/Field.java	(revision 117705)
+++ java/lang/reflect/Field.java	(working copy)
@@ -734,6 +734,15 @@
     return p.getFieldType();
   }
 
+  public <T extends Annotation> T getAnnotation(Class<T> annoClass)
+  {
+    Annotation[] annos = getDeclaredAnnotations();
+    for (int i = 0; i < annos.length; ++i)
+      if (annos[i].annotationType() == annoClass)
+	return (T) annos[i];
+    return null;
+  }
+
   public Annotation[] getDeclaredAnnotations()
   {
     Annotation[] result = getDeclaredAnnotationsInternal();
Index: java/lang/reflect/natMethod.cc
===================================================================
--- java/lang/reflect/natMethod.cc	(revision 117705)
+++ java/lang/reflect/natMethod.cc	(working copy)
@@ -208,14 +208,12 @@
 anno_a_t
 java::lang::reflect::Method::getDeclaredAnnotationsInternal()
 {
-  anno_a_t result;
   return (anno_a_t) declaringClass->getDeclaredAnnotations(this, false);
 }
 
 anno_aa_t
 java::lang::reflect::Method::getParameterAnnotationsInternal()
 {
-  anno_aa_t result;
   return (anno_aa_t) declaringClass->getDeclaredAnnotations(this, true);
 }
 
Index: java/lang/reflect/natConstructor.cc
===================================================================
--- java/lang/reflect/natConstructor.cc	(revision 117705)
+++ java/lang/reflect/natConstructor.cc	(working copy)
@@ -41,14 +41,12 @@
 anno_a_t
 java::lang::reflect::Constructor::getDeclaredAnnotationsInternal()
 {
-  anno_a_t result;
   return (anno_a_t) declaringClass->getDeclaredAnnotations(this, false);
 }
 
 anno_aa_t
 java::lang::reflect::Constructor::getParameterAnnotationsInternal()
 {
-  anno_aa_t result;
   return (anno_aa_t) declaringClass->getDeclaredAnnotations(this, true);
 }
 
Index: java/lang/reflect/Constructor.java
===================================================================
--- java/lang/reflect/Constructor.java	(revision 117705)
+++ java/lang/reflect/Constructor.java	(working copy)
@@ -378,6 +378,15 @@
     return p.getGenericParameterTypes();
   }
 
+  public <T extends Annotation> T getAnnotation(Class<T> annoClass)
+  {
+    Annotation[] annos = getDeclaredAnnotations();
+    for (int i = 0; i < annos.length; ++i)
+      if (annos[i].annotationType() == annoClass)
+	return (T) annos[i];
+    return null;
+  }
+
   public Annotation[] getDeclaredAnnotations()
   {
     Annotation[] result = getDeclaredAnnotationsInternal();
Index: java/lang/reflect/natField.cc
===================================================================
--- java/lang/reflect/natField.cc	(revision 117705)
+++ java/lang/reflect/natField.cc	(working copy)
@@ -46,7 +46,6 @@
 anno_a_t
 java::lang::reflect::Field::getDeclaredAnnotationsInternal()
 {
-  anno_a_t result;
   return (anno_a_t) declaringClass->getDeclaredAnnotations(this);
 }
 
Index: classpath/sun/reflect/annotation/AnnotationInvocationHandler.java
===================================================================
--- classpath/sun/reflect/annotation/AnnotationInvocationHandler.java	(revision 117705)
+++ classpath/sun/reflect/annotation/AnnotationInvocationHandler.java	(working copy)
@@ -39,11 +39,13 @@
 package sun.reflect.annotation;
 
 import java.io.Serializable;
+import java.lang.annotation.Annotation;
 import java.lang.annotation.AnnotationTypeMismatchException;
 import java.lang.annotation.IncompleteAnnotationException;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.Map;
@@ -74,6 +76,24 @@
         this.memberValues = memberValues;
     }
 
+    public static Annotation create(Class type, Map memberValues)
+    {
+      for (Method m : type.getDeclaredMethods())
+	{
+	  String name = m.getName();
+	  if (! memberValues.containsKey(name))
+	    {
+	      // FIXME: what to do about exceptions here?
+	      memberValues.put(name, m.getDefaultValue());
+	    }
+	}
+      AnnotationInvocationHandler handler
+	= new AnnotationInvocationHandler(type, memberValues);
+      return (Annotation) Proxy.newProxyInstance(type.getClassLoader(),
+						 new Class[] { type },
+						 handler);
+    }
+
     /**
      * Compare an instance of AnnotationInvocationHandler with another object.
      * Note that the other object does not have to be an
@@ -295,6 +315,38 @@
         return returnType;
     }
 
+    private Object arrayClone(Object obj)
+    {
+        if (obj instanceof boolean[])
+	    return ((boolean[]) obj).clone();
+
+        if (obj instanceof byte[])
+	    return ((byte[]) obj).clone();
+
+        if (obj instanceof char[])
+	    return ((char[]) obj).clone();
+
+        if (obj instanceof short[])
+	    return ((short[]) obj).clone();
+
+        if (obj instanceof int[])
+	    return ((int[]) obj).clone();
+
+        if (obj instanceof float[])
+	    return ((float[]) obj).clone();
+
+        if (obj instanceof long[])
+	    return ((long[]) obj).clone();
+
+        if (obj instanceof double[])
+	    return ((double[]) obj).clone();
+
+        if (obj instanceof Object[])
+	    return ((Object[]) obj).clone();
+
+        return obj;
+    }
+
     public Object invoke(Object proxy, Method method, Object[] args)
       throws Throwable
     {
@@ -325,6 +377,10 @@
                     throw new AnnotationTypeMismatchException(method,
                         val.getClass().getName());
                 }
+		if (val.getClass().isArray())
+		{
+		    val = arrayClone(val);
+		}
                 return val;
             }
         }
Index: defineclass.cc
===================================================================
--- defineclass.cc	(revision 117830)
+++ defineclass.cc	(working copy)
@@ -577,6 +577,7 @@
     {
       int name_index = read2u();
       check_tag (name_index, JV_CONSTANT_Utf8);
+      prepare_pool_entry (name_index, JV_CONSTANT_Utf8);
       handleAnnotationElement();
     }
 }
@@ -608,9 +609,9 @@
   if (member_type != JV_CLASS_ATTR)
     newLen += 2;
   stream->writeInt(newLen);
+  stream->writeByte(JV_ANNOTATIONS_KIND);
   if (member_type != JV_CLASS_ATTR)
     stream->writeShort(member_index);
-  stream->writeByte(JV_ANNOTATIONS_KIND);
   // Write the data as-is.
   stream->write(input_data, input_offset + orig_pos, len);
 }