Don't emit unnecessary synthetic overrides.
Change-Id: I43fab8f002de32efbf63d329f792461fc787cba6
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 53c40d7..f61d0b1 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
@@ -91,6 +91,7 @@
private boolean preventDevirtualization = false;
private boolean hasSideEffects = true;
private boolean defaultMethod = false;
+ private boolean syntheticAccidentalOverride = false;
@Override
public void setJsMemberInfo(String namespace, String name, boolean exported) {
@@ -244,6 +245,14 @@
return enclosingType != null && enclosingType.isJsNative();
}
+ public void setSyntheticAccidentalOverride() {
+ this.syntheticAccidentalOverride = true;
+ }
+
+ public boolean isSyntheticAccidentalOverride() {
+ return syntheticAccidentalOverride;
+ }
+
public void setSpecialization(List<JType> paramTypes, JType returnsType, String targetMethod) {
this.specialization = new Specialization(paramTypes,
returnsType == null ? this.getOriginalReturnType() : returnsType, targetMethod);
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ComputeOverridesAndImplementDefaultMethods.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ComputeOverridesAndImplementDefaultMethods.java
index 6fd94c2..0fd1d28 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ComputeOverridesAndImplementDefaultMethods.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ComputeOverridesAndImplementDefaultMethods.java
@@ -291,6 +291,8 @@
} else {
// Creates a forwarding method to act as the place holder for this accidental override.
implementingMethod = JjsUtils.createForwardingMethod(type, method);
+ implementingMethod.setSyntheticAccidentalOverride();
+
if (method.isFinal()) {
// To keep consistency we reset the final mark
method.setFinal(false);
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
index 1ce0f44..6b49d6a 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
@@ -2541,8 +2541,9 @@
*/
private static boolean doesNotHaveConcreteImplementation(JMethod method) {
return method.isAbstract()
- || JProgram.isClinit(method)
- && method.getEnclosingType().getClinitTarget() != method.getEnclosingType();
+ || JjsUtils.isUnnecessarySyntheticAccidentalOverride(method)
+ || (JProgram.isClinit(method)
+ && method.getEnclosingType().getClinitTarget() != method.getEnclosingType());
}
private static class JavaToJsOperatorMap {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java b/dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java
index c9eb44a..003f7fb 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java
@@ -69,6 +69,7 @@
import com.google.gwt.thirdparty.guava.common.collect.Collections2;
import com.google.gwt.thirdparty.guava.common.collect.FluentIterable;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableMap;
+import com.google.gwt.thirdparty.guava.common.collect.Iterables;
import com.google.gwt.thirdparty.guava.common.collect.Lists;
import java.util.Arrays;
@@ -320,6 +321,30 @@
}
/**
+ * Returns true if the method is a synthetic accidental override that trivially dispatches to its
+ * same name super.
+ */
+ public static boolean isUnnecessarySyntheticAccidentalOverride(JMethod method) {
+ // Assumptions on synthethic overrides, if any of these change.
+ assert !method.isSyntheticAccidentalOverride() || !method.exposesPackagePrivateMethod();
+
+ if (!method.isSyntheticAccidentalOverride()) {
+ return false;
+ }
+
+ boolean overridesConcreteMethod = Iterables.any(method.getOverriddenMethods(),
+ new Predicate<JMethod>() {
+ @Override
+ public boolean apply(JMethod method) {
+ return !method.isAbstract();
+ }
+ });
+ // A synthetic accidental override is unnecessary iff it retains the same property
+ // name (polyname) as the the concrete method it overrides.
+ return overridesConcreteMethod && !method.exposesNonJsMethod();
+ }
+
+ /**
* Mangles a qualified name into a Javah signature.
*/
public static String javahSignatureFromName(String name) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java b/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java
index d2461b7..c31d31c 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java
@@ -374,6 +374,12 @@
private void checkNoStaticJsPropertyCalls() {
new JVisitor() {
@Override
+ public boolean visit(JMethod x, Context ctx) {
+ // Skip unnecessary synthetic override, as they will not be generated.
+ return !JjsUtils.isUnnecessarySyntheticAccidentalOverride(x);
+ }
+
+ @Override
public void endVisit(JMethodCall x, Context ctx) {
JMethod target = x.getTarget();
if (x.isStaticDispatchOnly() && target.isJsPropertyAccessor()) {
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
index a9bb22a..88a8e50 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
@@ -637,8 +637,7 @@
assertBuggySucceeds();
}
- // TODO(rluble): this test should succeed, fix after eliminating the accidental overrides.
- public void testJsPropertyAccidentalSuperCallFails()
+ public void testJsPropertyAccidentalSuperCallSucceeds()
throws UnableToCompleteException {
addSnippetImport("jsinterop.annotations.JsType");
addSnippetImport("jsinterop.annotations.JsProperty");
@@ -653,9 +652,7 @@
"@JsType public static class Buggy extends Super implements Interface {",
"}");
- assertBuggyFails(
- "Cannot call property accessor 'test.EntryPoint$Super.getX()I' via super "
- + "(test/EntryPoint.java:6).");
+ assertBuggySucceeds();
}
public void testMultiplePrivateConstructorsExportSucceeds() throws Exception {
diff --git a/user/test/com/google/gwt/core/client/interop/JsPropertyTest.java b/user/test/com/google/gwt/core/client/interop/JsPropertyTest.java
index ce4dd93..d131d95 100644
--- a/user/test/com/google/gwt/core/client/interop/JsPropertyTest.java
+++ b/user/test/com/google/gwt/core/client/interop/JsPropertyTest.java
@@ -365,6 +365,48 @@
}
@JsType(isNative = true)
+ interface AccidentalOverridePropertyJsTypeInterface {
+ @JsProperty
+ int getX();
+ }
+
+ static class AccidentalOverridePropertyBase {
+ public int getX() {
+ return 50;
+ }
+ }
+
+ static class AccidentalOverrideProperty extends AccidentalOverridePropertyBase
+ implements AccidentalOverridePropertyJsTypeInterface {
+ }
+
+ public void testJsPropertyAccidentalOverrideSuperCall() {
+ AccidentalOverrideProperty object = new AccidentalOverrideProperty();
+ assertEquals(50, object.getX());
+ assertEquals(50, getProperty(object, "x"));
+ }
+
+ @JsType
+ static class RemovedAccidentalOverridePropertyBase {
+ @JsProperty
+ public int getX() {
+ return 55;
+ }
+ }
+
+ static class RemovedAccidentalOverrideProperty extends RemovedAccidentalOverridePropertyBase
+ implements AccidentalOverridePropertyJsTypeInterface {
+ }
+
+ public void testJsPropertyRemovedAccidentalOverrideSuperCall() {
+ RemovedAccidentalOverrideProperty object = new RemovedAccidentalOverrideProperty();
+ // If the accidental override here were not removed the access to property x would result in
+ // an infinite loop
+ assertEquals(55, object.getX());
+ assertEquals(55, getProperty(object, "x"));
+ }
+
+ @JsType(isNative = true)
interface JsTypeGetProperty {
@JsProperty
