RFR: 8226216: parameter modifiers are not visible to javac plugins across compilation boundaries

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8226216: parameter modifiers are not visible to javac plugins across compilation boundaries

Guoxiong Li-2
Hi all,

javac fails to read modifiers from the MethodParameters attribute in class files, which prevents plugins from accessing those modifiers across compilation boundaries. Because `com.sun.tools.javac.jvm.ClassReader` doesn't read and store the access flags(modifiers) from MethodParameters attribute.

This patch fixes it and adds a corresponding test case. All the existing tests of javac passed locally.

Thank you for taking the time to review.

Best Regards.

-------------

Commit messages:
 - 8226216: parameter modifiers are not visible to javac plugins across compilation boundaries

Changes: https://git.openjdk.java.net/jdk/pull/1890/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1890&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8226216
  Stats: 172 lines in 2 files changed: 171 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1890.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1890/head:pull/1890

PR: https://git.openjdk.java.net/jdk/pull/1890
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8226216: parameter modifiers are not visible to javac plugins across compilation boundaries

Jonathan Gibbons-2
On Thu, 24 Dec 2020 16:02:19 GMT, Guoxiong Li <[hidden email]> wrote:

> Hi all,
>
> javac fails to read modifiers from the MethodParameters attribute in class files, which prevents plugins from accessing those modifiers across compilation boundaries. Because `com.sun.tools.javac.jvm.ClassReader` doesn't read and store the access flags(modifiers) from MethodParameters attribute.
>
> This patch fixes it and adds a corresponding test case. All the existing tests of javac passed locally.
>
> Thank you for taking the time to review.
>
> Best Regards.

I've noted a couple of minor issues in the test.
I'll leave it to other javac folk to decide the merits of the change to `ClassReader`.
Note that the `MethodParameters` attribute may not always be present.

test/langtools/tools/javac/classreader/8226216/T8226216.java line 9:

> 7:  * published by the Free Software Foundation.  Oracle designates this
> 8:  * particular file as subject to the "Classpath" exception as provided
> 9:  * by Oracle in the LICENSE file that accompanied this code.

Tests should not contain the text `Oracle designates ... this code.` or the immediately preceding whitespace.

test/langtools/tools/javac/classreader/8226216/T8226216.java line 123:

> 121:
> 122:     @Test
> 123:     public void testParameterModifiersNotVisiable() throws Exception {

spelling: should be "Visible"

-------------

Changes requested by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1890
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8226216: parameter modifiers are not visible to javac plugins across compilation boundaries [v2]

Guoxiong Li-2
In reply to this post by Guoxiong Li-2
> Hi all,
>
> javac fails to read modifiers from the MethodParameters attribute in class files, which prevents plugins from accessing those modifiers across compilation boundaries. Because `com.sun.tools.javac.jvm.ClassReader` doesn't read and store the access flags(modifiers) from MethodParameters attribute.
>
> This patch fixes it and adds a corresponding test case. All the existing tests of javac passed locally.
>
> Thank you for taking the time to review.
>
> Best Regards.

Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:

  Modify legal header. Fix typo.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1890/files
  - new: https://git.openjdk.java.net/jdk/pull/1890/files/f064d8e8..a074937f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1890&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1890&range=00-01

  Stats: 4 lines in 1 file changed: 0 ins; 2 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1890.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1890/head:pull/1890

PR: https://git.openjdk.java.net/jdk/pull/1890
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8226216: parameter modifiers are not visible to javac plugins across compilation boundaries [v2]

Guoxiong Li-2
In reply to this post by Jonathan Gibbons-2
On Thu, 24 Dec 2020 16:18:55 GMT, Jonathan Gibbons <[hidden email]> wrote:

>> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Modify legal header. Fix typo.
>
> test/langtools/tools/javac/classreader/8226216/T8226216.java line 9:
>
>> 7:  * published by the Free Software Foundation.  Oracle designates this
>> 8:  * particular file as subject to the "Classpath" exception as provided
>> 9:  * by Oracle in the LICENSE file that accompanied this code.
>
> Tests should not contain the text `Oracle designates ... this code.` or the immediately preceding whitespace.

Fixed.

> test/langtools/tools/javac/classreader/8226216/T8226216.java line 123:
>
>> 121:
>> 122:     @Test
>> 123:     public void testParameterModifiersNotVisiable() throws Exception {
>
> spelling: should be "Visible"

Fixed.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1890
Reply | Threaded
Open this post in threaded view
|

Withdrawn: 8226216: parameter modifiers are not visible to javac plugins across compilation boundaries

duke
In reply to this post by Guoxiong Li-2
On Thu, 24 Dec 2020 16:02:19 GMT, Guoxiong Li <[hidden email]> wrote:

> Hi all,
>
> javac fails to read modifiers from the MethodParameters attribute in class files, which prevents plugins from accessing those modifiers across compilation boundaries. Because `com.sun.tools.javac.jvm.ClassReader` doesn't read and store the access flags(modifiers) from MethodParameters attribute.
>
> This patch fixes it and adds a corresponding test case. All the existing tests of javac passed locally.
>
> Thank you for taking the time to review.
>
> Best Regards.

This pull request has been closed without being integrated.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1890