Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class [v3]

2024-04-17 Thread Mandy Chung
On Wed, 17 Apr 2024 16:08:31 GMT, Adam Sotona  wrote:

>> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
>> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
>> 
>> This patch includes lambda implementation in a hidden class under the 
>> special handling of `useImplMethodHandle`.
>> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
>> correctly cover this test case.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - typo in comment fix
>  - applied suggested changes

Looks fine to me.   A hidden class cannot be resolved by name.   I tweaked the 
comment to make it clear.

src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java 
line 182:

> 180: // live implementation method handle to the proxy class to invoke
> 181: // directly. (javac prefers to avoid this situation by generating
> 182: // bridges in the target class)

Suggestion:

// lambda class has no access to the resolved method, or does
// 'invokestatic' on a hidden class which cannot be resolved by name.
// Instead, we need to pass the live implementation method handle to
// the proxy class to invoke directly. (javac prefers to avoid this
// situation by generating bridges in the target class)

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18810#pullrequestreview-2007006920
PR Review Comment: https://git.openjdk.org/jdk/pull/18810#discussion_r1569431448


Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class [v3]

2024-04-17 Thread Paul Sandoz
On Wed, 17 Apr 2024 16:08:31 GMT, Adam Sotona  wrote:

>> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
>> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
>> 
>> This patch includes lambda implementation in a hidden class under the 
>> special handling of `useImplMethodHandle`.
>> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
>> correctly cover this test case.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - typo in comment fix
>  - applied suggested changes

Looks good to me. I conservatively bumped the review count to 2.

-

Marked as reviewed by psandoz (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18810#pullrequestreview-2006903761


Re: RFR: 8330467: NoClassDefFoundError when lambda is in a hidden class [v3]

2024-04-17 Thread Adam Sotona
> Current implementation of `LambdaMetafactory` does not allow to use lambdas 
> in hidden classes. Invocation throws `NoClassDefFoundError` instead.
> 
> This patch includes lambda implementation in a hidden class under the special 
> handling of `useImplMethodHandle`.
> The patch also fixes `j/l/i/defineHiddenClass/BasicTest::testLambda` to 
> correctly cover this test case.
> 
> Please review.
> 
> Thanks,
> Adam

Adam Sotona has updated the pull request incrementally with two additional 
commits since the last revision:

 - typo in comment fix
 - applied suggested changes

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18810/files
  - new: https://git.openjdk.org/jdk/pull/18810/files/c637ab90..e8eb1752

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18810=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=18810=01-02

  Stats: 18 lines in 3 files changed: 15 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/18810.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18810/head:pull/18810

PR: https://git.openjdk.org/jdk/pull/18810