Hi All,
Please see updated webrev
http://cr.openjdk.java.net/~srastogi/8147585/webrev.02/
On 5/31/2016 2:21 PM, Paul Sandoz wrote:
On 31 May 2016, at 10:35, shilpi.rast...@oracle.com wrote:
Thanks Paul for comments.
Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.01/
Now processing only public abstract methods of interface.
Thanks. It would be good to get some got feedback from those wiser than I in
this regard.
Have you looked at the existing annotation-based tests to see if they test edge
cases e.g. annotation classes generated with incorrect methods? that might give
us some clues.
I saw existing annotation-based test, valid modifier for annotations,
valid method for annotation tests we are checking in javac code.
default, static, private modifier we are restricting at compile time
so we can not add test cases for this (compilation will fail).
So only scenario i can add is for synthetic methods. ( according to my
assumption)
In AnnotationType.java
public Method[] run() {
// Initialize memberTypes and defaultValues
return annotationClass.getDeclaredMethods();
}
});
As here, calling getDeclaredMethods() on annotationclass and
getDeclaredMethods() doc says
https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#getDeclaredMethods--
"Returns an array containing Method objects reflecting all the declared
methods of the class or interface represented by this Class object,
including public, protected, default (package) access, and private
methods, but excluding inherited methods."
Doc did not mention anything about synthetic methods so i am not sure
this is expected behavior or not.
If yes, Could you please suggest how to add testcases to test synthetic
method?
Shall I use ASM?
Regards,
Shilpi
Testing wise:
- you can avoid the filtering of the test method by placing the annotations in
another auxiliary class (my preference is to nest rather than using auxiliary
classes, up to you)
- there is a couple of minor style inconsistencies e.g. a space here "@ interface
LambdaWithoutParameter"
- 30 * @run testng/othervm -ea -esa AnnotationWithLambda
You can just do:
@run testng AnnotationWithLambda
?
Thanks,
Paul.
Thanks,
Shilpi
On 5/30/2016 6:35 PM, Paul Sandoz wrote:
Hi Shilpi,
You have found the right place but i am not sure your fix is entirely correct.
(Tip: if you use -Xlog:exceptions=info you can observe the IAE exception when
the annotation is processed)
In your test you have:
@Target(value = ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ interface LambdaWithParameter {
Consumer<Integer> f1 = a -> {
System.out.println("lambda has parameter");
};
}
@Target(value = ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ interface LambdaWithoutParameter {
Runnable r = () -> System.out.println("lambda without parameter");
}
Both of those annotations will have static synthetic methods generated by the
compiler that the indy resolves and links to (look at the javap output). The
former will have a method with one parameter.
The code in sun/reflect/annotation/AnnotationType.java:
for (Method method : methods) {
if (method.getParameterTypes().length != 0)
throw new IllegalArgumentException(method + " has params");
has thrown the IAE since 2004, but it’s not clear why it was added as opposed
to something more general (see below).
The correct fix appears to be to skip over any non-abstract/non-public methods.
Thus only public abstract methods get processed.
Your current fix now processes synthetic methods with parameters, in addition
to those which were already processed such as synthetic methods without
parameters, or even private methods that could have been generated by some
tool. I dunno how much say the verifier has in all this, perhaps little or no
say.
Thus non-public/non-abstract methods could add inconsistent information to the
data structures of AnnotationType. Perhaps this is mostly harmless?
Perhaps Joel (CC’ed) can she some more light on this?
Paul.
On 30 May 2016, at 08:00, shilpi.rast...@oracle.com wrote:
Hi All,
Please review fix for https://bugs.openjdk.java.net/browse/JDK-8147585
http://cr.openjdk.java.net/~srastogi/8147585/webrev.00/
Problem: Annotation with Lamda has parameters results into wrong behavior ( not
considered as valid annotation) because
According to JLS "By virtue of the AnnotationTypeElementDeclaration production, a
method declaration in an annotation type declaration cannot have formal parameters, type
parameters, or a throws clause. The following production from §4.3 is shown here for
convenience:"
(Ref: https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.1)
Solution: We should skip synthetic methods during Annotation parsing. According to JLS
"An annotation type has no elements other than those defined by the methods it
explicitly declares."
(Ref https://docs.oracle.com/javase/specs/jls/se8/html/.html#jls-9jls-9.6.1)
Thanks,
Shilpi
