On Fri, 11 Jun 2021 15:01:20 GMT, Vicente Romero <vrom...@openjdk.org> wrote:

> This PR is a copy of [PR-4416](https://github.com/openjdk/jdk/pull/4416) 
> which was intended to openjdk/jdk.
> 
> Please review this PR which is syncing the implementation of 
> `DirectMethodHandleDesc.Kind.valueOf(int)` and 
> `DirectMethodHandleDesc.Kind.valueOf(int, boolean)` with its spec. My reading 
> of the method's spec is that if the value of the refKind parameter is: 
> MethodHandleInfo.REF_invokeInterface, then 
> DirectMethodHandleDesc.Kind.valueOf(int, boolean) should be called with a 
> value of true for its second argument, currently it is invoked with false 
> regardless of the value of the refKind parameter
> 
> TIA

src/java.base/share/classes/java/lang/constant/DirectMethodHandleDesc.java line 
143:

> 141:                 }
> 142:                 if (kind.refKind == refKind &&
> 143:                         (refKind != REF_invokeStatic || refKind != 
> REF_invokeSpecial || kind.isInterface == isInterface)){

@mlchung 13 hours ago Member

It reads to me that the static initializer tries to set up the table such that 
valueOf(refKind, isInterface) should find the proper kind to return except this:

// There is not a specific Kind for interfaces
if (kind == VIRTUAL) kind = INTERFACE_VIRTUAL;

This changes the entry for REF_invokeVirtual to kind INTERFACE_VIRTUAL. Do you 
know why? If this entry is set to VIRTUAL, then each refKind has two entries in 
the table corresponding to the correct result for valueOf.

test/jdk/java/lang/constant/MethodHandleDescTest.java line 364:

> 362:     public void testKind() {
> 363:         for (Kind k : Kind.values()) {
> 364:             assertEquals(Kind.valueOf(k.refKind), 
> Kind.valueOf(k.refKind, k.refKind == MethodHandleInfo.REF_invokeInterface));

@mlchung mlchung 2 days ago Member

Looks like the test does not verify the cases specified by valueOf(int refKind, 
boolean isInterface).
i.e. For most values of refKind, there is an exact match regardless of the 
value of isInterface except REF_invokeStatic and REF_invokeSpecial.

Do you mind adding those cases?

@vicente-romero-oracle vicente-romero-oracle 22 hours ago •

hum, the implementation for valueOf(int refKind, boolean isInterface) is 
incorrect, the behavior does depends on the value of isInterface for example: 
Kind.valueOf(1, false) returns GETTER while Kind.valueOf(1, true) fails with 
java.lang.IllegalArgumentException will fix the implementation of valueOf(int 
refKind, boolean isInterface) for it to match its spec

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

PR: https://git.openjdk.java.net/jdk17/pull/29

Reply via email to