> On 31 May 2016, at 10:35, [email protected] 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.
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, [email protected] 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
>
