This patch improves the correctness of SingleJso types by following all super-interfaces when calculating singleJso data. Additionally, casts to singleJso interfaces will trigger a rescue of the concrete JSO type.
Without these changes, singleJso types that are referred to only via cast operations would generate incorrect code at the cast site, as a Cast.throwClassCastExceptionUnlessNull() call would be made instead of a dynamicCastJso() as the compiler would decide there are no concrete implementations of the singleJso interface type.
Patch by: bobv, mtsui
Review by: spoon
Reported by: macpherson, mtsui
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@5466 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
index 5cc2fbd..337b94f 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
@@ -183,8 +183,8 @@
*
* @param type any type
* @param instantiatedTypes a set of types assumed to be instantiated. If
- * <code>null</code>, then there are no assumptions about which
- * types are instantiated.
+ * <code>null</code>, then there are no assumptions about which types
+ * are instantiated.
* @return whether the type is instantiated
*/
private static boolean isInstantiatedType(JReferenceType type,
@@ -536,6 +536,25 @@
}
/**
+ * Computes the set of all interfaces implemented by a type.
+ */
+ private Set<JInterfaceType> allSuperInterfaces(JDeclaredType type) {
+ Set<JInterfaceType> toReturn = new IdentityHashSet<JInterfaceType>();
+ List<JInterfaceType> q = new LinkedList<JInterfaceType>();
+ q.addAll(type.getImplements());
+
+ while (!q.isEmpty()) {
+ JInterfaceType t = q.remove(0);
+
+ if (toReturn.add(t)) {
+ q.addAll(t.getImplements());
+ }
+ }
+
+ return toReturn;
+ }
+
+ /**
* Compute all of the things I might conceivably implement, either through
* super types or sub types.
*/
@@ -653,7 +672,7 @@
for (JDeclaredType type : program.getDeclaredTypes()) {
if (!program.isJavaScriptObject(type)) {
if (type instanceof JClassType) {
- dualImpl.addAll(type.getImplements());
+ dualImpl.addAll(allSuperInterfaces(type));
}
continue;
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
index 383835a..8e08a11 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
@@ -175,7 +175,23 @@
JType targetType = x.getCastType();
if (program.isJavaScriptObject(targetType)) {
rescue((JReferenceType) targetType, true, true);
+ } else {
+ /*
+ * If there's a cast to a SingleJso interface, rescue the implementing
+ * JSO type. If the JSO type isn't rescued (and there's no other regular
+ * Java type implementing the interface), then the cast operation will
+ * be replaced with a throwCCEUnlessNull() call, since there's no type
+ * left in the type system that implements the interface. If there is an
+ * implementing Java type, then a dynamicCast() will be emitted which
+ * will throw a CCE if it hits a JSO type.
+ */
+ JClassType maybeSingleJso = program.typeOracle.getSingleJsoImpls().get(
+ targetType);
+ if (maybeSingleJso != null) {
+ rescue(maybeSingleJso, true, true);
+ }
}
+
return true;
}
diff --git a/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java b/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
index 8ed2db3..2e99313 100644
--- a/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
+++ b/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
@@ -993,7 +993,7 @@
if (unit != null) {
anonymousClassMap = unit.getAnonymousClassMap();
}
- byte[] newBytes = classRewriter.rewrite(className, classBytes,
+ byte[] newBytes = classRewriter.rewrite(this, className, classBytes,
anonymousClassMap);
if (CLASS_DUMP) {
if (!Arrays.equals(classBytes, newBytes)) {
diff --git a/dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java b/dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java
index 7b88173..6f1621a 100644
--- a/dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java
+++ b/dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java
@@ -183,12 +183,13 @@
/**
* Performs rewriting transformations on a class.
*
+ * @param ccl the ClassLoader requesting the rewrite
* @param className the name of the class
* @param classBytes the bytes of the class
* @param anonymousClassMap a map between the anonymous class names of java
* compiler used to compile code and jdt. Emma-specific.
*/
- public byte[] rewrite(String className, byte[] classBytes,
+ public byte[] rewrite(ClassLoader ccl, String className, byte[] classBytes,
Map<String, String> anonymousClassMap) {
String desc = toDescriptor(className);
assert (!jsoIntfDescs.contains(desc));
@@ -200,7 +201,7 @@
// v = new CheckClassAdapter(v);
// v = new TraceClassVisitor(v, new PrintWriter(System.out));
- v = new RewriteSingleJsoImplDispatches(v, singleJsoImplTypes,
+ v = new RewriteSingleJsoImplDispatches(v, ccl, singleJsoImplTypes,
mangledNamesToImplementations);
v = new RewriteRefsToJsoClasses(v, jsoIntfDescs, mapper);
diff --git a/dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java b/dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java
index b08e667..94bb354 100644
--- a/dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java
+++ b/dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java
@@ -23,9 +23,9 @@
import com.google.gwt.dev.asm.Type;
import com.google.gwt.dev.asm.commons.Method;
-import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
+import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -62,8 +62,57 @@
@Override
public void visitMethodInsn(int opcode, String owner, String name,
String desc) {
- if (singleJsoImplTypes.contains(owner)) {
- name = owner.replace('/', '_') + "_" + name;
+ if (opcode == Opcodes.INVOKEINTERFACE) {
+ if (singleJsoImplTypes.contains(owner)) {
+ // Simple case; referring directly to a SingleJso interface.
+ name = owner.replace('/', '_') + "_" + name;
+ assert mangledNamesToImplementations.containsKey(name) : "Missing "
+ + name;
+
+ } else {
+ /*
+ * Might be referring to a subtype of a SingleJso interface:
+ *
+ * interface IA { void foo() }
+ *
+ * interface JA extends JSO implements IA;
+ *
+ * interface IB extends IA {}
+ *
+ * void bar() { ((IB) object).foo(); }
+ */
+ for (String intf : computeAllInterfaces(owner)) {
+ if (singleJsoImplTypes.contains(intf)) {
+ /*
+ * Check that it really should be mangled and is not a reference
+ * to a method defined in a non-singleJso super-interface. If
+ * there are two super-interfaces that define methods with
+ * identical names and descriptors, the choice of implementation
+ * is undefined.
+ */
+ String maybeMangled = intf.replace('/', '_') + "_" + name;
+ Method method = mangledNamesToImplementations.get(maybeMangled);
+ if (method != null) {
+ /*
+ * Found a method with the right name, but we need to check the
+ * parameters and the return type. In order to do this, we'll
+ * look at the arguments and return type of the target method,
+ * removing the first argument, which is the instance.
+ */
+ assert method.getArgumentTypes().length >= 1;
+ Type[] argumentTypes = new Type[method.getArgumentTypes().length - 1];
+ System.arraycopy(method.getArgumentTypes(), 1, argumentTypes,
+ 0, argumentTypes.length);
+ String maybeDescriptor = Type.getMethodDescriptor(
+ method.getReturnType(), argumentTypes);
+ if (maybeDescriptor.equals(desc)) {
+ name = maybeMangled;
+ break;
+ }
+ }
+ }
+ }
+ }
}
super.visitMethodInsn(opcode, owner, name, desc);
@@ -76,10 +125,13 @@
private final Set<String> singleJsoImplTypes;
private boolean inSingleJsoImplInterfaceType;
- public RewriteSingleJsoImplDispatches(ClassVisitor v,
+ private final ClassLoader ccl;
+
+ public RewriteSingleJsoImplDispatches(ClassVisitor v, ClassLoader ccl,
Set<String> singleJsoImplTypes,
SortedMap<String, Method> mangledNamesToImplementations) {
super(v);
+ this.ccl = ccl;
this.singleJsoImplTypes = Collections.unmodifiableSet(singleJsoImplTypes);
this.mangledNamesToImplementations = Collections.unmodifiableSortedMap(mangledNamesToImplementations);
}
@@ -90,6 +142,15 @@
assert currentTypeName == null;
super.visit(version, access, name, signature, superName, interfaces);
+ /*
+ * This visitor would mangle JSO$ since it acts as a roll-up of all
+ * SingleJso types and the result would be repeated method definitions due
+ * to the trampoline methods this visitor would create.
+ */
+ if (name.equals(HostedModeClassRewriter.JAVASCRIPTOBJECT_IMPL_DESC)) {
+ return;
+ }
+
currentTypeName = name;
inSingleJsoImplInterfaceType = singleJsoImplTypes.contains(name);
@@ -100,8 +161,7 @@
* original methods.
*/
if (interfaces != null && (access & Opcodes.ACC_INTERFACE) == 0) {
- List<String> toStub = new ArrayList<String>();
- Collections.addAll(toStub, interfaces);
+ Set<String> toStub = computeAllInterfaces(interfaces);
toStub.retainAll(singleJsoImplTypes);
for (String stubIntr : toStub) {
@@ -146,6 +206,29 @@
return new MyMethodVisitor(mv);
}
+ private Set<String> computeAllInterfaces(String... interfaces) {
+ Set<String> toReturn = new HashSet<String>();
+ List<String> q = new LinkedList<String>();
+ Collections.addAll(q, interfaces);
+
+ while (!q.isEmpty()) {
+ String intf = q.remove(0);
+ if (toReturn.add(intf)) {
+ try {
+ Class<?> intfClass = Class.forName(intf.replace('/', '.'), false, ccl);
+ for (Class<?> i : intfClass.getInterfaces()) {
+ q.add(i.getName().replace('.', '/'));
+ }
+ } catch (ClassNotFoundException e) {
+ assert false : intf;
+ e.printStackTrace();
+ }
+ }
+ }
+
+ return toReturn;
+ }
+
/**
* Given a resource name of a class, find all mangled method names that must
* be implemented.
diff --git a/user/test/com/google/gwt/dev/jjs/CompilerSuite.java b/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
index db89ae3..a2d754d 100644
--- a/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
+++ b/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
@@ -44,6 +44,7 @@
import com.google.gwt.dev.jjs.test.SingleJsoImplTest;
import com.google.gwt.dev.jjs.test.UnstableGeneratorTest;
import com.google.gwt.dev.jjs.test.VarargsTest;
+import com.google.gwt.dev.jjs.test.singlejso.TypeHierarchyTest;
import com.google.gwt.junit.tools.GWTTestSuite;
import junit.framework.Test;
@@ -84,6 +85,7 @@
suite.addTestSuite(RunAsyncFailureTest.class);
suite.addTestSuite(RunAsyncTest.class);
suite.addTestSuite(SingleJsoImplTest.class);
+ suite.addTestSuite(TypeHierarchyTest.class);
suite.addTestSuite(UnstableGeneratorTest.class);
suite.addTestSuite(VarargsTest.class);
// $JUnit-END$
diff --git a/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java b/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java
index 3926e4e..4847899 100644
--- a/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java
+++ b/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java
@@ -66,6 +66,10 @@
String call(int a, int b);
}
+ interface CreatedWithCast {
+ String foo();
+ }
+
interface Divider extends Multiplier {
int divide(int a, int b);
}
@@ -81,6 +85,16 @@
}
/**
+ * Even though this type is never instantiated, it's necessary to ensure that
+ * the CreatedWithCast test isn't short-circuited due to type tightening.
+ */
+ static class JavaCreatedWithCast implements CreatedWithCast {
+ public String foo() {
+ return "foo";
+ }
+ }
+
+ /**
* The extra declaration of implementing Multiplier should still be legal.
*/
static class JavaDivider extends JavaMultiplier implements Divider,
@@ -189,6 +203,16 @@
}
}
+ static class JsoCreatedWithCast extends JavaScriptObject implements
+ CreatedWithCast {
+ protected JsoCreatedWithCast() {
+ }
+
+ public final String foo() {
+ return "foo";
+ }
+ }
+
static class JsoDivider extends JsoMultiplier implements Divider, Tag {
protected JsoDivider() {
}
@@ -452,6 +476,16 @@
a.acceptObject3Array(a.returnString3Array());
}
+ /**
+ * Ensure that SingleJSO types that are referred to only via a cast to the
+ * interface type are retained. If the JsoCreatedWithCast type isn't rescued
+ * correctly, the cast in this test will throw a ClassCastException since the
+ * compiler would assume there are types that implement the interface.
+ */
+ public void testCreatedWithCast() {
+ Object o = (CreatedWithCast) JavaScriptObject.createObject();
+ }
+
public void testDualCase() {
// Direct dispatch
{
diff --git a/user/test/com/google/gwt/dev/jjs/test/singlejso/A.java b/user/test/com/google/gwt/dev/jjs/test/singlejso/A.java
new file mode 100644
index 0000000..068fb94
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/test/singlejso/A.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2009 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.dev.jjs.test.singlejso;
+
+import com.google.gwt.core.client.JavaScriptObject;
+
+/**
+ * A class.
+ */
+public class A extends JavaScriptObject implements IA {
+ protected A() {
+ }
+
+ public static A create() {
+ return JavaScriptObject.createObject().cast();
+ }
+
+ public final String whoAmI() {
+ return "A";
+ }
+}
diff --git a/user/test/com/google/gwt/dev/jjs/test/singlejso/B1.java b/user/test/com/google/gwt/dev/jjs/test/singlejso/B1.java
new file mode 100644
index 0000000..f337cea
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/test/singlejso/B1.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2009 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.dev.jjs.test.singlejso;
+
+/**
+ * B1 class.
+ */
+public class B1 implements IB {
+ public String sayHello() {
+ return "Hey, I'm " + whoAmI();
+ }
+
+ public String whoAmI() {
+ return "B1";
+ }
+}
diff --git a/user/test/com/google/gwt/dev/jjs/test/singlejso/B2.java b/user/test/com/google/gwt/dev/jjs/test/singlejso/B2.java
new file mode 100644
index 0000000..6dc39fa
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/test/singlejso/B2.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2009 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.dev.jjs.test.singlejso;
+
+/**
+ * B2 class.
+ */
+public class B2 implements IB {
+ public String sayHello() {
+ return "Hi, I'm " + whoAmI();
+ }
+
+ public String whoAmI() {
+ return "B2";
+ }
+}
diff --git a/user/test/com/google/gwt/dev/jjs/test/singlejso/IA.java b/user/test/com/google/gwt/dev/jjs/test/singlejso/IA.java
new file mode 100644
index 0000000..faaf611
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/test/singlejso/IA.java
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2009 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.dev.jjs.test.singlejso;
+
+/**
+ * Interface A.
+ */
+public interface IA {
+ String whoAmI();
+}
diff --git a/user/test/com/google/gwt/dev/jjs/test/singlejso/IB.java b/user/test/com/google/gwt/dev/jjs/test/singlejso/IB.java
new file mode 100644
index 0000000..d11422b
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/test/singlejso/IB.java
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2009 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.dev.jjs.test.singlejso;
+
+/**
+ * Interface B.
+ */
+public interface IB extends IA {
+ String sayHello();
+}
diff --git a/user/test/com/google/gwt/dev/jjs/test/singlejso/TypeHierarchyTest.java b/user/test/com/google/gwt/dev/jjs/test/singlejso/TypeHierarchyTest.java
new file mode 100644
index 0000000..f50be8b
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/test/singlejso/TypeHierarchyTest.java
@@ -0,0 +1,63 @@
+/*
+ * Copyright 2009 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.dev.jjs.test.singlejso;
+
+import com.google.gwt.junit.client.GWTTestCase;
+
+/**
+ * Tests SingleJso semantics in non-trivial type hierarchies.
+ */
+public class TypeHierarchyTest extends GWTTestCase {
+
+ @Override
+ public String getModuleName() {
+ return "com.google.gwt.dev.jjs.CompilerSuite";
+ }
+
+ public void testCase1() {
+ A a = A.create();
+ assertEquals("A", a.whoAmI());
+
+ B1 b1 = new B1();
+ assertEquals("B1", b1.whoAmI());
+
+ B2 b2 = new B2();
+ assertEquals("B2", b2.whoAmI());
+ }
+
+ public void testCase2() {
+ IA a = A.create();
+ assertEquals("A", a.whoAmI());
+
+ IA b1 = new B1();
+ assertEquals("B1", b1.whoAmI());
+
+ IA b2 = new B2();
+ assertEquals("B2", b2.whoAmI());
+ }
+
+ public void testCase3() {
+ IA a = A.create();
+ assertEquals("A", a.whoAmI());
+
+ IB b1 = new B1();
+ assertEquals("B1", b1.whoAmI());
+
+ IB b2 = new B2();
+ assertEquals("B2", b2.whoAmI());
+ }
+
+}
