On Sat, 16 Dec 2023 15:54:57 GMT, Chen Liang <li...@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 548:
>
>> 546: static ClassDesc classDesc(Class<?> cls) {
>> 547: return cls.isHidden() ?
>> ClassDesc.ofInternalName(cls.getName().replace('.', '/'))
>> 548: :
>> ClassDesc.ofDescriptor(cls.descriptorString());
>
> That said, all the usages already anticipate a valid `ClassDesc` that can be
> encoded in bytecode except `implClass`. You should make 2 versions of
> `classDesc()`, one that eagerly throws for all other usages, and another
> special version that handles hidden class conversion (with the same code as
> existing method) for
> https://github.com/openjdk/jdk/pull/17108/files#diff-76236a0dd20022f749d95ad5890e2bece0c4ac212d1b2ffab90ca86875f14282R173
Split, thank you.
> src/java.base/share/classes/java/lang/invoke/TypeConvertingMethodAdapter.java
> line 162:
>
>> 160: } else {
>> 161: Class<?> src;
>> 162: if (arg == functional || functional.isPrimitive()) {
>
> `arg == functional` avoids a cast, though it's not in parity with the older
> code. Should probably restore the old cast that takes a source argument to
> avoid adding `==` checks to avoid casts.
Original cast is avoided when class names (derived from descriptors, derived
from classes) are equal.
Correct me, please, but I think that this simple `arg == functional` check does
the same job, without unnecessary String conversions and comparisons.
Or is there a case, where the cast is missing or added incorrectly?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17108#discussion_r1429931780
PR Review Comment: https://git.openjdk.org/jdk/pull/17108#discussion_r1429919564
