Wed, 14 Jan 2015 15:54:18 +0100
8068573: POJO setter using [] syntax throws an exception
Reviewed-by: lagergren, jlaskey
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 +}