RFR: JDK-8190452: javac should not add MethodParameters attributes to v51 and earlier class files

[Liam Miller-Cushon]

Hi Vicente, thanks for the review!

I inlined the golden files into the test class. I don't think inlining
Lib.java works with the current approach of using `@compile --release 7/8`
and reflection to access the parameter names, because the reflective APIs
were added in 8. I could rewrite the test to use class reading instead of
reflection if you prefer.

Here's the updated webrev:
http://cr.openjdk.java.net/~cushon/8190452/webrev.01/

> Regarding your questions about what to do with the -Xlint:X options. I
don't have any opinion on one way or the other, is there any reason to
change them?

I thought it was potentially surprising that -parameters will now be
silently ignored for Java 7 and earlier language levels. Warning about that
flag combination would make the new behaviour more discoverable.

Since the bug shipped in 9 and 10 there are some <v52 class files with
MethodParameters in the wild, and I've seen cases where it broke builds
using -Xlint:classfile and -Werror.

I don't think either of those are common problems. If you think we should
leave the -Xlint handling as-is that sounds good to me.

On Fri, Feb 2, 2018 at 6:17 AM, Vicente Romero <vicente.romero at oracle.com>
wrote:

> Hi Liam,
>
> The fix looks OK, regarding the test, I don't see the need for the two
> golden files as they can be constants in the test per se. In addition, the
> whole test could be self contained in only one class that compiles the
> Lib.java source. Regarding your questions about what to do with the
> -Xlint:X options. I don't have any opinion on one way or the other, is
> there any reason to change them?
>
> Thanks,
> Vicente
>
>
> On 02/01/2018 01:56 PM, Liam Miller-Cushon wrote:
>
> Bump. I'm happy to implement either of the alternatives I mentioned, but
> was hoping to get feedback on the approach first.
>
> On Wed, Jan 17, 2018 at 11:51 AM, Liam Miller-Cushon <cushon at google.com>
> wrote:
>
>> Please review a fix for JDK-8190452. The change causes javac to not emit MethodParameters
>> attributes when targeting v51 class files.
>>
>> The change implements the suggestion from this thread:
>> http://mail.openjdk.java.net/pipermail/compiler-dev/2018-Jan
>> uary/011579.html
>>
>> There are two related changes that may be worth considering:
>> * now that -parameters will be ignored when compiling with --release < 8,
>> should this combination of flags result in a warning if -Xlint:options is
>> enabled?
>> * since this wasn't fixed in JDK 9, there are v51 class files in the wild
>> that contain unexpected MethodParameters attributes. Should
>> -Xlint:classfile be relaxed to avoid warning on those?
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8190452
>> webrev: http://cr.openjdk.java.net/~cushon/8190452/webrev.00/
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20180202/33b7b2a1/attachment.html>