Fix AIOOBE when compiling method references involving varargs.

The construction of lambdas for method references is very fragile
and should be cleaned up.

Bug: gwtproject#9550
Bug-Link: gwtproject#9550
Change-Id: I4e3156468cefe645421a41fe49271f51f3a23feb

dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java

@@ -1854,8 +1854,10 @@ public void endVisit(ReferenceExpression x, BlockScope blockScope) {
       // Comparator<T>.
       // 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);
-      }
     }
   }
 }

user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java8Test.java

@@ -1453,6 +1453,26 @@ interface MyFunction2<T, U, V> {
     V apply(T t, U u);
   }
 
+  @FunctionalInterface
+  interface MyFunction3<T, U, V, W> {
+    W apply(T t, U u, V v);
+  }
+
+  @FunctionalInterface
+  interface IntFunction1<U> {
+    U apply(int t);
+  }
+
+  @FunctionalInterface
+  interface IntFunction2<V> {
+    V apply(int t, int u);
+  }
+
+  @FunctionalInterface
+  interface IntFunction3<W> {
+    W apply(int t, int u, int v);
+  }
+
   public void testMethodReference_implementedInSuperclass() {
     MyFunction1<StringBuilder, String> toString = StringBuilder::toString;
     assertEquals("Hello", toString.apply(new StringBuilder("Hello")));
@@ -1465,6 +1485,145 @@ public void testMethodReference_genericTypeParameters() {
         new Some<String>("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<Integer, Inner> createInner1Param() {
+      return (MyFunction1<Integer, Inner>) Inner::new;
+    }
+
+    private MyFunction2<Integer, Integer, Inner> createInner2Param() {
+      return (MyFunction2<Integer, Integer, Inner>) Inner::new;
+    }
+
+    private MyFunction3<Integer, Integer, Integer, Inner> createInner3Param() {
+      return (MyFunction3<Integer, Integer, Integer, Inner>) Inner::new;
+    }
+
+    private MyFunction2<Integer, Integer[], Inner> createInner2ParamArray() {
+      return (MyFunction2<Integer, Integer[], Inner>) Inner::new;
+    }
+
+    private IntFunction1<Inner> createInner1IntParam() {
+      return (IntFunction1<Inner>) Inner::new;
+    }
+
+    private IntFunction2<Inner> createInner2IntParam() {
+      return (IntFunction2<Inner>) Inner::new;
+    }
+
+    private IntFunction3<Inner> createInner3IntParam() {
+      return (IntFunction3<Inner>) Inner::new;
+    }
+  }
+
+  public void testMethodReference_varargs() {
+    // More functional arguments than varargs
+    MyFunction2<String, String, String> concat = Java8Test::concat;
+    assertEquals("ab", concat.apply("a", "b"));
+
+    // Less functional arguments than varargs
+    MyFunction2<String, String, String> anotherConcat = Java8Test::anotherConcat;
+    assertEquals("ab", anotherConcat.apply("a", "b"));
+
+    MyFunction2<Java8Test, String, String> instanceConcat = Java8Test::instanceConcat;
+    assertEquals("a", instanceConcat.apply(this, "a"));
+
+    MyFunction2<Java8Test, String, String> anotherInstanceConcat = Java8Test::anotherInstanceConcat;
+    assertEquals("a", anotherInstanceConcat.apply(this, "a"));
+
+    // constructor varargs
+    MyFunction1<Integer, ClassWithVarArgsConstructor> constructor1Param =
+        ClassWithVarArgsConstructor::new;
+    assertEquals(1, constructor1Param.apply(1).sum);
+
+    MyFunction2<Integer, Integer, ClassWithVarArgsConstructor> constructor2Param =
+        ClassWithVarArgsConstructor::new;
+    assertEquals(3, constructor2Param.apply(1, 2).sum);
+
+    MyFunction3<Integer, Integer, Integer, ClassWithVarArgsConstructor> constructor3Param =
+        ClassWithVarArgsConstructor::new;
+    assertEquals(6, constructor3Param.apply(1, 2, 3).sum);
+
+    MyFunction2<Integer, Integer[], ClassWithVarArgsConstructor> constructor2ParamArray =
+        ClassWithVarArgsConstructor::new;
+    assertEquals(6, constructor2ParamArray.apply(1, new Integer[] {2, 3}).sum);
+
+    // constructor varargs + autoboxing
+    IntFunction1<ClassWithVarArgsConstructor> constructor1IntParam =
+        ClassWithVarArgsConstructor::new;
+    assertEquals(1, constructor1IntParam.apply(1).sum);
+
+    IntFunction2<ClassWithVarArgsConstructor> constructor2IntParam =
+        ClassWithVarArgsConstructor::new;
+    assertEquals(3, constructor2IntParam.apply(1, 2).sum);
+
+    IntFunction3<ClassWithVarArgsConstructor> 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 <T> void testMethodReference_genericTypeParameters(
       Some<T> some, T t1, T t2, MyFunction2<T, T, T> combine) {
     T t1t2 = combine.apply(t1, t2);

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;
   }
-}
+}