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

[Vicente Romero]

On 02/03/2018 12:37 AM, Liam Miller-Cushon wrote:
> 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.

yes I was thinking about class reading but I'm OK with the current 
version of the patch

>
> Here's the updated webrev: 
> http://cr.openjdk.java.net/~cushon/8190452/webrev.01/ 
> <http://cr.openjdk.java.net/%7Ecushon/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.

as discussed off-line with Jon, yes please add the warning, but feel 
free to do it as part of this bug or in a different one.

Vicente

>
> On Fri, Feb 2, 2018 at 6:17 AM, Vicente Romero 
> <vicente.romero at oracle.com <mailto: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 <mailto: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-January/011579.html
>>         <http://mail.openjdk.java.net/pipermail/compiler-dev/2018-January/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
>>         <https://bugs.openjdk.java.net/browse/JDK-8190452>
>>         webrev: http://cr.openjdk.java.net/~cushon/8190452/webrev.00/
>>         <http://cr.openjdk.java.net/%7Ecushon/8190452/webrev.00/>
>>
>>
>
>

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