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

Reply via email to