Revision: 10459
Author: [email protected]
Date: Thu Jul 14 13:39:15 2011
Log: JsoNormalizer did not give unique names to devirtualized
methods from Object overrides or DualJsoImpl types. The
obfuscated namer papered over this, but if you turn on
pretty mode, you can run into problems if two interfaces
have DualJsoImpl types with the same method names.
Review at http://gwt-code-reviews.appspot.com/1467812
http://code.google.com/p/google-web-toolkit/source/detail?r=10459
Added:
/trunk/dev/core/test/com/google/gwt/dev/jjs/impl/JsoDevirtualizerTest.java
Modified:
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/JsoDevirtualizer.java
=======================================
--- /dev/null
+++
/trunk/dev/core/test/com/google/gwt/dev/jjs/impl/JsoDevirtualizerTest.java
Thu Jul 14 13:39:15 2011
@@ -0,0 +1,95 @@
+/*
+ * Copyright 2011 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.impl;
+
+import com.google.gwt.core.ext.UnableToCompleteException;
+import com.google.gwt.dev.javac.testing.impl.MockJavaResource;
+import com.google.gwt.dev.jjs.ast.JMethod;
+import com.google.gwt.dev.jjs.ast.JProgram;
+
+/**
+ * Tests for the {@link JsoDevirtualizer} visitor.
+ */
+public class JsoDevirtualizerTest extends OptimizerTestBase {
+
+ /**
+ * JsoDevirtualizer should allow dual Java/JSO implementations of the
same
+ * interface, so long as there is only one of each. If there are multiple
+ * methods with the same method name, it should distinguish between them.
+ */
+ public void testDualJsoImpl() throws UnableToCompleteException {
+
+ sourceOracle.addOrReplace(new
MockJavaResource("com.google.gwt.lang.Cast") {
+ @Override
+ public CharSequence getContent() {
+ StringBuffer code = new StringBuffer();
+ code.append("package com.google.gwt.lang;");
+ code.append("public class Cast {");
+ code.append(" public static boolean isJavaObject(Object o) {
return true; };");
+ code.append(" public static boolean isJavaScriptObject(Object o)
{ return true; };");
+ code.append("}");
+ return code;
+ }
+ });
+
+ addSnippetImport("com.google.gwt.lang.Cast");
+ addSnippetImport("com.google.gwt.core.client.JavaScriptObject");
+
+ addSnippetClassDecl(
+ "interface Iface1 { int a(); }",
+ "static class J1 implements Iface1 {",
+ " public int a() { return 1; }",
+ "}",
+ "static class Jso1 extends JavaScriptObject implements Iface1 {",
+ " protected Jso1() { }",
+ " public final int a() { return 2; }",
+ " public static native Jso1 create() /*-{ return {} }-*/;",
+ "}",
+ "static interface Iface2 { int a(); }",
+ "static class J2 implements Iface2 {",
+ " public int a() { return 3; }",
+ "}",
+ "static class Jso2 extends JavaScriptObject implements Iface2 {",
+ " protected Jso2() { }", " public final int a() { return 4; }",
+ " public static native Jso2 create() /*-{ return {} }-*/;",
+ "}",
+ "static Iface1 val1 = new J1();",
+ "static Iface1 val2 = Jso1.create();",
+ "static Iface2 val3 = new J2();",
+ "static Iface2 val4 = Jso2.create();");
+
+ StringBuilder code = new StringBuilder();
+ code.append("int result = val1.a() + val2.a() + val3.a() + val4.a();");
+
+ // The salient point in the results below is that the JSO method used
for
+ // val1 and val1 has a different name the method used for val2 and
val3.
+ StringBuffer expected = new StringBuffer();
+ expected.append("int result = ");
+ expected.append("JavaScriptObject.a__devirtual$(EntryPoint.val1) + ");
+ expected.append("JavaScriptObject.a__devirtual$(EntryPoint.val2) + ");
+ expected.append("JavaScriptObject.a0__devirtual$(EntryPoint.val3) + ");
+ expected.append("JavaScriptObject.a0__devirtual$(EntryPoint.val4);");
+
+ optimize("void", code.toString()).intoString(expected.toString());
+ }
+
+ @Override
+ protected boolean optimizeMethod(JProgram program, JMethod method) {
+ JsoDevirtualizer.exec(program);
+ return true;
+ }
+}
=======================================
---
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
Tue Jul 12 06:13:56 2011
+++
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
Thu Jul 14 13:39:15 2011
@@ -153,6 +153,7 @@
import com.google.gwt.dev.util.collect.IdentityHashSet;
import com.google.gwt.dev.util.collect.Lists;
import com.google.gwt.dev.util.collect.Maps;
+import com.google.gwt.dev.util.collect.Sets;
import java.io.StringReader;
import java.util.ArrayList;
@@ -774,6 +775,9 @@
}
}
}
+
+ // TODO(zundel): Check that each unique method has a unique
+ // name / poly name.
}
@Override
@@ -1419,7 +1423,13 @@
if (x.getSuperClass() != null && !alreadyRan.contains(x)) {
accept(x.getSuperClass());
}
-
+
+ return super.visit(x, ctx);
+ }
+
+ @Override
+ public boolean visit(JDeclaredType x, Context ctx) {
+ checkForDupMethods(x);
return true;
}
@@ -1609,6 +1619,20 @@
push(jsSwitch);
return false;
}
+
+ private void checkForDupMethods(JDeclaredType x) {
+ // Sanity check to see that all methods are uniquely named.
+ List<JMethod> methods = x.getMethods();
+ Set<String> methodSignatures = Sets.create();
+ for (JMethod method : methods) {
+ String sig = method.getSignature();
+ if (methodSignatures.contains(sig)) {
+ throw new InternalCompilerException("Signature collision in
Type " + x.getName()
+ + " for method " + sig);
+ }
+ methodSignatures = Sets.add(methodSignatures, sig);
+ }
+ }
private JsExpression createAssignment(JsExpression lhs, JsExpression
rhs) {
return new JsBinaryOperation(lhs.getSourceInfo(),
JsBinaryOperator.ASG, lhs, rhs);
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/JsoDevirtualizer.java
Wed Jul 6 13:03:16 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/JsoDevirtualizer.java
Thu Jul 14 13:39:15 2011
@@ -150,6 +150,12 @@
*/
private final JMethod isJavaObjectMethod;
+ /**
+ * Key is the method signature, value is the number of unique instances
with
+ * the same signature.
+ */
+ private Map<String, Integer> jsoMethodInstances = new HashMap<String,
Integer>();
+
/**
* Contains the set of devirtualizing methods that replace polymorphic
calls
* to Object methods.
@@ -224,10 +230,21 @@
SourceInfo sourceInfo =
jsoType.getSourceInfo().makeChild(SourceOrigin.UNKNOWN);
// Create the new method.
- String name = polyMethod.getName() + "__devirtual$";
+ String prefix;
+ Integer methodCount;
+ methodCount = jsoMethodInstances.get(polyMethod.getSignature());
+ if (methodCount == null) {
+ prefix = polyMethod.getName();
+ methodCount = 0;
+ } else {
+ prefix = polyMethod.getName() + methodCount;
+ methodCount++;
+ }
+ jsoMethodInstances.put(polyMethod.getSignature(), methodCount);
+ String devirtualName = prefix + "__devirtual$";
JMethod newMethod =
- program.createMethod(sourceInfo, name, jsoType,
polyMethod.getType(), false, true, true,
- AccessModifier.PUBLIC, false);
+ program.createMethod(sourceInfo, devirtualName, jsoType,
polyMethod.getType(), false, true,
+ true, AccessModifier.PUBLIC, false);
newMethod.setSynthetic();
// Setup parameters.
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
