Fix for issue #319. Reviewed by gwt.team.mmendez
http://code.google.com/p/google-web-toolkit/issues/detail?id=319
The problem is that calling super.foo() fails to dispatch statically if
super.foo() happens to be a native method. The generated code actually
performs a polymorphic call, which incorrectly does dynamic dispatch to
the most derived subclass implementation.
Why does this only happen with a native super.foo()?
Well, it's kind of tricky. :) If super.foo() is non-native, we will
create a static impl method, super.$foo() and call that one statically
from the super.foo() call site. In fact, we have been depending on this
ability to correctly dispatch super calls in general. The problem is, we
specifically cannot staticify native methods right now. So the call site
remains bound to the instance version, and during code gen creates a
polymorphic call, even though the correct field is set on the JMethodCall
to not do dynamic dispatch.
My fix is this: if we find during code gen that we must statically
dispatch an instance method, we now generate a "call" syntax construct in
the output. For example:
Old, bad code:
function Hello$onModuleLoad(this$static) {
this$static.onModuleLoad__(); // wrong
}
New, good code:
function Hello_$onModuleLoad(this$static) {
Foo_onModuleLoad.call(this$static);
}
As part of this fix, I refactored JMethodCall's concept of
"canBePolymorphic", which was ambiguous between the instance variable
(and setter) and the getter. Instead, the variable, getter, and setter
are now "staticDispatchOnly", and "canBePolymorphic" is a computed result.
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@221 8db76d5a-ed1c-0410-87a9-c151d255dfc7
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 21a5f13..e0503cb 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
@@ -159,7 +159,6 @@
JMethodCall onModuleLoadCall = new JMethodCall(program, qualifier,
mainMethod);
- onModuleLoadCall.setCanBePolymorphic(true);
bootStrapMethod.body.statements.add(new JExpressionStatement(program,
onModuleLoadCall));
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethodCall.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethodCall.java
index ef19cf0..5f75fdc 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethodCall.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethodCall.java
@@ -20,17 +20,17 @@
*/
public class JMethodCall extends JExpression {
- public final Holder instance = new Holder();
public HolderList args = new HolderList();
+ public final Holder instance = new Holder();
private final JMethod method;
private final JType overrideReturnType;
- private boolean canBePolymorphic;
+ private boolean staticDispatchOnly = false;
public JMethodCall(JProgram program, JExpression instance, JMethod method) {
super(program);
this.instance.set(instance);
this.method = method;
- this.canBePolymorphic = false;
+ this.staticDispatchOnly = false;
this.overrideReturnType = null;
}
@@ -50,13 +50,12 @@
super(program);
this.instance.set(instance);
this.method = method;
- this.canBePolymorphic = false;
assert (overrideReturnType != null);
this.overrideReturnType = overrideReturnType;
}
public boolean canBePolymorphic() {
- return canBePolymorphic && !method.isFinal() && !method.isStatic();
+ return !staticDispatchOnly && !method.isFinal() && !method.isStatic();
}
public JExpression getInstance() {
@@ -80,8 +79,12 @@
return true;
}
- public void setCanBePolymorphic(boolean canBePolymorphic) {
- this.canBePolymorphic = canBePolymorphic;
+ public boolean isStaticDispatchOnly() {
+ return staticDispatchOnly;
+ }
+
+ public void setStaticDispatchOnly() {
+ this.staticDispatchOnly = true;
}
public void traverse(JVisitor visitor) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/js/JsniMethodRef.java b/dev/core/src/com/google/gwt/dev/jjs/ast/js/JsniMethodRef.java
index 509decc..f479a99 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/js/JsniMethodRef.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/js/JsniMethodRef.java
@@ -27,7 +27,6 @@
public JsniMethodRef(JProgram program, JMethod method) {
super(program, null, method);
- setCanBePolymorphic(true);
}
public void traverse(JVisitor visitor) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
index b75dbbd..d733368 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
@@ -863,7 +863,9 @@
JMethodCall call = new JMethodCall(program, qualifier, method);
boolean isSuperRef = x.receiver instanceof SuperReference;
- call.setCanBePolymorphic(!isSuperRef);
+ if (isSuperRef) {
+ call.setStaticDispatchOnly();
+ }
// The arguments come first...
if (x.arguments != null) {
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 88ee143..ce0738e 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
@@ -898,8 +898,24 @@
}
qualifier = getName(method).makeRef();
} else {
- qualifier = getPolyName(method).makeRef();
- qualifier.setQualifier((JsExpression) pop()); // instance
+ if (x.isStaticDispatchOnly()) {
+ /*
+ * Dispatch statically (odd case). This happens when a call that must
+ * be static is targetting an instance method that could not be
+ * transformed into a static. For example, making a super call into a
+ * native method currently causes this, because we cannot currently
+ * staticify native methods.
+ *
+ * Have to use a "call" construct.
+ */
+ qualifier = objectScope.getOrCreateUnobfuscatableName("call").makeRef();
+ qualifier.setQualifier(getName(method).makeRef());
+ jsInvocation.getArguments().add(0, (JsExpression) pop()); // instance
+ } else {
+ // Dispatch polymorphically (normal case).
+ qualifier = getPolyName(method).makeRef();
+ qualifier.setQualifier((JsExpression) pop()); // instance
+ }
}
jsInvocation.setQualifier(qualifier);
push(createCommaExpression(unnecessaryQualifier, jsInvocation));
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java b/dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java
index 9269611..141460d 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java
@@ -106,7 +106,6 @@
// Update the call site
JMethodCall call = new JMethodCall(program, null, foundMethod);
- call.setCanBePolymorphic(true);
changes.replaceExpression(m, call);
// Copy the qualifier
