8192970: Element getters/setters with fixed key fail to link properly
authorattila
Fri, 08 Dec 2017 11:48:38 +0100
changeset 48219 f3b561b13ddf
parent 48218 8ad12da0cbc7
child 48220 ef5199ed52f0
8192970: Element getters/setters with fixed key fail to link properly Reviewed-by: hannesw, sundar
src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeanLinker.java
test/nashorn/src/jdk/dynalink/beans/test/BeanLinkerTest.java
--- a/src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeanLinker.java	Fri Dec 08 09:48:10 2017 +0100
+++ b/src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeanLinker.java	Fri Dec 08 11:48:38 2017 +0100
@@ -205,9 +205,8 @@
             return nextComponent;
         }
 
-        return guardComponentWithRangeCheck(gicact, linkerServices,
-                callSiteDescriptor, nextComponent, new Binder(linkerServices, callSiteType, typedName),
-                isFixedKey ? NULL_GETTER_1 : NULL_GETTER_2);
+        return guardComponentWithRangeCheck(gicact, callSiteType, nextComponent,
+                new Binder(linkerServices, callSiteType, typedName), isFixedKey ? NULL_GETTER_1 : NULL_GETTER_2);
     }
 
     private static class GuardedInvocationComponentAndCollectionType {
@@ -276,21 +275,19 @@
     }
 
     private static GuardedInvocationComponent guardComponentWithRangeCheck(
-            final GuardedInvocationComponentAndCollectionType gicact, final LinkerServices linkerServices,
-            final CallSiteDescriptor callSiteDescriptor, final GuardedInvocationComponent nextComponent, final Binder binder,
-            final MethodHandle noOp) {
-        final MethodType callSiteType = callSiteDescriptor.getMethodType();
+            final GuardedInvocationComponentAndCollectionType gicact, final MethodType callSiteType,
+            final GuardedInvocationComponent nextComponent, final Binder binder, final MethodHandle noOp) {
 
         final MethodHandle checkGuard;
         switch(gicact.collectionType) {
             case LIST:
-                checkGuard = convertArgToNumber(RANGE_CHECK_LIST, linkerServices, callSiteDescriptor);
+                checkGuard = binder.convertArgToNumber(RANGE_CHECK_LIST);
                 break;
             case MAP:
-                checkGuard = linkerServices.filterInternalObjects(CONTAINS_MAP);
+                checkGuard = binder.linkerServices.filterInternalObjects(CONTAINS_MAP);
                 break;
             case ARRAY:
-                checkGuard = convertArgToNumber(RANGE_CHECK_ARRAY, linkerServices, callSiteDescriptor);
+                checkGuard = binder.convertArgToNumber(RANGE_CHECK_ARRAY);
                 break;
             default:
                 throw new AssertionError();
@@ -301,7 +298,7 @@
         if (nextComponent != null) {
             finalNextComponent = nextComponent;
         } else {
-            finalNextComponent = createGuardedInvocationComponentAsType(noOp, callSiteType, linkerServices);
+            finalNextComponent = createGuardedInvocationComponentAsType(noOp, callSiteType, binder.linkerServices);
         }
 
         final GuardedInvocationComponent gic = gicact.gic;
@@ -377,18 +374,6 @@
         return intIndex;
     }
 
-    private static MethodHandle convertArgToNumber(final MethodHandle mh, final LinkerServices ls, final CallSiteDescriptor desc) {
-        final Class<?> sourceType = desc.getMethodType().parameterType(1);
-        if(TypeUtilities.isMethodInvocationConvertible(sourceType, Number.class)) {
-            return mh;
-        } else if(ls.canConvert(sourceType, Number.class)) {
-            final MethodHandle converter = ls.getTypeConverter(sourceType, Number.class);
-            return MethodHandles.filterArguments(mh, 1, converter.asType(converter.type().changeReturnType(
-                    mh.type().parameterType(1))));
-        }
-        return mh;
-    }
-
     /**
      * Contains methods to adapt an item getter/setter method handle to the requested type, optionally binding it to a
      * fixed key first.
@@ -412,6 +397,18 @@
             return bindToFixedKey(Guards.asType(handle, methodType));
         }
 
+        /*private*/ MethodHandle convertArgToNumber(final MethodHandle mh) {
+            final Class<?> sourceType = methodType.parameterType(1);
+            if(TypeUtilities.isMethodInvocationConvertible(sourceType, Number.class)) {
+                return mh;
+            } else if(linkerServices.canConvert(sourceType, Number.class)) {
+                final MethodHandle converter = linkerServices.getTypeConverter(sourceType, Number.class);
+                return MethodHandles.filterArguments(mh, 1, converter.asType(converter.type().changeReturnType(
+                        mh.type().parameterType(1))));
+            }
+            return mh;
+        }
+
         private MethodHandle bindToFixedKey(final MethodHandle handle) {
             return fixedKey == null ? handle : MethodHandles.insertArguments(handle, 1, fixedKey);
         }
@@ -506,8 +503,7 @@
             return gic.replaceInvocation(binder.bind(invocation));
         }
 
-        return guardComponentWithRangeCheck(gicact, linkerServices, callSiteDescriptor,
-                nextComponent, binder, isFixedKey ? NO_OP_SETTER_2 : NO_OP_SETTER_3);
+        return guardComponentWithRangeCheck(gicact, callSiteType, nextComponent, binder, isFixedKey ? NO_OP_SETTER_2 : NO_OP_SETTER_3);
     }
 
     private static final MethodHandle GET_COLLECTION_LENGTH = Lookup.PUBLIC.findVirtual(Collection.class, "size",
--- a/test/nashorn/src/jdk/dynalink/beans/test/BeanLinkerTest.java	Fri Dec 08 09:48:10 2017 +0100
+++ b/test/nashorn/src/jdk/dynalink/beans/test/BeanLinkerTest.java	Fri Dec 08 11:48:38 2017 +0100
@@ -243,13 +243,56 @@
         Assert.assertEquals((int) cs.getTarget().invoke(list, 1), (int) list.get(1));
         Assert.assertEquals((int) cs.getTarget().invoke(list, 2), (int) list.get(2));
         try {
-            final int x = (int) cs.getTarget().invoke(list, -1);
+            cs.getTarget().invoke(list, -1);
             throw new RuntimeException("expected IndexOutOfBoundsException");
         } catch (final IndexOutOfBoundsException ex) {
         }
 
         try {
-            final int x = (int) cs.getTarget().invoke(list, list.size());
+            cs.getTarget().invoke(list, list.size());
+            throw new RuntimeException("expected IndexOutOfBoundsException");
+        } catch (final IndexOutOfBoundsException ex) {
+        }
+    }
+
+    private Object invokeWithFixedKey(boolean publicLookup, Operation op, Object name, MethodType mt, Object... args) throws Throwable {
+        return createCallSite(publicLookup, op.named(name), mt).getTarget().invokeWithArguments(args);
+    }
+
+    @Test(dataProvider = "flags")
+    public void getElementWithFixedKeyTest(final boolean publicLookup) throws Throwable {
+        final MethodType mt = MethodType.methodType(int.class, Object.class);
+
+        final int[] arr = {23, 42};
+        Assert.assertEquals((int) invokeWithFixedKey(publicLookup, GET_ELEMENT, 0, mt, arr), 23);
+        Assert.assertEquals((int) invokeWithFixedKey(publicLookup, GET_ELEMENT, 1, mt, arr), 42);
+        try {
+            invokeWithFixedKey(publicLookup, GET_ELEMENT, -1, mt, arr);
+            throw new RuntimeException("expected ArrayIndexOutOfBoundsException");
+        } catch (final ArrayIndexOutOfBoundsException ex) {
+        }
+
+        try {
+            invokeWithFixedKey(publicLookup, GET_ELEMENT, arr.length, mt, arr);
+            throw new RuntimeException("expected ArrayIndexOutOfBoundsException");
+        } catch (final ArrayIndexOutOfBoundsException ex) {
+        }
+
+        final List<Integer> list = new ArrayList<>();
+        list.add(23);
+        list.add(430);
+        list.add(-4354);
+        for (int i = 0; i < 3; ++i) {
+            Assert.assertEquals((int) invokeWithFixedKey(publicLookup, GET_ELEMENT, i, mt, list), (int) list.get(i));
+        }
+        try {
+            invokeWithFixedKey(publicLookup, GET_ELEMENT, -1, mt, list);
+            throw new RuntimeException("expected IndexOutOfBoundsException");
+        } catch (final IndexOutOfBoundsException ex) {
+        }
+
+        try {
+            invokeWithFixedKey(publicLookup, GET_ELEMENT, list.size(), mt, list);
             throw new RuntimeException("expected IndexOutOfBoundsException");
         } catch (final IndexOutOfBoundsException ex) {
         }
@@ -286,7 +329,9 @@
         cs.getTarget().invoke(list, 0, -list.get(0));
         Assert.assertEquals((int) list.get(0), -23);
         cs.getTarget().invoke(list, 1, -430);
+        Assert.assertEquals((int) list.get(1), -430);
         cs.getTarget().invoke(list, 2, 4354);
+        Assert.assertEquals((int) list.get(2), 4354);
         try {
             cs.getTarget().invoke(list, -1, 343);
             throw new RuntimeException("expected IndexOutOfBoundsException");
@@ -301,6 +346,52 @@
     }
 
     @Test(dataProvider = "flags")
+    public void setElementWithFixedKeyTest(final boolean publicLookup) throws Throwable {
+        final MethodType mt = MethodType.methodType(void.class, Object.class, int.class);
+
+        final int[] arr = {23, 42};
+        invokeWithFixedKey(publicLookup, SET_ELEMENT, 0, mt, arr, 0);
+        Assert.assertEquals(arr[0], 0);
+        invokeWithFixedKey(publicLookup, SET_ELEMENT, 1, mt, arr, -5);
+        Assert.assertEquals(arr[1], -5);
+
+        try {
+            invokeWithFixedKey(publicLookup, SET_ELEMENT, -1, mt, arr, 12);
+            throw new RuntimeException("expected ArrayIndexOutOfBoundsException");
+        } catch (final ArrayIndexOutOfBoundsException ex) {
+        }
+
+        try {
+            invokeWithFixedKey(publicLookup, SET_ELEMENT, arr.length, mt, arr, 20);
+            throw new RuntimeException("expected ArrayIndexOutOfBoundsException");
+        } catch (final ArrayIndexOutOfBoundsException ex) {
+        }
+
+        final List<Integer> list = new ArrayList<>();
+        list.add(23);
+        list.add(430);
+        list.add(-4354);
+
+        invokeWithFixedKey(publicLookup, SET_ELEMENT, 0, mt, list, -list.get(0));
+        Assert.assertEquals((int) list.get(0), -23);
+        invokeWithFixedKey(publicLookup, SET_ELEMENT, 1, mt, list, -430);
+        Assert.assertEquals((int) list.get(1), -430);
+        invokeWithFixedKey(publicLookup, SET_ELEMENT, 2, mt, list, 4354);
+        Assert.assertEquals((int) list.get(2), 4354);
+        try {
+            invokeWithFixedKey(publicLookup, SET_ELEMENT, -1, mt, list, 343);
+            throw new RuntimeException("expected IndexOutOfBoundsException");
+        } catch (final IndexOutOfBoundsException ex) {
+        }
+
+        try {
+            invokeWithFixedKey(publicLookup, SET_ELEMENT, list.size(), mt, list, 43543);
+            throw new RuntimeException("expected IndexOutOfBoundsException");
+        } catch (final IndexOutOfBoundsException ex) {
+        }
+    }
+
+    @Test(dataProvider = "flags")
     public void newObjectTest(final boolean publicLookup) {
         final MethodType mt = MethodType.methodType(Object.class, Object.class);
         final CallSite cs = createCallSite(publicLookup, NEW, mt);