RFR 8189708: class reading issue with named and annotated parameters

[Liam Miller-Cushon]

Thanks,

I updated the copyright header and moved the tests to the correct location:
http://cr.openjdk.java.net/~cushon/8007720/webrev.02/

Also - note that the patch currently includes a fix for JDK-8177486 as
well, since you suggested I add test coverage for enum and inner class
constructors, and mandated parameters aren't handled correctly without
fixing the other bug. Is it OK to have one changeset for both issues, or
should they be separated out? I could remove the enum and inner class test
coverage and the fix, and then add it back in as a follow-up change if you
prefer.

On Wed, Dec 13, 2017 at 8:01 AM, Jan Lahoda <jan.lahoda at oracle.com> wrote:

> Sorry for late reply.
>
> Seems OK to me, only:
> -the tests should be in:
> test/langtools/tools/javac/MethodParameters/8189708
> not in:
> test/tools/javac/MethodParameters/8189708
>
> -test/tools/javac/MethodParameters/8189708/Test.java has a weird license
> header
>
> Vicente, could you please take a look as well?
>
> Thanks,
>     Jan
>
> On 4.12.2017 22:37, Liam Miller-Cushon wrote:
>
>> Hi, is there any more feedback on this?
>>
>> On Fri, Oct 20, 2017 at 5:51 PM, Liam Miller-Cushon <cushon at google.com
>> <mailto:cushon at google.com>> wrote:
>>
>>     Here's an updated patch that incorporates your approach:
>>     http://cr.openjdk.java.net/~cushon/8007720/webrev.01/
>>     <http://cr.openjdk.java.net/~cushon/8007720/webrev.01/>
>>
>>     I included the fix for JDK-8177486 so I could test the inner class /
>>     enum constructor case. If this looks like it's on the right track
>>     I'll move that part (and the corresponding tests) back into a
>>     separate change.
>>
>>     On Fri, Oct 20, 2017 at 9:30 AM, Liam Miller-Cushon
>>     <cushon at google.com <mailto:cushon at google.com>> wrote:
>>
>>         Thanks for the comments,
>>
>>         On Fri, Oct 20, 2017 at 12:28 AM, Jan Lahoda
>>         <jan.lahoda at oracle.com <mailto:jan.lahoda at oracle.com>> wrote:
>>
>>             -what happens if there are both runtime invisible and
>>             visible annotations of method's parameters? Seems those that
>>             appear later will overwrite those that appear sooner?
>>
>>
>>         Oops, thanks. The way your patch handles that looks good to me.
>>
>>             -the MethodSymbol.savedParameterAnnotations is only used
>>             during reading inside the ClassReader, right? It seems
>>             wasteful to have it as a field on each MethodSymbol, better
>>             would be a field in ClassReader.
>>
>>
>>         Sounds good. I'll try to avoid having savedParameterNames as a
>>         field in MethodSymbol also. Do you remember if you encountered
>>         any issues with that in your patch?
>>
>>             -please check what happens for annotations on constructors
>>             of enums/non-static innerclasses
>>
>>
>>         Will do. (Also, note that there appears to be an issue with
>>         reading MethodParameters on constructors of enums/non-static
>>         inner classes: JDK-8177486)
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20171213/da1e05b4/attachment-0001.html>