8068573: POJO setter using [] syntax throws an exception

Wed, 14 Jan 2015 15:54:18 +0100

author
attila
date
Wed, 14 Jan 2015 15:54:18 +0100
changeset 1183
6ed91931b5a7
parent 1182
3903ddaab26a
child 1184
690acc40065e

8068573: POJO setter using [] syntax throws an exception
Reviewed-by: lagergren, jlaskey

src/jdk/internal/dynalink/beans/AbstractJavaLinker.java file | annotate | diff | comparison | revisions
src/jdk/internal/dynalink/beans/OverloadedMethod.java file | annotate | diff | comparison | revisions
src/jdk/internal/dynalink/support/TypeUtilities.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/runtime/linker/Bootstrap.java file | annotate | diff | comparison | revisions
test/script/basic/JDK-8020324.js.EXPECTED file | annotate | diff | comparison | revisions
test/script/basic/JDK-8068573.js file | annotate | diff | comparison | revisions
     1.1 --- a/src/jdk/internal/dynalink/beans/AbstractJavaLinker.java	Wed Jan 14 18:25:01 2015 +0100
     1.2 +++ b/src/jdk/internal/dynalink/beans/AbstractJavaLinker.java	Wed Jan 14 15:54:18 2015 +0100
     1.3 @@ -491,8 +491,9 @@
     1.4  
     1.5                  // We want setters that conform to "Object(O, V)". Note, we aren't doing "R(O, V)" as it might not be
     1.6                  // valid for us to convert return values proactively. Also, since we don't know what setters will be
     1.7 -                // invoked, we'll conservatively presume Object return type.
     1.8 -                final MethodType type = callSiteDescriptor.getMethodType().changeReturnType(Object.class);
     1.9 +                // invoked, we'll conservatively presume Object return type. The one exception is void return.
    1.10 +                final MethodType origType = callSiteDescriptor.getMethodType();
    1.11 +                final MethodType type = origType.returnType() == void.class ? origType : origType.changeReturnType(Object.class);
    1.12  
    1.13                  // What's below is basically:
    1.14                  //   foldArguments(guardWithTest(isNotNull, invoke, null|nextComponent.invocation),
    1.15 @@ -508,7 +509,7 @@
    1.16                  // Bind property setter handle to the expected setter type and linker services. Type is
    1.17                  // MethodHandle(Object, String, Object)
    1.18                  final MethodHandle boundGetter = MethodHandles.insertArguments(getPropertySetterHandle, 0,
    1.19 -                        CallSiteDescriptorFactory.dropParameterTypes(callSiteDescriptor, 1, 2), linkerServices);
    1.20 +                        callSiteDescriptor.changeMethodType(setterType), linkerServices);
    1.21  
    1.22                  // Cast getter to MethodHandle(O, N, V)
    1.23                  final MethodHandle typedGetter = linkerServices.asType(boundGetter, type.changeReturnType(
     2.1 --- a/src/jdk/internal/dynalink/beans/OverloadedMethod.java	Wed Jan 14 18:25:01 2015 +0100
     2.2 +++ b/src/jdk/internal/dynalink/beans/OverloadedMethod.java	Wed Jan 14 15:54:18 2015 +0100
     2.3 @@ -123,7 +123,6 @@
     2.4          varArgMethods = new ArrayList<>(methodHandles.size());
     2.5          final int argNum = callSiteType.parameterCount();
     2.6          for(MethodHandle mh: methodHandles) {
     2.7 -            mh = mh.asType(mh.type().changeReturnType(commonRetType));
     2.8              if(mh.isVarargsCollector()) {
     2.9                  final MethodHandle asFixed = mh.asFixedArity();
    2.10                  if(argNum == asFixed.type().parameterCount()) {
     3.1 --- a/src/jdk/internal/dynalink/support/TypeUtilities.java	Wed Jan 14 18:25:01 2015 +0100
     3.2 +++ b/src/jdk/internal/dynalink/support/TypeUtilities.java	Wed Jan 14 15:54:18 2015 +0100
     3.3 @@ -118,17 +118,13 @@
     3.4      public static Class<?> getCommonLosslessConversionType(final Class<?> c1, final Class<?> c2) {
     3.5          if(c1 == c2) {
     3.6              return c1;
     3.7 +        } else if (c1 == void.class || c2 == void.class) {
     3.8 +            return Object.class;
     3.9          } else if(isConvertibleWithoutLoss(c2, c1)) {
    3.10              return c1;
    3.11          } else if(isConvertibleWithoutLoss(c1, c2)) {
    3.12              return c2;
    3.13 -        }
    3.14 -        if(c1 == void.class) {
    3.15 -            return c2;
    3.16 -        } else if(c2 == void.class) {
    3.17 -            return c1;
    3.18 -        }
    3.19 -        if(c1.isPrimitive() && c2.isPrimitive()) {
    3.20 +        } else if(c1.isPrimitive() && c2.isPrimitive()) {
    3.21              if((c1 == byte.class && c2 == char.class) || (c1 == char.class && c2 == byte.class)) {
    3.22                  // byte + char = int
    3.23                  return int.class;
    3.24 @@ -268,20 +264,24 @@
    3.25      }
    3.26  
    3.27      /**
    3.28 -     * Determines whether a type can be converted to another without losing any
    3.29 -     * precision.
    3.30 +     * Determines whether a type can be converted to another without losing any precision. As a special case,
    3.31 +     * void is considered convertible only to Object and void, while anything can be converted to void. This
    3.32 +     * is because a target type of void means we don't care about the value, so the conversion is always
    3.33 +     * permissible.
    3.34       *
    3.35       * @param sourceType the source type
    3.36       * @param targetType the target type
    3.37       * @return true if lossless conversion is possible
    3.38       */
    3.39      public static boolean isConvertibleWithoutLoss(final Class<?> sourceType, final Class<?> targetType) {
    3.40 -        if(targetType.isAssignableFrom(sourceType)) {
    3.41 +        if(targetType.isAssignableFrom(sourceType) || targetType == void.class) {
    3.42              return true;
    3.43          }
    3.44          if(sourceType.isPrimitive()) {
    3.45              if(sourceType == void.class) {
    3.46 -                return false; // Void can't be losslessly represented by any type
    3.47 +                // Void should be losslessly representable by Object, either as null or as a custom value that
    3.48 +                // can be set with DynamicLinkerFactory.setAutoConversionStrategy.
    3.49 +                return targetType == Object.class;
    3.50              }
    3.51              if(targetType.isPrimitive()) {
    3.52                  return isProperPrimitiveLosslessSubtype(sourceType, targetType);
     4.1 --- a/src/jdk/nashorn/internal/runtime/linker/Bootstrap.java	Wed Jan 14 18:25:01 2015 +0100
     4.2 +++ b/src/jdk/nashorn/internal/runtime/linker/Bootstrap.java	Wed Jan 14 15:54:18 2015 +0100
     4.3 @@ -68,6 +68,8 @@
     4.4  
     4.5      private static final MethodHandleFunctionality MH = MethodHandleFactory.getFunctionality();
     4.6  
     4.7 +    private static final MethodHandle VOID_TO_OBJECT = MH.constant(Object.class, ScriptRuntime.UNDEFINED);
     4.8 +
     4.9      /**
    4.10       * The default dynalink relink threshold for megamorphisism is 8. In the case
    4.11       * of object fields only, it is fine. However, with dual fields, in order to get
    4.12 @@ -481,14 +483,16 @@
    4.13      private static MethodHandle unboxReturnType(final MethodHandle target, final MethodType newType) {
    4.14          final MethodType targetType = target.type();
    4.15          final Class<?> oldReturnType = targetType.returnType();
    4.16 +        final Class<?> newReturnType = newType.returnType();
    4.17          if (TypeUtilities.isWrapperType(oldReturnType)) {
    4.18 -            final Class<?> newReturnType = newType.returnType();
    4.19              if (newReturnType.isPrimitive()) {
    4.20                  // The contract of setAutoConversionStrategy is such that the difference between newType and targetType
    4.21                  // can only be JLS method invocation conversions.
    4.22                  assert TypeUtilities.isMethodInvocationConvertible(oldReturnType, newReturnType);
    4.23                  return MethodHandles.explicitCastArguments(target, targetType.changeReturnType(newReturnType));
    4.24              }
    4.25 +        } else if (oldReturnType == void.class && newReturnType == Object.class) {
    4.26 +            return MethodHandles.filterReturnValue(target, VOID_TO_OBJECT);
    4.27          }
    4.28          return target;
    4.29      }
     5.1 --- a/test/script/basic/JDK-8020324.js.EXPECTED	Wed Jan 14 18:25:01 2015 +0100
     5.2 +++ b/test/script/basic/JDK-8020324.js.EXPECTED	Wed Jan 14 15:54:18 2015 +0100
     5.3 @@ -17,7 +17,7 @@
     5.4  bean.readWrite = 18: 18
     5.5  obj1.readWrite: 18
     5.6  obj1.getReadWrite(): 18
     5.7 -obj1.setReadWrite(19): null
     5.8 +obj1.setReadWrite(19): undefined
     5.9  obj1.readWrite: 19
    5.10  bean.readWrite: 19
    5.11  
    5.12 @@ -52,7 +52,7 @@
    5.13  PropertyBind.staticReadWrite = 26: 26
    5.14  obj2.staticReadWrite: 26
    5.15  obj2.getStaticReadWrite(): 26
    5.16 -obj2.setStaticReadWrite(27): null
    5.17 +obj2.setStaticReadWrite(27): undefined
    5.18  obj2.staticReadWrite: 27
    5.19  PropertyBind.staticReadWrite: 27
    5.20  
     6.1 --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
     6.2 +++ b/test/script/basic/JDK-8068573.js	Wed Jan 14 15:54:18 2015 +0100
     6.3 @@ -0,0 +1,57 @@
     6.4 +/*
     6.5 + * Copyright (c) 2014 Oracle and/or its affiliates. All rights reserved.
     6.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
     6.7 + * 
     6.8 + * This code is free software; you can redistribute it and/or modify it
     6.9 + * under the terms of the GNU General Public License version 2 only, as
    6.10 + * published by the Free Software Foundation.
    6.11 + * 
    6.12 + * This code is distributed in the hope that it will be useful, but WITHOUT
    6.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
    6.14 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
    6.15 + * version 2 for more details (a copy is included in the LICENSE file that
    6.16 + * accompanied this code).
    6.17 + * 
    6.18 + * You should have received a copy of the GNU General Public License version
    6.19 + * 2 along with this work; if not, write to the Free Software Foundation,
    6.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
    6.21 + * 
    6.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
    6.23 + * or visit www.oracle.com if you need additional information or have any
    6.24 + * questions.
    6.25 + */
    6.26 +
    6.27 +/**
    6.28 + * JDK-8068573: POJO setter using [] syntax throws an exception
    6.29 + *
    6.30 + * @test
    6.31 + * @run
    6.32 + */
    6.33 +
    6.34 +// Invoke a setter using []. It's important that the setter returns void.
    6.35 +var pb = new (Java.type("jdk.nashorn.test.models.PropertyBind"))
    6.36 +var n = "writeOnly";
    6.37 +pb[n] = 2;
    6.38 +Assert.assertEquals(pb.peekWriteOnly(), 2);
    6.39 +
    6.40 +// Invoke an overloaded setter using []. It's important that one of the 
    6.41 +// overloads returns void.
    6.42 +var os = new (Java.type("jdk.nashorn.test.models.OverloadedSetter"))
    6.43 +var n2 = "color";
    6.44 +os[n2] = 3; // exercise int overload
    6.45 +Assert.assertEquals(os.peekColor(), "3");
    6.46 +os[n2] = "blue";  // exercise string overload
    6.47 +Assert.assertEquals(os.peekColor(), "blue");
    6.48 +for each(var x in [42, "42"]) {
    6.49 +  os[n2] = x; // exercise both overloads in the same call site
    6.50 +  Assert.assertEquals(os.peekColor(), "42");
    6.51 +}
    6.52 +
    6.53 +// Invoke an overloaded method using [], repeatedly in the same call 
    6.54 +// site. It's important that one of the overloads returns void.
    6.55 +var n3="foo";
    6.56 +var param=["xyz", 1, "zyx", 2];
    6.57 +var expected=["boo", void 0, "boo", void 0];
    6.58 +for(var i in param) {
    6.59 +  Assert.assertEquals(os[n3](param[i]), expected[i]);
    6.60 +}

mercurial