On Mon, 12 Jun 2023 05:27:35 GMT, Chen Liang <li...@openjdk.org> wrote:
> Fixes the NPE in IndirectVarHandle.isAccessModeSupported by delegating to the > target var handle. In addition, simplified the MH caching in Indirect VH and > removed redundant overrides (already available via var form). This was split > from #13821 as suggested by Mandy Chung, which allows easy backports to > release 21 if desirable. The fix looks good. Also nice clean up. test/jdk/java/lang/invoke/VarHandles/IndirectVarHandleTest.java line 28: > 26: * @summary tests for IndirectVarHandle > 27: * @compile --enable-preview --release 22 IndirectVarHandleTest.java > 28: * @run junit/othervm --enable-preview IndirectVarHandleTest Suggestion: * @enablePreview * @run junit IndirectVarHandleTest * @summary Test VarHandle::isAccessModeSupported on indirect VarHandle produced by MethodHandles.filterCoordinates Should simply use jtreg `@enablePreview`. Suggest to move `@summary` to the end and also can be more specific. test/jdk/java/lang/invoke/VarHandles/IndirectVarHandleTest.java line 44: > 42: var lookup = MethodHandles.lookup(); > 43: var intArrayVh = MethodHandles.arrayElementVarHandle(int[].class); > 44: var addOne = lookup.bind((IntUnaryOperator) a -> a + 1, > "applyAsInt", MethodType.methodType(int.class, int.class)); Nit: a couple of long lines (L44, 48 and 55) can be wrapped into separate line. ------------- Marked as reviewed by mchung (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14409#pullrequestreview-1475930012 PR Review Comment: https://git.openjdk.org/jdk/pull/14409#discussion_r1227316722 PR Review Comment: https://git.openjdk.org/jdk/pull/14409#discussion_r1227319788