Saturday, June 23, 2007

PATCH Abstract Enum with OpenJDK

The full solution in OpenJDK

Applying from KSL to OpenJDK

First, there are no issues (except a diff of 2 lines in the GPL license block ;-) to apply a patch from KSL to the OpenJDK project trunk checkout.
So I applied my KSL patch and I started modifying "sun/reflect/annotation/AnnotationParser.java" to retrieve the Enum.valueOf() from the parameter class type and not from the type definition. It means that in:
@AnnoAETest(ae2 = E21.one)
private String first;
where:
public @interface AnnoAETest {
AbstractE2 ae2();
}
the AnnotationParser was doing Enum.valueOf(AbstractE2.class,"one"), but it had actually the class E21 defined, so I changed it to the good Enum.valueOf(E21.class,"one").
So the patch is:
 Index: j2se/src/share/classes/sun/reflect/annotation/AnnotationParser.java
===================================================================
--- j2se/src/share/classes/sun/reflect/annotation/AnnotationParser.java (revision 237)
+++ j2se/src/share/classes/sun/reflect/annotation/AnnotationParser.java (working copy)
@@ -422,9 +422,12 @@
if (!enumType.getName().equals(typeName))
return new AnnotationTypeMismatchExceptionProxy(
typeName + "." + constName);
- } else if (enumType != parseSig(typeName, container)) {
- return new AnnotationTypeMismatchExceptionProxy(
- typeName + "." + constName);
+ } else {
+ Class paramEnumType = parseSig(typeName, container);
+ if (!enumType.isAssignableFrom(paramEnumType))
+ return new AnnotationTypeMismatchExceptionProxy(
+ typeName + "." + constName);
+ enumType = paramEnumType;
}

try {
Now, another issue appears, is that the complicated test to know if a class is an enum with member or an abstract enum or anonymous class is complicated and the Class.isEnum() implementation was wrong.
After changing to the following implementation to remove specialized enum (anonymous):
 Index: j2se/src/share/classes/java/lang/Class.java
===================================================================
--- j2se/src/share/classes/java/lang/Class.java (revision 237)
+++ j2se/src/share/classes/java/lang/Class.java (working copy)
@@ -2877,8 +2877,7 @@
// An enum must both directly extend java.lang.Enum and have
// the ENUM bit set; classes for specialized enum constants
// don't do the former.
- return (this.getModifiers() & ENUM) != 0 &&
- this.getSuperclass() == java.lang.Enum.class;
+ return ((this.getModifiers() & ENUM) != 0 && !isAnonymousClass());
}

// Fetches the factory for reflective objects
I'm getting for my following last failed test:
public class AnnoAEUsage {
@AnnoAETest(ae2 = E21.one)
private String first;

public static void main(String[] args) throws Exception {
System.out.println("Is enum "+E21.class.isEnum());
E21 e21 = Enum.valueOf(E21.class, "one");
switch (e21) {
case one:
System.out.println("Got 1");
break;
default:
System.err.println("No got 1");
}
Field field = AnnoAEUsage.class.getDeclaredField("first");
AnnoAETest anno = field.getAnnotation(AnnoAETest.class);
AbstractE2 ae2 = anno.ae2();
System.out.println("Youpi again "+ae2.full());
}
}

enum E21 extends AbstractE2 {
one, two;
}

The following output:

JDK Build:/work/java.net/ksl.trunk$/work/java.net/openjdk.trunk/control/build/linux-amd64/j2sdk-image/bin/java -cp build/test-classes abstractEnum.annotation.AnnoAEUsage
Is enum true
Got 1
Exception in thread "main" java.lang.IllegalAccessError: tried to access class abstractEnum.annotation.AbstractE2 from class $Proxy3
at $Proxy3.ae2(Unknown Source)
at abstractEnum.annotation.AnnoAEUsage.main(AnnoAEUsage.java:32)

Some more work...

Stupid isn't it
Well it turn out this one is logical...
It's related to an open bug: 6256803 which is not really a bug in my view. You just need to expose public your interfaces, since the dynamic proxy implementing annotation interface is not part of the package. So my AbstractE2 abstract enum, and the E22, E21 real enums were package protected, so: IllegalAccessError.
After making all the test classes public, it works...

The final OpenJDK Patch
For the b14 of openJDK I needed to apply for my platform: 6567018
The above patch file is for "hotspot/src/share/vm/gc_implementation/"

The root project for my patches (KSL and OpenJDK) and all tests files is here.

For the OpenJDK all patches were executed under:
URL: https://openjdk.dev.java.net/svn/openjdk/jdk/trunk
Repository Root: https://openjdk.dev.java.net/svn/openjdk
Revision: 239
And they can be downloaded here.

What's next
So, now what's left to do is to remove the "extends Enum" and make the generics declarations ">" generated by synthetic sugar.
I think that during the work on generics I will try to see the difficulty of having generics for Enum also?
But before all that I think I will start to apply this internal JDK to the fields-enum and other projects, to really judge the improvements...

The "reopened" RFE was accepted
Thank you for taking the time to suggest this enhancement to the Java Standard Edition.

We have determined that this report is an RFE and has been entered into our internal RFE tracking system under Bug Id: 6570766

1. Voting for the RFE
Click http://bugs.sun.com/bugdatabase/addVote.do?bug_id=6570766

2. Adding the report to your Bug Watch list.
You will receive an email notification when this RFE is updated. Click http://bugs.sun.com/bugdatabase/addBugWatch.do?bug_id=6570766

2 comments:

Frederic Simon said...

The full patch for build 21 is here.

Frederic Simon said...

The full patch for build 21 is now here!
And, if you are interested or want to learn more please check the extended enums web site: http://www.extended-enums.org/