RFR: 8144185: javac produces incorrect RuntimeInvisibleTypeAnnotations length attribute

[Srikanth]

I made the assertions about the differences in the patches simply by 
looking at the patches without ascertaining the behavior on tip due to 
other fixes.

I will look into this in detail to see what is the minimal set of 
changes that would suffice.

Srikanth

On Thursday 11 May 2017 07:47 PM, Liam Miller-Cushon wrote:
> Thanks for reviewing,
>
> On Thu, May 11, 2017 at 2:46 AM, Srikanth 
> <srikanth.adayapalam at oracle.com 
> <mailto:srikanth.adayapalam at oracle.com>> wrote:
>
>
>     I am looking into this one - turns out I had a patch for this that
>     was put through internal review process and some open issues that
>     needed addressing caused it to be deferred.
>
>
> If it'd be less work to continue with your patch that sounds good to 
> me, thanks. As I said I ran into an ASM crash related to this, and 
> thought it might be a simple fix.
>
>     (1) Your proposed patch simply switches on the kind to determine
>     whether annotations need to be carried over. This looks
>     incorrect/insufficient. Type annotations and declarations
>     annotations have different rules to be carried over.
>
>     From
>     http://mail.openjdk.java.net/pipermail/compiler-dev/2015-December/009865.html
>     <http://mail.openjdk.java.net/pipermail/compiler-dev/2015-December/009865.html>:
>
>         - Declaration annotations on a lambda formal should not make
>     it to the
>            class file at all.
>
>
> I think that is handled in ClassWriter: 
> http://hg.openjdk.java.net/jdk9/dev/langtools/file/ee84b7d44339/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java#l1183 
>
>
>          - Type annotations on the type of a lambda formal should be
>     carried
>     over to the type
>            of the formal parameter of the synthetic method that
>     implements
>     the lambda[*]
>
>     I don't think this is handled by your patch ???
>
>
> As far as I can tell this is handled correctly before and after the patch:
>
> class T {
>   @Retention(RUNTIME) @Target(TYPE_USE) @interface A {}
>   Function<String, String> r = (@A String x) -> x;
> }
> ...
>     RuntimeVisibleTypeAnnotations:
>       0: #20(): METHOD_FORMAL_PARAMETER, param_index=0
>
>     (2) From https://bugs.openjdk.java.net/browse/JDK-8144185
>     <https://bugs.openjdk.java.net/browse/JDK-8144185>:
>
>     Not only is the range incorrect, but also LVT index appears incorrect:
>
>     I don't think this is handled by your patch ???
>
>
> I thought it was handled by the patch and exercised by the included 
> test case. What am I missing? The incorrect LVT index comes from 
> associating the annotation with the capture parameter in the synthetic 
> lambda method instead of with the original local variable. In the 
> test, the LVT index of the variable in f() is 1, and the LVT index of 
> the capture parameter in the synthetic lambda method is 0.
>
> Before the patch, I see:
>
>       RuntimeVisibleTypeAnnotations:
>         0: #23(): LOCAL_VARIABLE, {start_pc=0, length=31, index=0}
>
> With the patch applied:
>
>       RuntimeVisibleTypeAnnotations:
>         0: #23(): LOCAL_VARIABLE, {start_pc=3, length=8, index=1}

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20170511/19dec869/attachment-0001.html>