Author: [email protected]
Date: Mon Apr 6 12:48:03 2009
New Revision: 5191
Modified:
trunk/dev/core/src/com/google/gwt/core/ext/soyc/impl/StandardMethodMember.java
trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
trunk/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java
trunk/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
Log:
Adds more bridge methods so as to simplify and make more robust
virtual-method dispatch in the face of tricky generics cases.
Fixes issue 3517.
Patch by: scottb
Review by: spoon
Modified:
trunk/dev/core/src/com/google/gwt/core/ext/soyc/impl/StandardMethodMember.java
==============================================================================
---
trunk/dev/core/src/com/google/gwt/core/ext/soyc/impl/StandardMethodMember.java
(original)
+++
trunk/dev/core/src/com/google/gwt/core/ext/soyc/impl/StandardMethodMember.java
Mon Apr 6 12:48:03 2009
@@ -51,6 +51,7 @@
sb.append(type.getJsniSignatureName());
}
sb.append(")");
+ sb.append(method.getOriginalReturnType().getJsniSignatureName());
this.sourceName = sb.toString();
SortedSet<String> aliases = new TreeSet<String>();
Modified: trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
==============================================================================
--- trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java (original)
+++ trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java Mon Apr 6
12:48:03 2009
@@ -47,6 +47,7 @@
private final boolean isStatic;
private final String name;
private List<JType> originalParamTypes;
+ private JType originalReturnType;
/**
* References to any methods which this method overrides. This should be
an
@@ -102,7 +103,7 @@
for (JParameter param : params) {
paramTypes.add(param.getType());
}
- setOriginalParamTypes(paramTypes);
+ setOriginalTypes(returnType, paramTypes);
}
public JAbstractMethodBody getBody() {
@@ -124,6 +125,10 @@
return originalParamTypes;
}
+ public JType getOriginalReturnType() {
+ return originalReturnType;
+ }
+
/**
* Returns the transitive closure of all the methods this method
overrides.
*/
@@ -189,10 +194,11 @@
isFinal = true;
}
- public void setOriginalParamTypes(List<JType> paramTypes) {
+ public void setOriginalTypes(JType returnType, List<JType> paramTypes) {
if (originalParamTypes != null) {
throw new InternalCompilerException("Param types already frozen");
}
+ originalReturnType = returnType;
originalParamTypes = Lists.normalize(paramTypes);
// Determine if we should trace this method.
Modified: trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
==============================================================================
--- trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java (original)
+++ trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java Mon Apr 6
12:48:03 2009
@@ -124,6 +124,7 @@
sb.append(type.getJsniSignatureName());
}
sb.append(")");
+ sb.append(method.getOriginalReturnType().getJsniSignatureName());
return sb.toString();
}
Modified: trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
==============================================================================
--- trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
(original)
+++ trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java Mon Apr
6 12:48:03 2009
@@ -156,6 +156,11 @@
return false;
}
+ // original return type must be identical
+ if (method1.getOriginalReturnType() !=
method2.getOriginalReturnType()) {
+ return false;
+ }
+
// original parameter types must be identical
List<JType> params1 = method1.getOriginalParamTypes();
List<JType> params2 = method2.getOriginalParamTypes();
Modified:
trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
==============================================================================
--- trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
(original)
+++ trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java Mon
Apr 6 12:48:03 2009
@@ -309,8 +309,8 @@
* </p>
*/
public void addBridgeMethods(SourceTypeBinding clazzBinding) {
- if (clazzBinding.isInterface() || clazzBinding.isAbstract()) {
- // Only add bridges in concrete classes, to simplify matters.
+ if (clazzBinding.isInterface()) {
+ // Only add bridges in classes, to simplify matters.
return;
}
@@ -322,7 +322,8 @@
*/
if (clazzBinding.syntheticMethods() != null) {
for (SyntheticMethodBinding synthmeth :
clazzBinding.syntheticMethods()) {
- if (synthmeth.purpose == SyntheticMethodBinding.BridgeMethod) {
+ if (synthmeth.purpose == SyntheticMethodBinding.BridgeMethod
+ && !synthmeth.isStatic()) {
JMethod implmeth = (JMethod)
typeMap.get(synthmeth.targetMethod);
createBridgeMethod(clazz, synthmeth, implmeth);
@@ -1482,7 +1483,7 @@
MethodBinding b = x.binding;
JMethod method = (JMethod) typeMap.get(b);
try {
- if (b.isImplementing() || b.isOverriding()) {
+ if (!b.isStatic() && (b.isImplementing() || b.isOverriding())) {
tryFindUpRefs(method, b);
}
@@ -2030,20 +2031,6 @@
}
}
- private boolean classHasMethodOverriding(JClassType clazz, JMethod
over) {
- for (JMethod meth : clazz.methods) {
- if (meth.getOverrides().contains(over)) {
- return true;
- }
- }
-
- if (clazz.extnds != null && classHasMethodOverriding(clazz.extnds,
over)) {
- return true;
- }
-
- return false;
- }
-
/**
* Create a bridge method. It calls a same-named method with the same
* arguments, but with a different type signature.
@@ -2101,15 +2088,20 @@
JMethodBody body = (JMethodBody) bridgeMethod.getBody();
body.getBlock().addStmt(callOrReturn);
- // add overrides, but only for interface methods that the class does
not
- // already override
+ // Add overrides.
List<JMethod> overrides = new ArrayList<JMethod>();
tryFindUpRefs(bridgeMethod, overrides);
+ assert !overrides.isEmpty();
for (JMethod over : overrides) {
- if (!classHasMethodOverriding(clazz, over)) {
- bridgeMethod.addOverride(over);
- bridgeMethod.addOverrides(over.getOverrides());
- }
+ bridgeMethod.addOverride(over);
+ /*
+ * TODO(scottb): with a diamond-shape inheritance hierarchy, it
may be
+ * possible to get dups in this way. Really, method.overrides
should
+ * probably just be an IdentitySet to avoid having to check
contains in
+ * various places. Left as a todo because I don't think dups is
super
+ * harmful.
+ */
+ bridgeMethod.addOverrides(over.getOverrides());
}
}
@@ -2369,8 +2361,8 @@
* expression. Beware that when autoboxing, the type of the expression
is
* not necessarily the same as the type of the box to be created. The
JDT
* figures out what the necessary conversion is, depending on the
context
- * the expression appears in, and stores it in
<code>x.implicitConversion</code>,
- * so extract it from there.
+ * the expression appears in, and stores it in
+ * <code>x.implicitConversion</code>, so extract it from there.
*/
private JPrimitiveType implicitConversionTargetType(Expression x)
throws InternalCompilerException {
@@ -2557,6 +2549,8 @@
* overrides/implements.
*/
private void tryFindUpRefs(JMethod method, MethodBinding binding) {
+ // Should never get a parameterized instance here.
+ assert binding == binding.original();
tryFindUpRefsRecursive(method, binding, binding.declaringClass);
}
@@ -2595,11 +2589,18 @@
*/
private void tryFindUpRefsRecursive(JMethod method, MethodBinding
binding,
ReferenceBinding searchThisType) {
+ /*
+ * Always look for uprefs in the original, so we can correctly
compare
+ * erased signatures. The general design for uprefs is to model what
the
+ * JVM does in terms of matching up overrides based on binary match.
+ */
+ searchThisType = (ReferenceBinding) searchThisType.original();
// See if this class has any uprefs, unless this class is myself
if (binding.declaringClass != searchThisType) {
for (MethodBinding tryMethod :
searchThisType.getMethods(binding.selector)) {
- if (binding.areParameterErasuresEqual(tryMethod)) {
+ if (binding.returnType.erasure() ==
tryMethod.returnType.erasure()
+ && binding.areParameterErasuresEqual(tryMethod)) {
JMethod upRef = (JMethod) typeMap.get(tryMethod);
if (!method.getOverrides().contains(upRef)) {
method.addOverride(upRef);
@@ -2790,6 +2791,8 @@
if (method.getName().equals(methodName)) {
String sig = JProgram.getJsniSig(method);
if (sig.equals(jsniSig)) {
+ return method;
+ } else if (sig.startsWith(jsniSig) &&
jsniSig.endsWith(")")) {
return method;
} else {
almostMatches.add(sig);
Modified:
trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
==============================================================================
---
trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
(original)
+++
trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
Mon Apr 6 12:48:03 2009
@@ -405,10 +405,12 @@
if (x instanceof JMethod) {
sb.append('(');
- for (JType t : ((JMethod) x).getOriginalParamTypes()) {
+ JMethod method = ((JMethod) x);
+ for (JType t : method.getOriginalParamTypes()) {
sb.append(t.getJsniSignatureName());
}
sb.append(')');
+ sb.append(method.getOriginalReturnType().getJsniSignatureName());
}
SymbolData symbolData = StandardSymbolData.forMember(
@@ -1958,6 +1960,7 @@
JType type = x.getOriginalParamTypes().get(i);
s += type.getJavahSignatureName();
}
+ s += x.getOriginalReturnType().getJavahSignatureName();
return s;
}
@@ -1980,6 +1983,7 @@
JType type = x.getOriginalParamTypes().get(i);
s += type.getJavahSignatureName();
}
+ s += x.getOriginalReturnType().getJavahSignatureName();
return s;
}
Modified:
trunk/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java
==============================================================================
--- trunk/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java
(original)
+++ trunk/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java Mon
Apr 6 12:48:03 2009
@@ -166,7 +166,7 @@
List<JType> originalParamTypes = new ArrayList<JType>();
originalParamTypes.add(enclosingType);
originalParamTypes.addAll(x.getOriginalParamTypes());
- newMethod.setOriginalParamTypes(originalParamTypes);
+ newMethod.setOriginalTypes(x.getOriginalReturnType(),
originalParamTypes);
// Move the body of the instance method to the static method
JAbstractMethodBody movedBody = x.getBody();
@@ -230,6 +230,9 @@
*/
private class FindStaticDispatchSitesVisitor extends JVisitor {
+ private JMethod currentMethod;
+ private ControlFlowAnalyzer initiallyLive;
+
@Override
public void endVisit(JMethodCall x, Context ctx) {
JMethod method = x.getTarget();
@@ -259,9 +262,36 @@
// The target method was already pruned (TypeTightener will fix
this).
return;
}
+
+ if (initiallyLive.getLiveFieldsAndMethods().contains(currentMethod)
+
&& !initiallyLive.getLiveFieldsAndMethods().contains(x.getTarget())) {
+ /*
+ * Don't devirtualize calls from initial code to non-initial code.
+ */
+ return;
+ }
// Let's do it!
toBeMadeStatic.add(method);
+ }
+
+ @Override
+ public boolean visit(JMethod x, Context ctx) {
+ currentMethod = x;
+ return true;
+ }
+
+ @Override
+ public boolean visit(JProgram x, Context ctx) {
+ // TODO(spoon) factor out this computation of the initially live
stuff to
+ // a method in CodeSplitter
+ initiallyLive = new ControlFlowAnalyzer(x);
+ for (JMethod entry : x.entryMethods.get(0)) {
+ initiallyLive.traverseFrom(entry);
+ }
+ initiallyLive.traverseFromClassLiteralFactories();
+
+ return true;
}
}
Modified: trunk/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
==============================================================================
--- trunk/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
(original)
+++ trunk/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java Mon Apr
6 12:48:03 2009
@@ -28,6 +28,10 @@
@SuppressWarnings("unused")
public class CompilerTest extends GWTTestCase {
+ interface MyMap {
+ Object get(String key);
+ }
+
interface Silly { }
interface SillyComparable<T extends Silly> extends Comparable<T> {
@@ -319,6 +323,26 @@
}
assertEquals(0, new MyFoo().compareTo(new MyFoo()));
+ }
+
+ /**
+ * Issue 3517. Sometimes JDT adds a bridge method when a subclass's
method
+ * differs only by return type. In some versions of GWT, this has
resulted in
+ * a bridge method overriding its own target, and eventually
TypeTightener
+ * producing an infinite recursion.
+ */
+ public void testBridgeMethods4() {
+ abstract class MyMapAbstract<V> implements MyMap {
+ public String get(String key) {
+ return null;
+ }
+ }
+
+ final class MyMapImpl<V> extends MyMapAbstract<V> {
+ }
+
+ MyMapImpl<String> mmap = new MyMapImpl<String>();
+ assertNull(mmap.get("foo"));
}
public void testCastOptimizer() {
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
