Switch from alias to forwarding methods.
This patch changes JsMethod and visibility upgraded methods as
forwarding methods instead of aliases at each level.
Earlier aliases was created at each override:
parent.prototype.m_alias = parent.prototype.m;
child.prototype.m_alias = parent.prototype.m;
Now it is forwarding method at the root overridde:
parent.prototype.m_alias = function() { return this.m() };
// note no alias in the child
This both helps to workaround closure compiler bug with method aliases
and will potentially help with code size where there are a lot
overriddes.
Change-Id: I039b0887fcb67cb661970479617354355ee2c7c9
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 1071a3c..c260486 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
@@ -95,7 +95,11 @@
}
private boolean isJsInterfaceMethod() {
- return enclosingType instanceof JInterfaceType && enclosingType.isJsType();
+ return isInterFaceMethod() && enclosingType.isJsType();
+ }
+
+ private boolean isInterFaceMethod() {
+ return enclosingType instanceof JInterfaceType;
}
@Override
@@ -138,6 +142,29 @@
return isConstructor() && jsName != null;
}
+ /**
+ * Returns {@code true} if this method is the first method in the method hierarchy that exposes a
+ * JsMethod inside a class.
+ */
+ public boolean exposesJsMethod() {
+ if (isInterFaceMethod()) {
+ return false;
+ }
+
+ boolean isJsMethod = jsName != null;
+ for (JMethod override : getOverriddenMethods()) {
+ if (override.jsName == null) {
+ continue;
+ }
+ isJsMethod = true;
+ if (!override.isInterFaceMethod()) {
+ return false; // some other method already exposed this method.
+ }
+ }
+
+ return isJsMethod;
+ }
+
public boolean isOrOverridesJsMethod() {
if (jsName != null) {
return true;
@@ -441,24 +468,26 @@
}
/**
- * Returns true if this method overrides a package private method and increases its
- * visibility.
+ * Returns {@code true} if this method is the first method in the method hierarchy that increases
+ * the visibility of a package private method.
*/
- public boolean exposesOverriddenPackagePrivateMethod() {
+ public boolean exposesPackagePrivateMethod() {
if (isPrivate() || isPackagePrivate()) {
return false;
}
+ boolean hasPackageVisibleParent = false;
for (JMethod overriddenMethod : overriddenMethods) {
- if (overriddenMethod.getEnclosingType() instanceof JInterfaceType) {
+ if (overriddenMethod.isInterFaceMethod()) {
continue;
}
- if (overriddenMethod.isPackagePrivate()) {
- return true;
+ if (!overriddenMethod.isPackagePrivate()) {
+ return false; // some other method already exposed this method.
}
+ hasPackageVisibleParent = true;
}
- return false;
+ return hasPackageVisibleParent;
}
public AccessModifier getAccess() {
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 aa4b900..c609e35 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
@@ -2657,9 +2657,9 @@
}
private void generateVTableAlias(List<JsStatement> globalStmts, JMethod method, JsName alias) {
- JsNameRef polyname = polymorphicNames.get(method).makeRef(method.getSourceInfo());
- polyname.setQualifier(getPrototypeQualifierOf(method));
- generateVTableAssignment(globalStmts, method, alias, polyname);
+ JsName polyName = polymorphicNames.get(method);
+ JsExpression bridge = JsUtils.createBridge(method, polyName, topScope);
+ generateVTableAssignment(globalStmts, method, alias, bridge);
}
private JsExprStmt outputDisplayName(JsNameRef function, JMethod method) {
@@ -2695,51 +2695,42 @@
private void generateVTables(JClassType x, List<JsStatement> globalStmts) {
assert x != program.getTypeJavaLangString();
for (JMethod method : x.getMethods()) {
- if (method.needsVtable() && !method.isAbstract()) {
- /*
- * Inline JsFunction rather than reference, e.g. _.vtableName =
- * function functionName() { ... }
- */
+ if (!method.needsVtable()) {
+ continue;
+ }
+
+ if (!method.isAbstract()) {
JsExpression rhs = getJsFunctionFor(method);
generateVTableAssignment(globalStmts, method, polymorphicNames.get(method), rhs);
+ }
- if (method.isOrOverridesJsMethod()) {
- String jsName = method.getJsName();
- JsName exportedName = interfaceScope.declareName(jsName, jsName);
- exportedName.setObfuscatable(false);
- generateVTableAlias(globalStmts, method, exportedName);
- }
+ if (method.exposesJsMethod()) {
+ String jsName = method.getJsName();
+ JsName exportedName = interfaceScope.declareName(jsName, jsName);
+ exportedName.setObfuscatable(false);
+ generateVTableAlias(globalStmts, method, exportedName);
+ }
- if (method.exposesOverriddenPackagePrivateMethod()) {
- // This method exposes a package private method that is actually live, hence it needs
- // to make the package private name and the public name to be the same implementation at
- // runtime. This is done by an assignment of the form.
- // _.package_private_name = _.exposed_name
-
- // Here is the situation where this is needed:
- //
- // class a.A { m() {} }
- // class b.B extends a.A { m() {} }
- // interface I { m(); }
- // class a.C {
- // { A a = new b.B(); a.m() // calls A::m()} }
- // { I i = new b.B(); a.m() // calls B::m()} }
- // }
- //
- // Up to this point it is clear that package private names need to be different than
- // public names.
- //
- // Add class a.D extends a.A implements I { public m() }
- //
- // a.D collapses A::m and I::m into the same function and it was clear that two
- // two different names were already needed, hence when creating the vtable for a.D
- // both names have to point to the same function.
- //
- // It should be noted that all subclasses of a.D will have the two methods collapsed,
- // and hence this assignment will be present in the vtable setup for all subclasses.
-
- generateVTableAlias(globalStmts, method, getPackagePrivateName(method));
- }
+ if (method.exposesPackagePrivateMethod()) {
+ // Here is the situation where this is needed:
+ //
+ // class a.A { m() {} }
+ // class b.B extends a.A { m() {} }
+ // interface I { m(); }
+ // class a.C {
+ // { A a = new b.B(); a.m() // calls A::m()} }
+ // { I i = new b.B(); a.m() // calls B::m()} }
+ // }
+ //
+ // Up to this point it is clear that package private names need to be different than
+ // public names.
+ //
+ // Add class a.D extends a.A implements I { public m() }
+ //
+ // a.D collapses A::m and I::m into the same function and it was clear that two
+ // two different names were already needed, hence when creating the vtable for a.D
+ // both names have to point to the same function.
+ generateVTableAlias(globalStmts, method, getPackagePrivateName(method));
}
}
}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsUtils.java b/dev/core/src/com/google/gwt/dev/js/JsUtils.java
index 9f8732f..73e9873 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsUtils.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsUtils.java
@@ -16,6 +16,9 @@
package com.google.gwt.dev.js;
import com.google.gwt.dev.jjs.SourceInfo;
+import com.google.gwt.dev.jjs.ast.JMethod;
+import com.google.gwt.dev.jjs.ast.JParameter;
+import com.google.gwt.dev.jjs.ast.JPrimitiveType;
import com.google.gwt.dev.js.ast.JsBinaryOperation;
import com.google.gwt.dev.js.ast.JsBinaryOperator;
import com.google.gwt.dev.js.ast.JsBlock;
@@ -25,7 +28,10 @@
import com.google.gwt.dev.js.ast.JsName;
import com.google.gwt.dev.js.ast.JsNameRef;
import com.google.gwt.dev.js.ast.JsNode;
+import com.google.gwt.dev.js.ast.JsParameter;
+import com.google.gwt.dev.js.ast.JsReturn;
import com.google.gwt.dev.js.ast.JsScope;
+import com.google.gwt.dev.js.ast.JsThisRef;
import com.google.gwt.dev.util.StringInterner;
/**
@@ -88,6 +94,30 @@
return new JsBinaryOperation(lhs.getSourceInfo(), JsBinaryOperator.ASG, lhs, rhs);
}
+ public static JsFunction createBridge(JMethod method, JsName polyName, JsScope scope) {
+ SourceInfo sourceInfo = method.getSourceInfo();
+ JsFunction bridge = new JsFunction(sourceInfo, scope);
+ for (JParameter p : method.getParams()) {
+ JsName name = bridge.getScope().declareName(p.getName());
+ bridge.getParameters().add(new JsParameter(sourceInfo, name));
+ }
+ JsNameRef ref = polyName.makeRef(sourceInfo);
+ ref.setQualifier(new JsThisRef(sourceInfo));
+ JsInvocation invocation = new JsInvocation(sourceInfo, ref);
+ for (JsParameter p : bridge.getParameters()) {
+ invocation.getArguments().add(p.getName().makeRef(sourceInfo));
+ }
+
+ JsBlock block = new JsBlock(sourceInfo);
+ if (method.getType() == JPrimitiveType.VOID) {
+ block.getStatements().add(invocation.makeStmt());
+ } else {
+ block.getStatements().add(new JsReturn(sourceInfo, invocation));
+ }
+ bridge.setBody(block);
+ return bridge;
+ }
+
public static JsFunction createEmptyFunctionLiteral(SourceInfo info, JsScope scope, JsName name) {
JsFunction func = new JsFunction(info, scope, name);
func.setBody(new JsBlock(info));
diff --git a/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/JavaClassHierarchySetupUtil.java b/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/JavaClassHierarchySetupUtil.java
index 2180870..ce2ac4b 100644
--- a/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/JavaClassHierarchySetupUtil.java
+++ b/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/JavaClassHierarchySetupUtil.java
@@ -166,20 +166,6 @@
}-*/;
/**
- * If the parameter o is a Javascript object, return the bridge method reference,
- * otherwise return the nonbridge reference.
- * @param o an instance object we want to invoke a method on
- * @param bridgeRef a reference to an exported, bridgereference or jstype method
- * @param nonbridgeRef the internal reference to Java obfuscated method
- * @return
- */
- public static native boolean trampolineBridgeMethod(Object o, Object bridgeRef,
- Object nonbridgeRef) /*-{
- return @com.google.gwt.lang.Cast::isJavaScriptObject(Ljava/lang/Object;)(o)
- ? bridgeRef : nonbridgeRef;
- }-*/;
-
- /**
* Do polyfills for all methods expected in a modern browser.
*/
public static native void modernizeBrowser() /*-{
diff --git a/user/test/com/google/gwt/core/client/interop/AbstractJsType.java b/user/test/com/google/gwt/core/client/interop/AbstractJsType.java
new file mode 100644
index 0000000..5f059e5
--- /dev/null
+++ b/user/test/com/google/gwt/core/client/interop/AbstractJsType.java
@@ -0,0 +1,26 @@
+/*
+ * 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.core.client.interop;
+
+import com.google.gwt.core.client.js.JsType;
+
+/**
+ * This concrete test class is *directly* annotated as a @JsType.
+ */
+@JsType
+abstract class AbstractJsType {
+ public abstract int publicMethod();
+}
diff --git a/user/test/com/google/gwt/core/client/interop/ConcreteJsTypeSubclass.java b/user/test/com/google/gwt/core/client/interop/ConcreteJsTypeSubclass.java
index 555a85a..789fb02 100644
--- a/user/test/com/google/gwt/core/client/interop/ConcreteJsTypeSubclass.java
+++ b/user/test/com/google/gwt/core/client/interop/ConcreteJsTypeSubclass.java
@@ -32,7 +32,8 @@
* do not override already exported members.
*/
- public void publicSubclassMethod() {
+ public int publicSubclassMethod() {
+ return super.publicMethod();
}
public static void publicStaticSubclassMethod() {
diff --git a/user/test/com/google/gwt/core/client/interop/JsTypeTest.java b/user/test/com/google/gwt/core/client/interop/JsTypeTest.java
index 496ad5a..4914698 100644
--- a/user/test/com/google/gwt/core/client/interop/JsTypeTest.java
+++ b/user/test/com/google/gwt/core/client/interop/JsTypeTest.java
@@ -81,10 +81,24 @@
assertJsTypeDoesntHaveFields(concreteJsType, "publicStaticMethod", "privateMethod",
"protectedMethod", "packageMethod", "publicStaticField", "privateField", "protectedField",
"packageField");
+
+ assertEquals(10, callIntFunction(concreteJsType, "publicMethod"));
+ }
+
+ public void testAbstractJsTypeAccess() {
+ AbstractJsType jsType = new AbstractJsType() {
+ @Override
+ public int publicMethod() {
+ return 32;
+ }
+ };
+
+ assertJsTypeHasFields(jsType, "publicMethod");
+ assertEquals(32, callIntFunction(jsType, "publicMethod"));
+ assertEquals(32, jsType.publicMethod());
}
public void testConcreteJsTypeSubclassAccess() {
- ConcreteJsType concreteJsType = new ConcreteJsType();
ConcreteJsTypeSubclass concreteJsTypeSubclass = new ConcreteJsTypeSubclass();
// A subclass of a JsType is not itself a JsType.
@@ -94,12 +108,10 @@
"privateSubclassField", "protectedSubclassField", "packageSubclassField");
// But if it overrides an exported method then the overriding method will be exported.
- assertJsTypeHasFields(concreteJsType, "publicMethod");
assertJsTypeHasFields(concreteJsTypeSubclass, "publicMethod");
- assertFalse(
- areSameFunction(concreteJsType, "publicMethod", concreteJsTypeSubclass, "publicMethod"));
- assertFalse(callIntFunction(concreteJsType, "publicMethod")
- == callIntFunction(concreteJsTypeSubclass, "publicMethod"));
+
+ assertEquals(20, callIntFunction(concreteJsTypeSubclass, "publicMethod"));
+ assertEquals(10, concreteJsTypeSubclass.publicSubclassMethod());
}
public void testConcreteJsTypeNoTypeTightenField() {
@@ -352,11 +364,6 @@
return !!$wnd;
}-*/;
- private static native boolean areSameFunction(
- Object thisObject, String thisFunctionName, Object thatObject, String thatFunctionName) /*-{
- return thisObject[thisFunctionName] === thatObject[thatFunctionName];
- }-*/;
-
private static native int callIntFunction(Object object, String functionName) /*-{
return object[functionName]();
}-*/;
