Fix bad rewriting of default methods.
ReplaceDefenderMethodRefernces rewrites default methods into static
methods and rerouting all the explicit callsites. The callsite rewrite
was incorrectly assuming that calls would always have JThisRef as a
qualifier, which is true initially but not in the midst of the
rewriting process.
A second issue was the rewriting of callsites might have been
incomplete as new methods are created by this process and might or
might not be themselves processed to rewrite default method dispatch.
Bug: #9453
Bug-Link: https://github.com/gwtproject/gwt/issues/9453
Change-Id: I4ee3b340ce780cd0458920c9951b442fc25f1523
diff --git a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
index 65aca8c..bf6274b 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
@@ -71,6 +71,7 @@
import com.google.gwt.dev.jjs.impl.ControlFlowAnalyzer;
import com.google.gwt.dev.jjs.impl.ControlFlowRecorder;
import com.google.gwt.dev.jjs.impl.DeadCodeElimination;
+import com.google.gwt.dev.jjs.impl.DevirtualizeDefaultMethodForwarding;
import com.google.gwt.dev.jjs.impl.Devirtualizer;
import com.google.gwt.dev.jjs.impl.EnumNameObfuscator;
import com.google.gwt.dev.jjs.impl.EnumOrdinalizer;
@@ -106,7 +107,6 @@
import com.google.gwt.dev.jjs.impl.RemoveEmptySuperCalls;
import com.google.gwt.dev.jjs.impl.RemoveSpecializations;
import com.google.gwt.dev.jjs.impl.ReplaceCallsToNativeJavaLangObjectOverrides;
-import com.google.gwt.dev.jjs.impl.ReplaceDefenderMethodReferences;
import com.google.gwt.dev.jjs.impl.ReplaceGetClassOverrides;
import com.google.gwt.dev.jjs.impl.ResolvePermutationDependentValues;
import com.google.gwt.dev.jjs.impl.ResolveRuntimeTypeReferences;
@@ -1151,8 +1151,8 @@
EnumNameObfuscator.exec(jprogram, logger, configurationProperties, options);
// (3) Normalize the unresolved Java AST
- // Replace defender method references
- ReplaceDefenderMethodReferences.exec(jprogram);
+ // Replace default methods by static implementations.
+ DevirtualizeDefaultMethodForwarding.exec(jprogram);
// Replace calls to native overrides of object methods.
ReplaceCallsToNativeJavaLangObjectOverrides.exec(jprogram);
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
index f70a96b..c9d561c 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
@@ -292,8 +292,8 @@
this.hasSideEffects = hasSideEffects;
}
- public void setDefaultMethod() {
- this.defaultMethod = true;
+ public void setDefaultMethod(boolean defaultMethod) {
+ this.defaultMethod = defaultMethod;
}
@Override
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/DevirtualizeDefaultMethodForwarding.java b/dev/core/src/com/google/gwt/dev/jjs/impl/DevirtualizeDefaultMethodForwarding.java
new file mode 100644
index 0000000..c96f833
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/DevirtualizeDefaultMethodForwarding.java
@@ -0,0 +1,76 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.dev.jjs.impl;
+
+import com.google.gwt.dev.jjs.ast.Context;
+import com.google.gwt.dev.jjs.ast.JDeclaredType;
+import com.google.gwt.dev.jjs.ast.JMethod;
+import com.google.gwt.dev.jjs.ast.JMethodCall;
+import com.google.gwt.dev.jjs.ast.JModVisitor;
+import com.google.gwt.dev.jjs.ast.JProgram;
+import com.google.gwt.thirdparty.guava.common.collect.ImmutableList;
+
+/**
+ * Java8 default methods are implemented by creating a forwarding (with static dispatch) the
+ * implementing method on implementing classes to an the default method in an interface (which is
+ * modeled as an instance method). JavaScript lacks the notion of interfaces and GWT consequently
+ * does not generate prototypes nor instance method for interfaces. Due to that fact this pass
+ * devirtualizes the default methods into static interface methods.
+ * <p>
+ * This devirtualization could also be handled by Devirtualizer.
+ */
+public class DevirtualizeDefaultMethodForwarding {
+
+ public static void exec(final JProgram program) {
+ final MakeCallsStatic.CreateStaticImplsVisitor staticImplCreator =
+ new MakeCallsStatic.CreateStaticImplsVisitor(program);
+ // 1. create the static implementations. Also process reference only types so that the mapping
+ // between a method and its devirtualized version is consistent during incremental compilation.
+ // NOTE: the process needs to be done in two steps because the creation of static implementation
+ // might introduce call sites in types that were already processed and hence would not be
+ // rewritten (bug #9453).
+ for (JDeclaredType type : program.getDeclaredTypes()) {
+ // Iterate over the methods using a copy to avoid ConcurrentModificationException as the
+ // the devirualized method is added to the type following the current method.
+ for (JMethod method : ImmutableList.copyOf(type.getMethods())) {
+ if (method.isDefaultMethod()) {
+ staticImplCreator.getOrCreateStaticImpl(program, method);
+ }
+ }
+ }
+
+ // 2. Devirtualize (static dispatch) calls to the default method.
+ new JModVisitor() {
+ @Override
+ public void endVisit(JMethodCall x, Context ctx) {
+ JMethod targetMethod = x.getTarget();
+ if (targetMethod.isDefaultMethod() && x.isStaticDispatchOnly()) {
+ assert x.getInstance() != null;
+
+ JMethod staticMethod = program.getStaticImpl(targetMethod);
+ assert staticMethod != null;
+ // Need to devirtualize because instance methods are not emitted for interfaces.
+ JMethodCall callStaticMethod = new JMethodCall(x.getSourceInfo(), null, staticMethod);
+ // Add the qualifier as a first parameter (usually 'this', unless the enclosing method was
+ // devirtualized first).
+ callStaticMethod.addArg(x.getInstance());
+ callStaticMethod.addArgs(x.getArgs());
+ ctx.replaceMe(callStaticMethod);
+ }
+ }
+ }.accept(program);
+ }
+}
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 7cb8ef5..328fe3b 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
@@ -2796,7 +2796,7 @@
if (enclosingType instanceof JInterfaceType) {
// Mark bridges created in interfaces as default methods so they are processed correctly
// by the rest of the pipeline.
- bridgeMethod.setDefaultMethod();
+ bridgeMethod.setDefaultMethod(true);
}
bridgeMethod.setSynthetic();
@@ -4136,7 +4136,7 @@
}
if (b.isDefaultMethod()) {
- method.setDefaultMethod();
+ method.setDefaultMethod(true);
}
enclosingType.addMethod(method);
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/JavaAstVerifier.java b/dev/core/src/com/google/gwt/dev/jjs/impl/JavaAstVerifier.java
index 0cf679e..9b66dd2 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/JavaAstVerifier.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/JavaAstVerifier.java
@@ -26,6 +26,7 @@
import com.google.gwt.dev.jjs.ast.JMethodCall;
import com.google.gwt.dev.jjs.ast.JNode;
import com.google.gwt.dev.jjs.ast.JProgram;
+import com.google.gwt.dev.jjs.ast.JThisRef;
import com.google.gwt.dev.jjs.ast.JVisitor;
import com.google.gwt.dev.jjs.ast.js.JsniFieldRef;
import com.google.gwt.dev.jjs.ast.js.JsniMethodRef;
@@ -128,6 +129,15 @@
assertNotSeenBefore(x);
}
+ JMethod currentMethod;
+
+ @Override
+ public boolean visit(JMethod x, Context ctx) {
+ assert currentMethod == null;
+ currentMethod = x;
+ return true;
+ }
+
@Override
public void endVisit(JMethod x, Context ctx) {
JDeclaredType enclosingType = x.getEnclosingType();
@@ -137,6 +147,8 @@
seenMethodsByType.put(enclosingType, methodSignature);
assertCorrectOverriddenOrder(program, x);
assertCorrectOverridingOrder(program, x);
+ assert currentMethod == x;
+ currentMethod = null;
}
@Override
@@ -144,6 +156,11 @@
assertCalledMethodIsInAst(x);
}
+ public void endVisit(JThisRef x, Context ctx) {
+ assert !currentMethod.isStatic() || currentMethod.isConstructor()
+ : "JThisRef found in static method " + currentMethod;
+ }
+
@Override
public void endVisit(JsniFieldRef x, Context ctx) {
assertReferencedFieldIsInAst(x);
@@ -160,6 +177,12 @@
}
assert membersByType.containsEntry(x.getTarget().getEnclosingType(), x.getTarget()) :
"Method " + x.getTarget() + " is called but is not part of the AST";
+
+ JMethod staticImpl = program.getStaticImpl(x.getTarget());
+ assert staticImpl == null
+ || membersByType.containsEntry(staticImpl.getEnclosingType(), staticImpl) :
+ "Method " + staticImpl + " is the static implementation of " + x.getTarget()
+ + " but is not part of the AST";
}
private void assertReferencedFieldIsInAst(JFieldRef x) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceDefenderMethodReferences.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceDefenderMethodReferences.java
deleted file mode 100644
index 57eb09b..0000000
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceDefenderMethodReferences.java
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * Copyright 2014 Google Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License"); you may not
- * use this file except in compliance with the License. You may obtain a copy of
- * the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations under
- * the License.
- */
-package com.google.gwt.dev.jjs.impl;
-
-import com.google.gwt.dev.jjs.ast.Context;
-import com.google.gwt.dev.jjs.ast.JMethod;
-import com.google.gwt.dev.jjs.ast.JMethodCall;
-import com.google.gwt.dev.jjs.ast.JModVisitor;
-import com.google.gwt.dev.jjs.ast.JProgram;
-import com.google.gwt.dev.jjs.ast.JThisRef;
-
-/**
- * Java8 defender methods are implemented by creating a forwarding method on each class that
- * inherits the implementation. For a concrete type C inheriting I.m(), there will be an
- * implementation <code>C.m() { return I.super.m(); }</code>.
- *
- * References to I.super.m() are replaced by creating a static version of this method on the
- * interface, and then delegating to it instead.
- */
-public class ReplaceDefenderMethodReferences extends JModVisitor {
-
- private final MakeCallsStatic.CreateStaticImplsVisitor staticImplCreator;
- private JProgram program;
-
- public static void exec(JProgram program) {
- ReplaceDefenderMethodReferences visitor =
- new ReplaceDefenderMethodReferences(program);
- visitor.accept(program);
- }
-
- private ReplaceDefenderMethodReferences(JProgram program) {
- this.program = program;
- this.staticImplCreator = new MakeCallsStatic.CreateStaticImplsVisitor(program);
- }
-
- @Override
- public void endVisit(JMethodCall x, Context ctx) {
- JMethod targetMethod = x.getTarget();
- if (targetMethod.isDefaultMethod() && x.isStaticDispatchOnly()) {
- assert x.getInstance() instanceof JThisRef;
-
- JMethod staticMethod = staticImplCreator.getOrCreateStaticImpl(program, targetMethod);
- // Cannot use setStaticDispatchOnly() here because interfaces don't have prototypes
- JMethodCall callStaticMethod = new JMethodCall(x.getSourceInfo(), null, staticMethod);
- // add 'this' as first parameter
- callStaticMethod.addArg(new JThisRef(x.getSourceInfo(), targetMethod.getEnclosingType()));
- callStaticMethod.addArgs(x.getArgs());
- ctx.replaceMe(callStaticMethod);
- }
- }
-}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java
index 453b14e..ece34c1 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java
@@ -50,7 +50,6 @@
import com.google.gwt.dev.jjs.ast.JIfStatement;
import com.google.gwt.dev.jjs.ast.JInstanceOf;
import com.google.gwt.dev.jjs.ast.JIntLiteral;
-import com.google.gwt.dev.jjs.ast.JInterfaceType;
import com.google.gwt.dev.jjs.ast.JLabel;
import com.google.gwt.dev.jjs.ast.JLabeledStatement;
import com.google.gwt.dev.jjs.ast.JLocal;
@@ -629,29 +628,16 @@
printTypeName(target.getEnclosingType());
print('.');
printName(target);
- } else if (x.isStaticDispatchOnly()) {
- // super(), this() or super.m() call.
- JReferenceType thisType = (JReferenceType) x.getInstance().getType().getUnderlyingType();
- if (thisType == target.getEnclosingType() && !(thisType instanceof JInterfaceType)) {
- print(CHARS_THIS);
- } else {
- if (thisType instanceof JInterfaceType) {
- // This is a static default method dispatch.
- printTypeName(target.getEnclosingType());
- print('.');
- }
- print(CHARS_SUPER);
- }
- if (!x.getTarget().isConstructor()) {
- print('.');
- printName(target);
- }
} else {
// Instance call.
parenPush(x, instance);
accept(instance);
parenPop(x, instance);
print('.');
+ if (x.isStaticDispatchOnly()) {
+ printTypeName(target.getEnclosingType());
+ print('.');
+ }
printName(target);
}
lparen();
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/Java8AstTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/Java8AstTest.java
index b7f4ba9..81971db 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/Java8AstTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/Java8AstTest.java
@@ -1187,7 +1187,7 @@
JMethod defaultMethod = findMethod(clazz, "method2");
assertNotNull(defaultMethod);
assertNotNull(defaultMethod.getBody());
- assertEquals("{return super.method2();}",
+ assertEquals("{return this.DefaultInterface.method2();}",
formatSource(defaultMethod.getBody().toSource()));
}
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java
index 511f2cc..5af563f 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java
@@ -181,7 +181,7 @@
// Neither super ctor call, nor super call's param is pruned
assertEquals(
"public EntryPoint$JsProtoImpl(){\n" +
- " super(10);\n" +
+ " this.EntryPoint$JsProto.EntryPoint$JsProto(10);\n" +
" this.$init();\n" +
"}",
findMethod(result.findClass("EntryPoint$JsProtoImpl"), "EntryPoint$JsProtoImpl").toSource());
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 e8aecbf..503475c 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
@@ -1717,5 +1717,34 @@
public void testLocalClassConstructorReferenceInStaticMethod() {
assertTrue(createInnerClassProducer().get() != null);
}
+
+ // NOTE: DO NOT reorder the following classes, bug #9453 is only reproducible in certain
+ // orderings.
+ interface SubSub_SuperDefaultMethodDevirtualizationOrder
+ extends Sub_SuperDefaultMethodDevirtualizationOrder {
+ default String m() {
+ return Sub_SuperDefaultMethodDevirtualizationOrder.super.m();
+ }
+ }
+
+ interface Sub_SuperDefaultMethodDevirtualizationOrder
+ extends Super_SuperDefaultMethodDevirtualizationOrder {
+ @Override
+ default String m() {
+ return Super_SuperDefaultMethodDevirtualizationOrder.super.m();
+ }
+ }
+
+ interface Super_SuperDefaultMethodDevirtualizationOrder {
+ default String m() {
+ return "Hi";
+ }
+ }
+
+ // Regression test for bug #9453.
+ public void testDefaultMethodDevirtualizationOrder() {
+ assertEquals("Hi", new SubSub_SuperDefaultMethodDevirtualizationOrder() {
+ }.m());
+ }
}
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 f9f945d..a87b1a3 100644
--- a/user/test/com/google/gwt/dev/jjs/test/Java8Test.java
+++ b/user/test/com/google/gwt/dev/jjs/test/Java8Test.java
@@ -316,6 +316,10 @@
assertFalse(isGwtSourceLevel8());
}
+ public void testDefaultMethodDevirtualizationOrder() {
+ assertFalse(isGwtSourceLevel8());
+ }
+
private boolean isGwtSourceLevel8() {
return JUnitShell.getCompilerOptions().getSourceLevel().compareTo(SourceLevel.JAVA8) >= 0;
}
