Author: jlaba...@google.com
Date: Tue Mar 31 10:58:25 2009
New Revision: 5137
Modified:
branches/snapshot-2009.03.30-r5111/branch-info.txt
branches/snapshot-2009.03.30-r5111/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
branches/snapshot-2009.03.30-r5111/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
Log:
A rollback of /tr...@c4974 and /tr...@c4943 to undo a compiler bug breaking
method overrides
Patch by: jlabanca
Modified: branches/snapshot-2009.03.30-r5111/branch-info.txt
==============================================================================
--- branches/snapshot-2009.03.30-r5111/branch-info.txt (original)
+++ branches/snapshot-2009.03.30-r5111/branch-info.txt Tue Mar 31 10:58:25
2009
@@ -4,3 +4,6 @@
Copies:
/branches/snapshot-2009.03.30 was created (r5135) as a straight copy from
/trunk/@5111
+
+Merges:
+A rollback of /tr...@c4974 and /tr...@c4943 was merged (r????) into this
branch to undo a compiler bug breaking method overrides
Modified:
branches/snapshot-2009.03.30-r5111/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
==============================================================================
---
branches/snapshot-2009.03.30-r5111/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
(original)
+++
branches/snapshot-2009.03.30-r5111/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
Tue Mar 31 10:58:25 2009
@@ -173,7 +173,6 @@
import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding;
import org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.SyntheticArgumentBinding;
-import org.eclipse.jdt.internal.compiler.lookup.SyntheticMethodBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeIds;
import org.eclipse.jdt.internal.compiler.lookup.VariableBinding;
@@ -185,8 +184,11 @@
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.LinkedList;
@@ -222,6 +224,55 @@
* them to our AST.
*/
private static class JavaASTGenerationVisitor {
+
+ /**
+ * Find all interface methods declared to be implemented by a specified
+ * class.
+ */
+ private static Collection<MethodBinding> findInterfaceMethods(
+ ReferenceBinding clazzBinding) {
+ List<MethodBinding> methods = new ArrayList<MethodBinding>();
+ Set<ReferenceBinding> seen = new HashSet<ReferenceBinding>();
+ findInterfaceMethodsRecursive(clazzBinding, methods, seen);
+ return methods;
+ }
+
+ private static void findInterfaceMethodsRecursive(
+ ReferenceBinding clazzBinding, List<MethodBinding> methods,
+ Set<ReferenceBinding> seen) {
+ if (!seen.add(clazzBinding)) {
+ return;
+ }
+
+ if (clazzBinding.isInterface()) {
+ methods.addAll(Arrays.asList(clazzBinding.methods()));
+ }
+
+ ReferenceBinding superclass = clazzBinding.superclass();
+ if (superclass != null) {
+ findInterfaceMethodsRecursive(superclass, methods, seen);
+ }
+
+ ReferenceBinding[] interfaces = clazzBinding.superInterfaces();
+ if (interfaces != null) {
+ for (ReferenceBinding supinterf : interfaces) {
+ findInterfaceMethodsRecursive(supinterf, methods, seen);
+ }
+ }
+ }
+
+ private static boolean inheritsMethodWithIdenticalSignature(
+ JClassType clazz, JMethod method) {
+ for (JClassType sup = clazz.extnds; sup != null; sup = sup.extnds) {
+ for (JMethod m : sup.methods) {
+ if (JTypeOracle.methodsDoMatch(m, method)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
private static InternalCompilerException translateException(JNode node,
Throwable e) {
if (e instanceof OutOfMemoryError) {
@@ -289,6 +340,11 @@
* </p>
*
* <p>
+ * This method assumes that method overrides are already recorded for
the
+ * class and all its inherited classes and interfaces.
+ * </p>
+ *
+ * <p>
* The need for these bridges was pointed out in issue 3064. The goal
is
* that virtual method calls through an interface type are translated
to
* JavaScript that will function correctly. If the interface signature
@@ -302,31 +358,57 @@
* case, a bridge method should be added that overrides the interface
method
* and then calls the implementation method.
* </p>
- *
- * <p>
- * This method should only be called once all regular, non-bridge
methods
- * have been installed on the GWT types.
- * </p>
*/
- public void addBridgeMethods(SourceTypeBinding clazzBinding) {
+ public void addBridgeMethods(ReferenceBinding clazzBinding,
+ JProgram program, Set<ReferenceBinding> typesWithBridges) {
if (clazzBinding.isInterface() || clazzBinding.isAbstract()) {
// Only add bridges in concrete classes, to simplify matters.
return;
}
- JClassType clazz = (JClassType) typeMap.get(clazzBinding);
+ if (!typesWithBridges.add(clazzBinding)) {
+ // Nothing to do -- this class already has bridges added.
+ return;
+ }
/*
- * The JDT adds bridge methods in all the places GWT needs them. Look
- * through the bridge methods the JDT added.
+ * Add to the superclass first, so that bridge methods end up as
high in
+ * the hierarchy as possible.
*/
- if (clazzBinding.syntheticMethods() != null) {
- for (SyntheticMethodBinding synthmeth :
clazzBinding.syntheticMethods()) {
- if (synthmeth.purpose == SyntheticMethodBinding.BridgeMethod) {
- JMethod implmeth = (JMethod)
typeMap.get(synthmeth.targetMethod);
+ if (clazzBinding.superclass() != null) {
+ addBridgeMethods(clazzBinding.superclass(), program,
typesWithBridges);
+ }
- createBridgeMethod(clazz, synthmeth, implmeth);
+ JClassType clazz = (JClassType) typeMap.get(clazzBinding);
+
+ for (MethodBinding imethBinding :
findInterfaceMethods(clazzBinding)) {
+ JMethod interfmeth = (JMethod) typeMap.get(imethBinding);
+ JMethod implmeth = (JMethod) typeMap.get(findMethodImplementing(
+ imethBinding, clazzBinding));
+
+ assert (!implmeth.isStatic());
+
+ if (!implmeth.overrides.contains(interfmeth)) {
+ if (JTypeOracle.methodsDoMatch(interfmeth, implmeth)) {
+ /*
+ * Two cases are caught here. First, a bridge method might
already
+ * be included in a superclass. In that case, there's nothing
to do.
+ * Second, the bridge method might have the exact signature as
the
+ * bridged-to method. In that case, leave out the bridge
method. It
+ * makes things harder on the optimizers, but it avoids adding
a
+ * bridge method that must later be removed. Further, such a
bridge
+ * method would need to be different from the current ones in
order
+ * to avoid an infinite recursion.
+ */
+ continue;
}
+
+ if (inheritsMethodWithIdenticalSignature(clazz, interfmeth)) {
+ // An equivalent bridge has already been added in a superclass
+ continue;
+ }
+
+ createBridgeMethod(program, clazz, interfmeth, implmeth);
}
}
}
@@ -2033,44 +2115,29 @@
}
}
- private boolean classHasMethodOverriding(JClassType clazz, JMethod
over) {
- for (JMethod meth : clazz.methods) {
- if (meth.overrides.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.
+ *
+ * @param program The program being modified
* @param clazz The class to put the bridge method in
- * @param jdtBridgeMethod The corresponding bridge method added in the
JDT
+ * @param interfmeth The interface method to bridge from
* @param implmeth The implementation method to bridge to
*/
- private void createBridgeMethod(JClassType clazz,
SyntheticMethodBinding jdtBridgeMethod,
- JMethod implmeth) {
+ private void createBridgeMethod(JProgram program, JClassType clazz,
+ JMethod interfmeth, JMethod implmeth) {
SourceInfo info = program.createSourceInfoSynthetic(
GenerateJavaAST.class, "bridge method");
// create the method itself
JMethod bridgeMethod = program.createMethod(info,
- jdtBridgeMethod.selector, clazz,
- (JType) typeMap.get(jdtBridgeMethod.returnType.erasure()), false,
- false, true, false, false);
- int paramIdx = 0;
- for (TypeBinding jdtParamType : jdtBridgeMethod.parameters) {
- String paramName = "p" + paramIdx++;
- JType paramType = (JType) typeMap.get(jdtParamType.erasure());
+ interfmeth.getName().toCharArray(), clazz, interfmeth.getType(),
+ false, false, true, false, false);
+ for (JParameter param : interfmeth.params) {
program.createParameter(program.createSourceInfoSynthetic(
GenerateJavaAST.class, "part of a bridge method"),
- paramName.toCharArray(), paramType, true, false, bridgeMethod);
+ param.getName().toCharArray(), param.getType(), true, false,
+ bridgeMethod);
}
bridgeMethod.freezeParamTypes();
@@ -2104,16 +2171,9 @@
JMethodBody body = (JMethodBody) bridgeMethod.getBody();
body.getStatements().add(callOrReturn);
- // add overrides, but only for interface methods that the class does
not
- // already override
- List<JMethod> overrides = new ArrayList<JMethod>();
- tryFindUpRefs(bridgeMethod, overrides);
- for (JMethod over : overrides) {
- if (!classHasMethodOverriding(clazz, over)) {
- bridgeMethod.overrides.add(over);
- bridgeMethod.overrides.addAll(over.overrides);
- }
- }
+ // make the bridge override the interface method
+ bridgeMethod.overrides.add(interfmeth);
+ bridgeMethod.overrides.addAll(interfmeth.overrides);
}
private JDeclarationStatement createDeclaration(SourceInfo info,
@@ -2294,6 +2354,27 @@
}
/**
+ * Search the class hierarchy starting at <code>clazz</code> looking
for a
+ * method implementing <code>imeth</code>. Look only in classes, not
+ * interfaces.
+ */
+ private MethodBinding findMethodImplementing(MethodBinding interfmeth,
+ ReferenceBinding clazz) {
+ for (MethodBinding tryMethod :
clazz.getMethods(interfmeth.selector)) {
+ if (methodParameterErasuresAreEqual(interfmeth, tryMethod)) {
+ return tryMethod;
+ }
+ }
+
+ if (clazz.superclass() == null) {
+ throw new InternalCompilerException("Could not find implementation
of "
+ + interfmeth + " for class " + clazz);
+ }
+
+ return findMethodImplementing(interfmeth, clazz.superclass());
+ }
+
+ /**
* Get a new label of a particular name, or create a new one if it
doesn't
* exist already.
*/
@@ -2431,6 +2512,32 @@
}
/**
+ * Check whether two methods have matching parameter types. Assumes the
+ * selectors match.
+ */
+ private boolean methodParameterErasuresAreEqual(MethodBinding meth1,
+ MethodBinding meth2) {
+ /*
+ * Don't use MethodBinding.areParameterErasuresEqual because that
method
+ * assumes equal types are ==, but that's not necessarily true in
this
+ * context.
+ */
+ if (meth1.parameters.length != meth2.parameters.length) {
+ return false;
+ }
+
+ for (int i = 0; i < meth1.parameters.length; i++) {
+ TypeBinding type1 = meth1.parameters[i].erasure();
+ TypeBinding type2 = meth2.parameters[i].erasure();
+ if (typeMap.get(type1) != typeMap.get(type2)) {
+ return false;
+ }
+ }
+
+ return true;
+ }
+
+ /**
* Sometimes a variable reference can be to a local or parameter in an
an
* enclosing method. This is a tricky situation to detect. There's no
* obvious way to tell, but the clue we can get from JDT is that the
local's
@@ -2541,15 +2648,13 @@
/**
* For a given method, try to find all methods that it
overrides/implements.
- * This version does not use JDT.
+ * An experimental version that doesn't use JDT. Right now it's only
used to
+ * resolve upRefs for Object.getClass(), which is synthetic on
everything
+ * other than object.
*/
private void tryFindUpRefs(JMethod method) {
- tryFindUpRefs(method, method.overrides);
- }
-
- private void tryFindUpRefs(JMethod method, List<JMethod> overrides) {
if (method.getEnclosingType() != null) {
- tryFindUpRefsRecursive(method, method.getEnclosingType(),
overrides);
+ tryFindUpRefsRecursive(method, method.getEnclosingType());
}
}
@@ -2566,14 +2671,28 @@
* methods that it overrides/implements.
*/
private void tryFindUpRefsRecursive(JMethod method,
- JReferenceType searchThisType, List<JMethod> overrides) {
+ JReferenceType searchThisType) {
// See if this class has any uprefs, unless this class is myself
if (method.getEnclosingType() != searchThisType) {
- for (JMethod upRef : searchThisType.methods) {
- if (JTypeOracle.methodsDoMatch(method, upRef)
- && !overrides.contains(upRef)) {
- overrides.add(upRef);
+ outer : for (JMethod upRef : searchThisType.methods) {
+ if (upRef.isStatic()) {
+ continue;
+ }
+ if (!upRef.getName().equals(method.getName())) {
+ continue;
+ }
+ if (upRef.params.size() != method.params.size()) {
+ continue;
+ }
+ for (int i = 0, c = upRef.params.size(); i < c; ++i) {
+ if (upRef.params.get(i).getType() !=
method.params.get(i).getType()) {
+ continue outer;
+ }
+ }
+
+ if (!method.overrides.contains(upRef)) {
+ method.overrides.add(upRef);
break;
}
}
@@ -2581,12 +2700,12 @@
// recurse super class
if (searchThisType.extnds != null) {
- tryFindUpRefsRecursive(method, searchThisType.extnds, overrides);
+ tryFindUpRefsRecursive(method, searchThisType.extnds);
}
// recurse super interfaces
for (JInterfaceType intf : searchThisType.implments) {
- tryFindUpRefsRecursive(method, intf, overrides);
+ tryFindUpRefsRecursive(method, intf);
}
}
@@ -2973,14 +3092,17 @@
// Construct the basic AST.
JavaASTGenerationVisitor v = new JavaASTGenerationVisitor(typeMap,
jprogram, options);
- for (TypeDeclaration type : types) {
- v.processType(type);
- }
- for (TypeDeclaration type : types) {
- v.addBridgeMethods(type.binding);
+ for (int i = 0; i < types.length; ++i) {
+ v.processType(types[i]);
}
Collections.sort(jprogram.getDeclaredTypes(), new HasNameSort());
+ // add any necessary bridge methods
+ Set<ReferenceBinding> typesWithBridges = new
HashSet<ReferenceBinding>();
+ for (TypeDeclaration decl : types) {
+ v.addBridgeMethods(decl.binding, jprogram, typesWithBridges);
+ }
+
// Process JSNI.
Map<JsniMethodBody, AbstractMethodDeclaration> jsniMethodMap =
v.getJsniMethodMap();
new JsniRefGenerationVisitor(jprogram, jsProgram,
jsniMethodMap).accept(jprogram);
@@ -2998,6 +3120,32 @@
IProblem.ExternalProblemNotFixable, null, ProblemSeverities.Error,
info.getStartPos(), info.getEndPos(), info.getStartLine(),
startColumn);
compResult.record(problem, methodDeclaration);
+ }
+
+ private static void addSuperclassesAndInterfaces(JReferenceType clazz,
+ Set<JReferenceType> supers) {
+ if (clazz == null) {
+ return;
+ }
+ if (supers.contains(clazz)) {
+ return;
+ }
+ supers.add(clazz);
+ addSuperclassesAndInterfaces(clazz.extnds, supers);
+ for (JReferenceType intf : clazz.implments) {
+ addSuperclassesAndInterfaces(intf, supers);
+ }
+ }
+
+ /**
+ * Returns a collection of all inherited classes and interfaces, plus the
+ * class itself.
+ */
+ private static Collection<JReferenceType> allSuperClassesAndInterfaces(
+ JClassType clazz) {
+ Set<JReferenceType> supers = new LinkedHashSet<JReferenceType>();
+ addSuperclassesAndInterfaces(clazz, supers);
+ return supers;
}
/**
Modified:
branches/snapshot-2009.03.30-r5111/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
==============================================================================
---
branches/snapshot-2009.03.30-r5111/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
(original)
+++
branches/snapshot-2009.03.30-r5111/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
Tue Mar 31 10:58:25 2009
@@ -20,20 +20,12 @@
import junit.framework.Assert;
-import java.util.EventListener;
-
/**
* Miscellaneous tests of the Java to JavaScript compiler.
*/
@SuppressWarnings("unused")
public class CompilerTest extends GWTTestCase {
- interface Silly { }
-
- interface SillyComparable<T extends Silly> extends Comparable<T> {
- int compareTo(T obj);
- }
-
private abstract static class AbstractSuper {
public static String foo() {
if (FALSE) {
@@ -47,23 +39,6 @@
private abstract static class Apple implements Fruit {
}
- private abstract static class Bm2BaseEvent {
- }
-
- private abstract static class Bm2ComponentEvent extends Bm2BaseEvent {
- }
-
- private static class Bm2KeyNav<E extends Bm2ComponentEvent> implements
- Bm2Listener<E> {
- public int handleEvent(Bm2ComponentEvent ce) {
- return 5;
- }
- }
-
- private interface Bm2Listener<E extends Bm2BaseEvent> extends
EventListener {
- int handleEvent(E be);
- }
-
private static class ConcreteSub extends AbstractSuper {
public static String foo() {
if (FALSE) {
@@ -265,7 +240,7 @@
* superclass, it can be necessary to add a bridge method that overrides
the
* interface method and calls the inherited method.
*/
- public void testBridgeMethods1() {
+ public void testBridgeMethods() {
abstract class AbstractFoo {
public int compareTo(AbstractFoo o) {
return 0;
@@ -287,39 +262,10 @@
Comparable<AbstractFoo> comparable1 = new MyFooSub();
assertEquals(0, comparable1.compareTo(new MyFoo()));
+
Comparable<AbstractFoo> comparable2 = new MyFoo();
assertEquals(0, comparable2.compareTo(new MyFooSub()));
- }
-
- /**
- * Issue 3304. Doing superclasses first is tricky when the superclass is
a raw
- * type.
- */
- @SuppressWarnings("unchecked")
- public void testBridgeMethods2() {
- Bm2KeyNav<?> obs = new Bm2KeyNav() {
- };
- assertEquals(5, obs.handleEvent(null));
- }
-
- /**
- * When adding a bridge method, be sure to handle transitive overrides
- * relationships.
- */
- public void testBridgeMethods3() {
- class AbstractFoo implements Silly {
- public int compareTo(AbstractFoo obj) {
- if (FALSE) {
- return compareTo(obj);
- }
- return 0;
- }
- }
- class MyFoo extends AbstractFoo implements
SillyComparable<AbstractFoo> {
- }
-
- assertEquals(0, new MyFoo().compareTo(new MyFoo()));
- }
+}
public void testCastOptimizer() {
Granny g = new Granny();
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
