On Wed, 8 Nov 2023 00:30:05 GMT, Chen Liang <li...@openjdk.org> wrote:

>> Mandy Chung has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Simplify the transformation code and easier to read
>
> test/jdk/jdk/lambda/separate/ClassToInterfaceConverter.java line 43:
> 
>> 41:         //  convert it to invokeinterface
>> 42:         CodeTransform ct = (b, e) -> {
>> 43:             if (e instanceof InvokeInstruction i && i.owner() == 
>> classModel.thisClass()) {
> 
> Suggestion:
> 
>             if (e instanceof InvokeInstruction i && 
> i.owner().equals(classModel.thisClass())) {
> 
> `ClassDesc` has to be compared by equality. This piece of code works 
> accidentally due to Classfile API caching descriptor in CP objects, but if 
> the `ClassDesc` is from a `DynamicConstantDesc`, then it breaks.

`owner()` returns `ClassEntry` not `ClassDesc`.

This is to compare the owner is `this_class`, i.e. `MethodRef` references a 
method in this class.  This breaks only if the compiled code has multiple 
`CONSTANT_Class_info` entries pointing to the same name of this class.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16526#discussion_r1385873267

Reply via email to