On Sat, 16 Dec 2023 17:28:13 GMT, ExE Boss <d...@openjdk.org> wrote:

>> Adam Sotona has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   added missing comment
>
> src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java 
> line 187:
> 
>> 185:         // generating bridges in the target class)
>> 186:         useImplMethodHandle = 
>> (Modifier.isProtected(implInfo.getModifiers()) &&
>> 187:                                !VerifyAccess.isSamePackage(targetClass, 
>> implInfo.getDeclaringClass())) ||
> 
> If the `implClass` is hidden, then it’s not possible to generate bytecode 
> which correctly refers to `implMethod`, so the only way is to use the method 
> handle in that case:
> Suggestion:
> 
>         useImplMethodHandle = (Modifier.isProtected(implInfo.getModifiers()) 
> &&
>                                !VerifyAccess.isSamePackage(targetClass, 
> implInfo.getDeclaringClass())) ||
>                                implClass.isHidden() ||

I think this belongs to another RFE and is out of scope of this one. In 
addition, we need extra tests to invoke the hidden class's lambda to assert its 
behavioral correctness. Currently, the previously failing test 
[`LambdaProxyCaller`](https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/test-classes/LambdaProxyCallerIsHiddenApp.java)
 only creates the hidden class for CDS testing; it doesn't actually invoke the 
lambda to test its implementation correctness.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17108#discussion_r1428875697

Reply via email to