Skip to content

Commit

Permalink
Fix bad code generation involving method references and generics.
Browse files Browse the repository at this point in the history 
When a method reference is assigned to a generic functional interface
like:

  interface I<T> { boolean m(T o); }

  public class C { public boolean f(); }

  ....
  I<C> methodReference = C::f;

The implementation of the method reference is roughly

  class SynthesizedImplementation implements I {
    public boolean m(Object o) {
       o.f();
    }

Note the lack of generics (internally GWT does all the transformations
suing raw types).

Because the type of "o" is object, the above code is not type correct and
some of the compiler passes get confused (in the small repro example it
is the Devirtualizer that gets confused and devitualizes that method as
if it were an Object method).

This patch correctly inserts the required cast.

  class SynthesizedImplementation implements I {
    public boolean m(Object o) {
       ((C) o).f();
    }

Change-Id: I2286516157aa58181987629891c3c2c00951bdd1
Bug-Link:#9214
  • Loading branch information
@rluble
rluble authored and Gerrit Code Review committed Nov 13, 2015
1 parent adca129 commit 184e750
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 24 deletions.
32 changes: 21 additions & 11 deletions dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1742,7 +1742,7 @@ public void endVisit(ReferenceExpression x, BlockScope blockScope) {

// Calculate what type this reference is going to bind to, and what single abstract method
TypeBinding binding = x.expectedType();
MethodBinding samBinding = binding.getSingleAbstractMethod(blockScope, false);
MethodBinding samBinding = binding.getSingleAbstractMethod(blockScope, false).original();

// Get the interface method is binds to
JMethod interfaceMethod = typeMap.get(samBinding);
Expand Down Expand Up @@ -1845,7 +1845,9 @@ public void endVisit(ReferenceExpression x, BlockScope blockScope) {
if (!haveReceiver && !referredMethod.isStatic() && instance == null &&
samMethod.getParams().size() == referredMethod.getParams().size() + 1) {
// the instance qualifier is the first parameter in this case.
instance = new JParameterRef(info, paramIt.next());
// Needs to be cast the actual type due to generics.
instance = new JCastOperation(info, typeMap.get(referredMethodBinding.declaringClass),
new JParameterRef(info, paramIt.next()));
}
JMethodCall samCall = null;

Expand All @@ -1871,13 +1873,13 @@ public void endVisit(ReferenceExpression x, BlockScope blockScope) {

// need to build up an array of passed parameters if we have varargs
List<JExpression> varArgInitializers = null;
int varArg = x.binding.parameters.length - 1;
int varArg = referredMethodBinding.parameters.length - 1;

// 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
if (x.binding.isVarargs() && !samBinding.parameters[varArg].isArrayType()) {
varArgInitializers = new ArrayList<JExpression>();
if (referredMethodBinding.isVarargs() && !samBinding.parameters[varArg].isArrayType()) {
varArgInitializers = Lists.newArrayList();
}

while (paramIt.hasNext()) {
Expand All @@ -1886,14 +1888,14 @@ public void endVisit(ReferenceExpression x, BlockScope blockScope) {
// params may need to be boxed or unboxed
TypeBinding destParam = null;
// if it is not the trailing param or varargs, or interface method is already varargs
if (varArgInitializers == null || !x.binding.isVarargs() || (paramNumber < varArg)) {
destParam = x.binding.parameters[paramNumber];
if (varArgInitializers == null || !referredMethodBinding.isVarargs() || (paramNumber < varArg)) {
destParam = referredMethodBinding.parameters[paramNumber];
paramExpr = boxOrUnboxExpression(paramExpr, samBinding.parameters[paramNumber],
destParam);
samCall.addArg(paramExpr);
} else if (!samBinding.parameters[paramNumber].isArrayType()) {
// else add trailing parameters to var-args initializer list for an array
destParam = x.binding.parameters[varArg].leafComponentType();
destParam = referredMethodBinding.parameters[varArg].leafComponentType();
paramExpr = boxOrUnboxExpression(paramExpr, samBinding.parameters[paramNumber],
destParam);
varArgInitializers.add(paramExpr);
Expand All @@ -1904,7 +1906,8 @@ public void endVisit(ReferenceExpression x, BlockScope blockScope) {
// add trailing new T[] { initializers } var-arg array
if (varArgInitializers != null) {
JArrayType lastParamType =
(JArrayType) typeMap.get(x.binding.parameters[x.binding.parameters.length - 1]);
(JArrayType) typeMap.get(
referredMethodBinding.parameters[referredMethodBinding.parameters.length - 1]);
JNewArray newArray =
JNewArray.createArrayWithInitializers(info, lastParamType, varArgInitializers);
samCall.addArg(newArray);
Expand All @@ -1913,7 +1916,7 @@ public void endVisit(ReferenceExpression x, BlockScope blockScope) {
// TODO(rluble): Make this a call to JjsUtils.makeMethodEndStatement once boxing/unboxing
// is handled there.
if (samMethod.getType() != JPrimitiveType.VOID) {
JExpression samExpression = boxOrUnboxExpression(samCall, x.binding.returnType,
JExpression samExpression = boxOrUnboxExpression(samCall, referredMethodBinding.returnType,
samBinding.returnType);
samMethodBody.getBlock().addStmt(simplify(samExpression, x).makeReturnStatement());
} else {
Expand Down Expand Up @@ -1983,7 +1986,11 @@ private JExpression boxOrUnboxExpression(JExpression expr, TypeBinding fromType,
return unbox(expr, implicitConversion);
}

return expr;
TypeBinding castToType = fromType.genericCast(toType);
if (castToType == null) {
return expr;
}
return new JCastOperation(expr.getSourceInfo(), typeMap.get(castToType), expr);
}

@Override
Expand Down Expand Up @@ -3649,6 +3656,9 @@ public MethodInfo(JMethod method, JMethodBody methodBody, MethodScope methodScop
* Reflective access to {@link ForeachStatement#collectionElementType}.
*/
private static final Field collectionElementTypeField;
/**
* Reflective access to {@link ReferenceExpression#haveReceiver}.
*/
private static final Field haveReceiverField;

private static final TypeBinding[] NO_TYPES = new TypeBinding[0];
Expand Down
7 changes: 4 additions & 3 deletions dev/core/test/com/google/gwt/dev/jjs/impl/Java8AstTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ public void setUp() throws Exception {
));
addAll(JavaResourceBase.createMockJavaResource("test.Lambda2",
"package test;",
"public interface Lambda2<String> {",
" boolean run(String a, String b);",
"public interface Lambda2<T> {",
" boolean run(T a, T b);",
"}"
));
addAll(JavaResourceBase.createMockJavaResource("test.AcceptsLambda",
Expand Down Expand Up @@ -936,7 +936,8 @@ public void testCompileImplicitQualifierReferenceBinding() throws Exception {
// should implement run method and invoke lambda via captured instance
JMethod samMethod = findMethod(lambdaInnerClass, "run");
assertEquals(
"public final boolean run(Object arg0,Object arg1){return arg0.equalsIgnoreCase(arg1);}",
"public final boolean run(Object arg0,Object arg1)"
+ "{return((String)arg0).equalsIgnoreCase((String)arg1);}",
formatSource(samMethod.toSource()));
}

Expand Down
18 changes: 18 additions & 0 deletions user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java8Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,24 @@ public void testInterfaceWithDefaultMethodsInitialization() {
assertContentsInOrder(initializationOrder, "A1", "B1", "A2", "C", "B2", "A3", "D");
}

/**
* Regression test for issue 9214.
*/
interface P<T> {
boolean apply(T obj);
}

static class B {
public boolean getTrue() {
return true;
}
}

public void testMethodReference_generics() {
P<B> p = B::getTrue;
assertTrue(p.apply(new B()));
}

private void assertContentsInOrder(Iterable<String> contents, String... elements) {
assertEquals(Arrays.asList(elements).toString(), contents.toString());
}
Expand Down
71 changes: 61 additions & 10 deletions user/test/com/google/gwt/dev/jjs/test/Java8Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,175 +37,226 @@ public String getModuleName() {
@Override
public void runTest() throws Throwable {
// Only run these tests if -sourceLevel 8 (or greater) is enabled.
if (JUnitShell.getCompilerOptions().getSourceLevel().compareTo(SourceLevel.JAVA8) >= 0) {
if (isGwtSourceLevel8()) {
super.runTest();
}
}

public void testLambdaNoCapture() {
// Make sure we are using the right Java8Test if the source compatibility level is set to Java 8
// or above.
assertFalse(JUnitShell.getCompilerOptions().getSourceLevel().compareTo(SourceLevel.JAVA8) >= 0);
assertFalse(isGwtSourceLevel8());
}

public void testLambdaCaptureLocal() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaCaptureLocalWithInnerClass() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaCaptureLocalAndField() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaCaptureLocalAndFieldWithInnerClass() {
assertFalse(isGwtSourceLevel8());
}

public void testCompileLambdaCaptureOuterInnerField() throws Exception {
assertFalse(isGwtSourceLevel8());
}

public void testStaticReferenceBinding() throws Exception {
}

public static Integer foo(int x, int y) {
return x + y;
}

public Integer fooInstance(int x, int y) {
return x + y + 1;
assertFalse(isGwtSourceLevel8());
}

public void testInstanceReferenceBinding() throws Exception {
assertFalse(isGwtSourceLevel8());
}

public void testImplicitQualifierReferenceBinding() throws Exception {
assertFalse(isGwtSourceLevel8());
}

public void testConstructorReferenceBinding() {
assertFalse(isGwtSourceLevel8());
}

public void testStaticInterfaceMethod() {
assertFalse(isGwtSourceLevel8());
}

public void testArrayConstructorReference() {
assertFalse(isGwtSourceLevel8());
}

public void testArrayConstructorReferenceBoxed() {
assertFalse(isGwtSourceLevel8());
}

public void testVarArgsReferenceBinding() {
assertFalse(isGwtSourceLevel8());
}

public void testVarArgsPassthroughReferenceBinding() {
assertFalse(isGwtSourceLevel8());
}

public void testVarArgsPassthroughReferenceBindingProvidedArray() {
assertFalse(isGwtSourceLevel8());
}

public void testSuperReferenceExpression() {
assertFalse(isGwtSourceLevel8());
}

public void testSuperReferenceExpressionWithVarArgs() {
assertFalse(isGwtSourceLevel8());
}

public void testPrivateConstructorReference() {
assertFalse(isGwtSourceLevel8());
}

public void testDefaultInterfaceMethod() {
assertFalse(isGwtSourceLevel8());
}

public void testDefaultInterfaceMethodVirtualUpRef() {
assertFalse(isGwtSourceLevel8());
}

public void testInterfaceWithDefaultMethodsInitialization() {
assertFalse(isGwtSourceLevel8());
}

public void testDefaultInterfaceMethodMultiple() {
assertFalse(isGwtSourceLevel8());
}

public void testDefaultMethodReference() {
assertFalse(isGwtSourceLevel8());
}

public void testDefenderMethodByInterfaceInstance() {
assertFalse(isGwtSourceLevel8());
}

public void testThisRefInDefenderMethod() {
assertFalse(isGwtSourceLevel8());
}

public void testClassImplementsTwoInterfacesWithSameDefenderMethod() {
assertFalse(isGwtSourceLevel8());
}

public void testAbstractClassImplementsInterface() {
assertFalse(isGwtSourceLevel8());
}

public void testSuperRefInDefenderMethod() {
assertFalse(isGwtSourceLevel8());
}

public void testSuperThisRefsInDefenderMethod() {
assertFalse(isGwtSourceLevel8());
}

public void testNestedInterfaceClass() {
assertFalse(isGwtSourceLevel8());
}

public void testBaseIntersectionCast() {
assertFalse(isGwtSourceLevel8());
}

public void testIntersectionCastWithLambdaExpr() {
assertFalse(isGwtSourceLevel8());
}

public void testIntersectionCastPolymorphism() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaCaptureParameter() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaNestingCaptureLocal() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaNestingInAnonymousCaptureLocal() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaNestingInMultipleMixedAnonymousCaptureLocal() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaNestingInMultipleMixedAnonymousCaptureLocal_withInterference() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaNestingInMultipleMixedAnonymousCaptureLocalAndField() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaNestingInMultipleAnonymousCaptureLocal() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaNestingCaptureField_InnerClassCapturingOuterClassVariable() {
assertFalse(isGwtSourceLevel8());
}

public void testInnerClassCaptureLocalFromOuterLambda() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaNestingCaptureField() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaMultipleNestingCaptureFieldAndLocal() {
assertFalse(isGwtSourceLevel8());
}

public void testLambdaMultipleNestingCaptureFieldAndLocalInnerClass() {
assertFalse(isGwtSourceLevel8());
}

public void testMethodRefWithSameName() {
assertFalse(isGwtSourceLevel8());
}

public void testMultipleDefaults_fromInterfaces_left() {
assertFalse(isGwtSourceLevel8());
}

public void testMultipleDefaults_fromInterfaces_right() {
assertFalse(isGwtSourceLevel8());
}

public void testMultipleDefaults_superclass_left() {
assertFalse(isGwtSourceLevel8());
}

public void testMultipleDefaults_superclass_right() {
assertFalse(isGwtSourceLevel8());
}

public void testInterfaceThis() {
assertFalse(isGwtSourceLevel8());
}

public void testMethodReference_generics() {
assertFalse(isGwtSourceLevel8());
}

private boolean isGwtSourceLevel8() {
return JUnitShell.getCompilerOptions().getSourceLevel().compareTo(SourceLevel.JAVA8) >= 0;
}
}

0 comments on commit 184e750

Please sign in to comment.