Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]
On Tue, 22 Jun 2021 00:45:36 GMT, Mandy Chung wrote:
>> Vicente Romero has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> updating after review comments
>
> test/jdk/java/lang/constant/MethodHandleDescTest.java line 379:
>
>> 377: for (int refKind : isInterfaceIgnored) {
>> 378: assertEquals(Kind.valueOf(refKind, false),
>> Kind.valueOf(refKind, true));
>> 379: }
>
> Can you also add the test cases to verify `Kind::valueOf` with
> `REF_invokeStatic` and `REF_invokeSpecial`?
please see them below, thanks
-
PR: https://git.openjdk.java.net/jdk17/pull/29
Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]
On Fri, 11 Jun 2021 18:17:10 GMT, Vicente Romero 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
>
> Vicente Romero has updated the pull request incrementally with one additional
> commit since the last revision:
>
> updating after review comments
Marked as reviewed by mchung (Reviewer).
src/java.base/share/classes/java/lang/constant/DirectMethodHandleDesc.java line
138:
> 136: int i = tableIndex(refKind, isInterface);
> 137: if (i >= 2 && i < TABLE.length) {
> 138: return TABLE[i];
This fix looks good. The resulting `TABLE` has two entries per `refKind` for
`isInterface` = `true` and `false`.
test/jdk/java/lang/constant/MethodHandleDescTest.java line 379:
> 377: for (int refKind : isInterfaceIgnored) {
> 378: assertEquals(Kind.valueOf(refKind, false),
> Kind.valueOf(refKind, true));
> 379: }
Can you also add the test cases to verify `Kind::valueOf` with
`REF_invokeStatic` and `REF_invokeSpecial`?
-
PR: https://git.openjdk.java.net/jdk17/pull/29
Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]
On Fri, 11 Jun 2021 18:17:10 GMT, Vicente Romero 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 > > Vicente Romero has updated the pull request incrementally with one additional > commit since the last revision: > > updating after review comments @mlchung I have uploaded a new commit, thanks for your comments - PR: https://git.openjdk.java.net/jdk17/pull/29
Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]
> 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 Vicente Romero has updated the pull request incrementally with one additional commit since the last revision: updating after review comments - Changes: - all: https://git.openjdk.java.net/jdk17/pull/29/files - new: https://git.openjdk.java.net/jdk17/pull/29/files/b6b3f87c..8ed4cdb3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17=29=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=29=00-01 Stats: 12 lines in 1 file changed: 0 ins; 9 del; 3 mod Patch: https://git.openjdk.java.net/jdk17/pull/29.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/29/head:pull/29 PR: https://git.openjdk.java.net/jdk17/pull/29
