Re: RFR: JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior [v4]

2021-10-28 Thread Dan Smith
On Thu, 28 Oct 2021 20:26:47 GMT, Mandy Chung  wrote:

>> Classes compiled prior to the nestmate support will generate 
>> `REF_invokeSpecial` if the implementation method is a private instance 
>> method.   Since a lambda proxy class is a hidden class, a nestmate of the 
>> host class, it can invoke the private implementation method but it has to 
>> use `REF_invokeVirtual` or `REF_invokeInterface`.   In order to support the 
>> old classes running on the new runtime, LMF implementation adjusts the 
>> reference kind from `REF_invokeSpecial` to 
>> `REF_invokeVirtual/REF_invokeInterface`. 
>> 
>> This PR fixes the check properly to ensure the adjustment is only made if 
>> the implementation method is private method in the host class.
>
> Mandy Chung has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   convert to TestNG test

Marked as reviewed by dlsmith (Committer).

Ahhh, much nicer. Thanks for updating the test.

-

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


Re: RFR: JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior [v4]

2021-10-28 Thread Mandy Chung
> Classes compiled prior to the nestmate support will generate 
> `REF_invokeSpecial` if the implementation method is a private instance 
> method.   Since a lambda proxy class is a hidden class, a nestmate of the 
> host class, it can invoke the private implementation method but it has to use 
> `REF_invokeVirtual` or `REF_invokeInterface`.   In order to support the old 
> classes running on the new runtime, LMF implementation adjusts the reference 
> kind from `REF_invokeSpecial` to `REF_invokeVirtual/REF_invokeInterface`. 
> 
> This PR fixes the check properly to ensure the adjustment is only made if the 
> implementation method is private method in the host class.

Mandy Chung has updated the pull request incrementally with one additional 
commit since the last revision:

  convert to TestNG test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5901/files
  - new: https://git.openjdk.java.net/jdk/pull/5901/files/c44a5910..980be2b9

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5901=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5901=02-03

  Stats: 35 lines in 1 file changed: 18 ins; 0 del; 17 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5901.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5901/head:pull/5901

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


Re: RFR: JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior [v4]

2021-10-28 Thread Paul Sandoz
On Thu, 28 Oct 2021 20:23:41 GMT, Mandy Chung  wrote:

>> Classes compiled prior to the nestmate support will generate 
>> `REF_invokeSpecial` if the implementation method is a private instance 
>> method.   Since a lambda proxy class is a hidden class, a nestmate of the 
>> host class, it can invoke the private implementation method but it has to 
>> use `REF_invokeVirtual` or `REF_invokeInterface`.   In order to support the 
>> old classes running on the new runtime, LMF implementation adjusts the 
>> reference kind from `REF_invokeSpecial` to 
>> `REF_invokeVirtual/REF_invokeInterface`. 
>> 
>> This PR fixes the check properly to ensure the adjustment is only made if 
>> the implementation method is private method in the host class.
>
> Mandy Chung has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   convert to TestNG test

Marked as reviewed by psandoz (Reviewer).

-

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


Re: RFR: JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior [v2]

2021-10-27 Thread Mandy Chung
On Mon, 25 Oct 2021 21:39:34 GMT, Dan Smith  wrote:

> I'd suggest invoking the LMF API directly instead, testing both private and 
> non-private invokespecial MethodHandles, just making sure the rules can be 
> used without error.

That's a good idea.   I updated the test and see what you think.

-

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


Re: RFR: JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior [v3]

2021-10-27 Thread Mandy Chung
> Classes compiled prior to the nestmate support will generate 
> `REF_invokeSpecial` if the implementation method is a private instance 
> method.   Since a lambda proxy class is a hidden class, a nestmate of the 
> host class, it can invoke the private implementation method but it has to use 
> `REF_invokeVirtual` or `REF_invokeInterface`.   In order to support the old 
> classes running on the new runtime, LMF implementation adjusts the reference 
> kind from `REF_invokeSpecial` to `REF_invokeVirtual/REF_invokeInterface`. 
> 
> This PR fixes the check properly to ensure the adjustment is only made if the 
> implementation method is private method in the host class.

Mandy Chung has updated the pull request with a new target base due to a merge 
or a rebase. The incremental webrev excludes the unrelated changes brought in 
by the merge/rebase. The pull request contains five additional commits since 
the last revision:

 - Update test to test LMF with no dependency on javac behavior
 - Merge branch 'master' of https://github.com/openjdk/jdk into invokespecial
 - Merge branch 'invokespecial' of https://github.com/mlchung/jdk into 
invokespecial
 - remove filelist which was added accidentally
 - JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method 
has incorrect behavior

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5901/files
  - new: https://git.openjdk.java.net/jdk/pull/5901/files/cfdd036e..c44a5910

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5901=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5901=01-02

  Stats: 29565 lines in 721 files changed: 21974 ins; 5102 del; 2489 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5901.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5901/head:pull/5901

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


Re: RFR: JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior [v2]

2021-10-25 Thread Dan Smith
On Tue, 12 Oct 2021 16:21:33 GMT, Mandy Chung  wrote:

>> Classes compiled prior to the nestmate support will generate 
>> `REF_invokeSpecial` if the implementation method is a private instance 
>> method.   Since a lambda proxy class is a hidden class, a nestmate of the 
>> host class, it can invoke the private implementation method but it has to 
>> use `REF_invokeVirtual` or `REF_invokeInterface`.   In order to support the 
>> old classes running on the new runtime, LMF implementation adjusts the 
>> reference kind from `REF_invokeSpecial` to 
>> `REF_invokeVirtual/REF_invokeInterface`. 
>> 
>> This PR fixes the check properly to ensure the adjustment is only made if 
>> the implementation method is private method in the host class.
>
> Mandy Chung has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   remove filelist which was added accidentally

Source changes look good.

The test seems like way too much overhead for this small thing. Looks like a 
lot of the ASM code is just to verify that javac generates the test case you 
expect? I'd suggest invoking the LMF API directly instead, testing both private 
and non-private `invokespecial` MethodHandles, just making sure the rules can 
be used without error.

-

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


Re: RFR: JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior [v2]

2021-10-12 Thread Mandy Chung
On Tue, 12 Oct 2021 10:22:07 GMT, Alan Bateman  wrote:

>> Mandy Chung has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   remove filelist which was added accidentally
>
> filelist line 1:
> 
>> 1: 
>> test/jdk/java/lang/invoke/lambda/invokeSpecial/InvokeSpecialMethodImpl.java
> 
> I assume "filelist" is not meant to be in this match, I assume this is why 
> the "jdk" label was added.

Fixed.   It was accidentally added.  Thanks.

-

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


Re: RFR: JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior [v2]

2021-10-12 Thread Mandy Chung
> Classes compiled prior to the nestmate support will generate 
> `REF_invokeSpecial` if the implementation method is a private instance 
> method.   Since a lambda proxy class is a hidden class, a nestmate of the 
> host class, it can invoke the private implementation method but it has to use 
> `REF_invokeVirtual` or `REF_invokeInterface`.   In order to support the old 
> classes running on the new runtime, LMF implementation adjusts the reference 
> kind from `REF_invokeSpecial` to `REF_invokeVirtual/REF_invokeInterface`. 
> 
> This PR fixes the check properly to ensure the adjustment is only made if the 
> implementation method is private method in the host class.

Mandy Chung has updated the pull request incrementally with one additional 
commit since the last revision:

  remove filelist which was added accidentally

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5901/files
  - new: https://git.openjdk.java.net/jdk/pull/5901/files/1358214d..cfdd036e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5901=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5901=00-01

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

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


Re: RFR: JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior

2021-10-12 Thread Alan Bateman
On Mon, 11 Oct 2021 20:55:23 GMT, Mandy Chung  wrote:

> Classes compiled prior to the nestmate support will generate 
> `REF_invokeSpecial` if the implementation method is a private instance 
> method.   Since a lambda proxy class is a hidden class, a nestmate of the 
> host class, it can invoke the private implementation method but it has to use 
> `REF_invokeVirtual` or `REF_invokeInterface`.   In order to support the old 
> classes running on the new runtime, LMF implementation adjusts the reference 
> kind from `REF_invokeSpecial` to `REF_invokeVirtual/REF_invokeInterface`. 
> 
> This PR fixes the check properly to ensure the adjustment is only made if the 
> implementation method is private method in the host class.

filelist line 1:

> 1: test/jdk/java/lang/invoke/lambda/invokeSpecial/InvokeSpecialMethodImpl.java

I assume "filelist" is not meant to be in this match, I assume this is why the 
"jdk" label was added.

-

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