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