From d2e9b539336ce2a532f940840323e1d6f17716b5 Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Wed, 27 Sep 2017 16:29:21 -0700 Subject: [PATCH] Fix AIOOBE when compiling method references involving varargs. The construction of lambdas for method references is very fragile and should be cleaned up. Bug: #9550 Bug-Link: https://github.com/gwtproject/gwt/issues/9550 Change-Id: I4e3156468cefe645421a41fe49271f51f3a23feb --- .../gwt/dev/jjs/impl/GwtAstBuilder.java | 49 +++--- .../google/gwt/dev/jjs/test/Java8Test.java | 159 ++++++++++++++++++ .../google/gwt/dev/jjs/test/Java8Test.java | 6 +- 3 files changed, 191 insertions(+), 23 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java index 688db657153..eafcf3e28f5 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java @@ -1854,8 +1854,10 @@ public void endVisit(ReferenceExpression x, BlockScope blockScope) { // Comparator. // The first argument serves as the qualifier, so for example, the method dispatch looks // like this: int compare(T a, T b) { a.compareTo(b); } - if (!hasQualifier && !referredMethod.isStatic() && instance == null && - samMethod.getParams().size() == referredMethod.getParams().size() + 1) { + if (!hasQualifier + && !referredMethod.isStatic() + && !referredMethod.isConstructor() + && instance == null) { // the instance qualifier is the first parameter in this case. // Needs to be cast the actual type due to generics. instance = new JCastOperation(info, typeMap.get(referredMethodBinding.declaringClass), @@ -1890,8 +1892,14 @@ public void endVisit(ReferenceExpression x, BlockScope blockScope) { // interface Foo { m(int x, int y); } bound to reference foo(int... args) // if varargs and incoming param is not already a var-arg, we'll need to convert // trailing args of the target interface into an array + boolean isVarargArgumentSuppliedDirectlyAsAnArray = + referredMethodBinding.isVarargs() + && samBinding.parameters.length == referredMethodBinding.parameters.length + && samBinding.parameters[varArg] + .isCompatibleWith(referredMethodBinding.parameters[varArg]); + if (referredMethodBinding.isVarargs() - && !samBinding.parameters[varArg].isArrayType()) { + && !isVarargArgumentSuppliedDirectlyAsAnArray) { varArgInitializers = Lists.newArrayList(); } @@ -1900,14 +1908,23 @@ public void endVisit(ReferenceExpression x, BlockScope blockScope) { JExpression paramExpr = param.makeRef(info); // params may need to be boxed or unboxed TypeBinding destParam = null; - // The method declared in the functional interface might have more parameters than the - // method referred by the method reference. In the case of an instance method without + + int declarationParameterOffset = + declarationSamBinding.parameters.length + - referredMethodBinding.parameters.length; + // The method declared in the functional interface might have more or less parameters than + // the method referred by the method reference. In the case of an instance method without // an explicit qualifier (A::m vs instance::m) the method in the functional interface will // have an additional parameter for the instance preceding all the method parameters. + // So truncate the value of the index to refer to the right parameter. + int declarationParameterIndex = Math.max(0, + Math.min( + paramNumber + + declarationParameterOffset, + declarationSamBinding.parameters.length - 1) + ); TypeBinding samParameterBinding = - declarationSamBinding.parameters[paramNumber - + (declarationSamBinding.parameters.length - - referredMethodBinding.parameters.length)]; + declarationSamBinding.parameters[declarationParameterIndex]; // if it is not the trailing param or varargs, or interface method is already varargs if (varArgInitializers == null || !referredMethodBinding.isVarargs() @@ -4408,11 +4425,11 @@ private static JExpression singleExpressionFromExpressionList(SourceInfo info, } private boolean hasQualifier(ReferenceExpression x) { - return (Boolean) accessPrivateField(JdtPrivateHacks.haveReceiverField, x); + return !x.isTypeAccess(); } private TypeBinding getCollectionElementTypeBinding(ForeachStatement x) { - return (TypeBinding) accessPrivateField(JdtPrivateHacks.collectionElementTypeField, x); + return (TypeBinding) accessPrivateField(JdtPrivateHacks.collectionElementTypeField, x); } private Object accessPrivateField(Field field, ASTNode astNode) { @@ -4428,10 +4445,6 @@ static class JdtPrivateHacks { * Reflective access to {@link ForeachStatement#collectionElementType}. */ private static final Field collectionElementTypeField; - /** - * Reflective access to {@link ReferenceExpression#haveReceiver}. - */ - private static final Field haveReceiverField; static { try { @@ -4443,14 +4456,6 @@ static class JdtPrivateHacks { "Unexpectedly unable to access ForeachStatement.collectionElementType via reflection", e); } - - try { - haveReceiverField = ReferenceExpression.class.getDeclaredField("haveReceiver"); - haveReceiverField.setAccessible(true); - } catch (Exception e) { - throw new RuntimeException( - "Unexpectedly unable to access ReferenceExpression.haveReceiver via reflection", e); - } } } } diff --git a/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java8Test.java b/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java8Test.java index 782a42c69b8..e9c7aec87eb 100644 --- a/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java8Test.java +++ b/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java8Test.java @@ -1453,6 +1453,26 @@ interface MyFunction2 { V apply(T t, U u); } + @FunctionalInterface + interface MyFunction3 { + W apply(T t, U u, V v); + } + + @FunctionalInterface + interface IntFunction1 { + U apply(int t); + } + + @FunctionalInterface + interface IntFunction2 { + V apply(int t, int u); + } + + @FunctionalInterface + interface IntFunction3 { + W apply(int t, int u, int v); + } + public void testMethodReference_implementedInSuperclass() { MyFunction1 toString = StringBuilder::toString; assertEquals("Hello", toString.apply(new StringBuilder("Hello"))); @@ -1465,6 +1485,145 @@ public void testMethodReference_genericTypeParameters() { new Some("Hell", concat), "Hell", "o", concat); } + static String concat(String... strs) { + String result = ""; + for (String s : strs) { + result += s; + } + return result; + } + + static String anotherConcat(String s1, String s2, String... strs) { + String result = s1 + s2; + for (String s : strs) { + result += s; + } + return result; + } + + public String instanceConcat(String... strs) { + String result = ""; + for (String s : strs) { + result += s; + } + return result; + } + + public String anotherInstanceConcat(String s1, String... strs) { + String result = s1; + for (String s : strs) { + result += s; + } + return result; + } + + private static class ClassWithVarArgsConstructor { + private class Inner { + private int sum; + Inner(int i, Integer... nums) { + this.sum = ClassWithVarArgsConstructor.this.sum + i; + for (Integer n: nums) { + sum += n; + } + } + } + + private int sum; + ClassWithVarArgsConstructor(int i, Integer... nums) { + sum = i; + for (Integer n: nums) { + sum += n; + } + } + + private MyFunction1 createInner1Param() { + return (MyFunction1) Inner::new; + } + + private MyFunction2 createInner2Param() { + return (MyFunction2) Inner::new; + } + + private MyFunction3 createInner3Param() { + return (MyFunction3) Inner::new; + } + + private MyFunction2 createInner2ParamArray() { + return (MyFunction2) Inner::new; + } + + private IntFunction1 createInner1IntParam() { + return (IntFunction1) Inner::new; + } + + private IntFunction2 createInner2IntParam() { + return (IntFunction2) Inner::new; + } + + private IntFunction3 createInner3IntParam() { + return (IntFunction3) Inner::new; + } + } + + public void testMethodReference_varargs() { + // More functional arguments than varargs + MyFunction2 concat = Java8Test::concat; + assertEquals("ab", concat.apply("a", "b")); + + // Less functional arguments than varargs + MyFunction2 anotherConcat = Java8Test::anotherConcat; + assertEquals("ab", anotherConcat.apply("a", "b")); + + MyFunction2 instanceConcat = Java8Test::instanceConcat; + assertEquals("a", instanceConcat.apply(this, "a")); + + MyFunction2 anotherInstanceConcat = Java8Test::anotherInstanceConcat; + assertEquals("a", anotherInstanceConcat.apply(this, "a")); + + // constructor varargs + MyFunction1 constructor1Param = + ClassWithVarArgsConstructor::new; + assertEquals(1, constructor1Param.apply(1).sum); + + MyFunction2 constructor2Param = + ClassWithVarArgsConstructor::new; + assertEquals(3, constructor2Param.apply(1, 2).sum); + + MyFunction3 constructor3Param = + ClassWithVarArgsConstructor::new; + assertEquals(6, constructor3Param.apply(1, 2, 3).sum); + + MyFunction2 constructor2ParamArray = + ClassWithVarArgsConstructor::new; + assertEquals(6, constructor2ParamArray.apply(1, new Integer[] {2, 3}).sum); + + // constructor varargs + autoboxing + IntFunction1 constructor1IntParam = + ClassWithVarArgsConstructor::new; + assertEquals(1, constructor1IntParam.apply(1).sum); + + IntFunction2 constructor2IntParam = + ClassWithVarArgsConstructor::new; + assertEquals(3, constructor2IntParam.apply(1, 2).sum); + + IntFunction3 constructor3IntParam = + ClassWithVarArgsConstructor::new; + assertEquals(6, constructor3IntParam.apply(1, 2, 3).sum); + + ClassWithVarArgsConstructor outer = new ClassWithVarArgsConstructor(1); + + // inner class constructor varargs + assertEquals(2, outer.createInner1Param().apply(1).sum); + assertEquals(4, outer.createInner2Param().apply(1, 2).sum); + assertEquals(7, outer.createInner3Param().apply(1, 2, 3).sum); + assertEquals(7, outer.createInner2ParamArray().apply(1, new Integer[] {2, 3}).sum); + + // inner class constructor varargs + autoboxing + assertEquals(2, outer.createInner1IntParam().apply(1).sum); + assertEquals(4, outer.createInner2IntParam().apply(1, 2).sum); + assertEquals(7, outer.createInner3IntParam().apply(1, 2, 3).sum); + } + private static void testMethodReference_genericTypeParameters( Some some, T t1, T t2, MyFunction2 combine) { T t1t2 = combine.apply(t1, t2); diff --git a/user/test/com/google/gwt/dev/jjs/test/Java8Test.java b/user/test/com/google/gwt/dev/jjs/test/Java8Test.java index cafda4c1f42..12281845345 100644 --- a/user/test/com/google/gwt/dev/jjs/test/Java8Test.java +++ b/user/test/com/google/gwt/dev/jjs/test/Java8Test.java @@ -288,6 +288,10 @@ public void testMethodReference_autoboxing() { assertFalse(isGwtSourceLevel8()); } + public void testMethodReference_varargs() { + assertFalse(isGwtSourceLevel8()); + } + public void testNativeJsOverlay_lambda() { assertFalse(isGwtSourceLevel8()); } @@ -327,4 +331,4 @@ public void testVarargsFunctionalConversion() { private boolean isGwtSourceLevel8() { return JUnitShell.getCompilerOptions().getSourceLevel().compareTo(SourceLevel.JAVA8) >= 0; } -} \ No newline at end of file +}