Small nits and new tests for devirtualization of Boolean, Double and String. Change-Id: I1923cd4aecc057ce2fe7e213b146b8d0171fe1a5
diff --git a/dev/core/src/com/google/gwt/dev/javac/JdtUtil.java b/dev/core/src/com/google/gwt/dev/javac/JdtUtil.java index 8c46eac..e1513b2 100644 --- a/dev/core/src/com/google/gwt/dev/javac/JdtUtil.java +++ b/dev/core/src/com/google/gwt/dev/javac/JdtUtil.java
@@ -51,7 +51,7 @@ * Utility functions to interact with JDT classes. */ public final class JdtUtil { - public static final String JSO_CLASS = "com/google/gwt/core/client/JavaScriptObject"; + private static final String JSO_CLASS = "com/google/gwt/core/client/JavaScriptObject"; /** * Returns a source name from an array of names.
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/Devirtualizer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/Devirtualizer.java index 9413c65..d28d70c 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/Devirtualizer.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/Devirtualizer.java
@@ -33,7 +33,6 @@ import com.google.gwt.dev.jjs.ast.JProgram.DispatchType; import com.google.gwt.dev.jjs.ast.JReferenceType; import com.google.gwt.dev.jjs.ast.JReturnStatement; -import com.google.gwt.dev.jjs.ast.JType; import com.google.gwt.dev.jjs.ast.JTypeOracle; import com.google.gwt.dev.jjs.ast.JVariableRef; import com.google.gwt.dev.jjs.ast.RuntimeConstants; @@ -215,15 +214,6 @@ } /** - * Returns true if getClass() is devirtualized for {@code type}; used in - * {@link ReplaceGetClassOverrides} to avoid replacing getClass() methods that need - * trampolines. - */ - public static boolean isGetClassDevirtualized(JProgram program, JType type) { - return type == program.getJavaScriptObject() || type == program.getTypeJavaLangString(); - } - - /** * Maps each Object instance methods (ie, {@link Object#equals(Object)}) onto * its corresponding devirtualizing method. */
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java index 66fb82f..54cf529 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
@@ -3990,8 +3990,7 @@ private boolean isSyntheticGetClassNeeded(TypeDeclaration typeDeclaration, JDeclaredType type) { // TODO(rluble): We should check whether getClass is implemented by type and only // instead of blacklisting. - return type != javaLangObject && type != javaLangString && type.getSuperClass() != null && - !JdtUtil.isJsoSubclass(typeDeclaration.binding); + return type.getSuperClass() != null && !JdtUtil.isJsoSubclass(typeDeclaration.binding); } private void createMethod(AbstractMethodDeclaration x) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceGetClassOverrides.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceGetClassOverrides.java index 1d04fdc..a789967 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceGetClassOverrides.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceGetClassOverrides.java
@@ -22,12 +22,14 @@ import com.google.gwt.dev.jjs.ast.JMethodCall; import com.google.gwt.dev.jjs.ast.JModVisitor; import com.google.gwt.dev.jjs.ast.JProgram; +import com.google.gwt.dev.jjs.ast.JType; import com.google.gwt.dev.jjs.ast.RuntimeConstants; /** - * Prune all overrides of Object.getClass() except when the enclosing class is JavaScriptObject - * (to permit getClass() devirtualization in Devirtualizer to continue to work). - * Also Inline all method calls to Object.getClass() as Object.clazz. + * Prune all overrides of Object.getClass() and inline all method calls to Object.getClass() + * as Object.___clazz. + * <p> + * The Devirtualizer pass needs to run before this pass. */ public class ReplaceGetClassOverrides { public static void exec(JProgram program) { @@ -48,27 +50,29 @@ @Override public void endVisit(JMethod x, Context ctx) { - // Don't prune getClass() for objects where it is devirtualized (String, JSOs for now) - if (Devirtualizer.isGetClassDevirtualized(program, x.getEnclosingType())) { - return; - } - if (x.getOverriddenMethods().contains(getClassMethod)) { + if (isGetClassMethod(x)) { ctx.removeMe(); } } @Override public void endVisit(JMethodCall x, Context ctx) { - // Don't inline getClass() for objects where it is devirtualized (String, JSOs for now) - if (Devirtualizer.isGetClassDevirtualized(program, x.getTarget().getEnclosingType())) { - return; - } - // replace overridden getClass() with reference to Object.clazz field - if (x.getTarget() == getClassMethod || - x.getTarget().getOverriddenMethods().contains(getClassMethod)) { + + // All calls to getClass with reference to Object.clazz field + if (isGetClassMethod(x.getTarget())) { + assert !isGetClassDevirtualized(x.getTarget().getEnclosingType()); ctx.replaceMe(new JFieldRef(x.getSourceInfo(), x.getInstance(), clazzField, clazzField.getEnclosingType())); } } + + private boolean isGetClassMethod(JMethod method) { + return method == getClassMethod || method.getOverriddenMethods().contains(getClassMethod); + } + + private boolean isGetClassDevirtualized(JType type) { + return type == program.getJavaScriptObject() + || program.getRepresentedAsNativeTypes().contains(type); + } } }
diff --git a/user/super/com/google/gwt/emul/java/lang/Object.java b/user/super/com/google/gwt/emul/java/lang/Object.java index 717f250..523beba 100644 --- a/user/super/com/google/gwt/emul/java/lang/Object.java +++ b/user/super/com/google/gwt/emul/java/lang/Object.java
@@ -29,7 +29,6 @@ /** * Holds class literal for subtypes of Object. */ - // BUG: If this field name conflicts with a method param name, JDT will complain // CHECKSTYLE_OFF private transient Class<?> ___clazz; // CHECKSTYLE_ON @@ -68,10 +67,7 @@ return this == other; } - /* - * magic; Actual assignment to this field is done by Class.createFor() methods by injecting it - * into the prototype. - */ + // TODO(rluble): declare as final public Class<? extends Object> getClass() { return ___clazz; } @@ -92,3 +88,5 @@ protected void finalize() throws Throwable { } } +// DO NOT REMOVE OR MODIFY THIS magic COMMENT. It is checked by the build system to ensure we +// are compiling GWT JRE. \ No newline at end of file
diff --git a/user/super/com/google/gwt/emul/java/lang/String.java b/user/super/com/google/gwt/emul/java/lang/String.java index e3fad9e..e03cfbf 100644 --- a/user/super/com/google/gwt/emul/java/lang/String.java +++ b/user/super/com/google/gwt/emul/java/lang/String.java
@@ -433,25 +433,6 @@ } } - /** - * Magic; JSODevirtualizer will use this implementation.<p> - * - * Each class gets a synthetic stubs for getClass at AST construction time with the exception of - * Object, JavaScriptObject and subclasses and String; see {@link GwtAstBuilder.createMembers()}. - * <p> - * - * These stubs are replaced in {@link ReplaceGetClassOverrides} by an access to field __clazz - * which is initialized in each class prototype to point to the class literal. String is - * implemented as a plain JavaScript string hence lacking said field.<p> - * - * The devirtualizer {@code JsoDevirtualizer} will insert a trampoline that uses this - * implementation. - */ - @Override - public Class<? extends Object> getClass() { - return String.class; - } - @Override public int hashCode() { return HashCodes.hashCodeForString(this);
diff --git a/user/test/com/google/gwt/dev/jjs/CompilerSuite.java b/user/test/com/google/gwt/dev/jjs/CompilerSuite.java index 1a2e150..efebaf6 100644 --- a/user/test/com/google/gwt/dev/jjs/CompilerSuite.java +++ b/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
@@ -48,6 +48,7 @@ import com.google.gwt.dev.jjs.test.MethodCallTest; import com.google.gwt.dev.jjs.test.MethodInterfaceTest; import com.google.gwt.dev.jjs.test.MiscellaneousTest; +import com.google.gwt.dev.jjs.test.NativeDevirtualizationTest; import com.google.gwt.dev.jjs.test.NativeLongTest; import com.google.gwt.dev.jjs.test.ObjectIdentityTest; import com.google.gwt.dev.jjs.test.SingleJsoImplTest; @@ -101,6 +102,7 @@ suite.addTestSuite(MethodCallTest.class); suite.addTestSuite(MethodInterfaceTest.class); suite.addTestSuite(MiscellaneousTest.class); + suite.addTestSuite(NativeDevirtualizationTest.class); suite.addTestSuite(NativeLongTest.class); suite.addTestSuite(ObjectIdentityTest.class); suite.addTestSuite(ScriptOnlyTest.class);
diff --git a/user/test/com/google/gwt/dev/jjs/test/MiscellaneousTest.java b/user/test/com/google/gwt/dev/jjs/test/MiscellaneousTest.java index bb81019..5835a14 100644 --- a/user/test/com/google/gwt/dev/jjs/test/MiscellaneousTest.java +++ b/user/test/com/google/gwt/dev/jjs/test/MiscellaneousTest.java
@@ -16,7 +16,6 @@ package com.google.gwt.dev.jjs.test; import com.google.gwt.core.client.JavaScriptException; -import com.google.gwt.core.client.JavaScriptObject; import com.google.gwt.junit.client.GWTTestCase; /** @@ -295,72 +294,6 @@ } } - public void testString() { - String x = "hi"; - assertEquals("hi", x); - assertEquals("hi", x.toString()); - x = new String(); - assertEquals("", x); - x = new String(x); - assertEquals("", x); - x = new String("hi"); - assertEquals("hi", x); - assertEquals('i', x.charAt(1)); - assertEquals("hiyay", x.concat("yay")); - assertEquals("hihi", x + x); - - assertEquals( - "blahcom.google.gwt.dev.jjs.test.MiscellaneousTestabctruefalsenullc27", - ("blah" + this + String.valueOf(new char[] {'a', 'b', 'c'}) + true - + false + null + 'c' + 27)); - } - - /** - * Ensures that polymorphic dispatch to String works correctly. - */ - @SuppressWarnings("unchecked") - public void testStringDynamicMethods() { - Object s = FALSE ? new Object() : "Hello, World!"; - assertEquals(String.class, s.getClass()); - assertEquals("Hello, World!".hashCode(), s.hashCode()); - assertTrue(s.equals("Hello, World!")); - assertTrue("Hello, World!".equals(s)); - assertFalse(s.equals("")); - assertFalse("".equals(s)); - assertEquals("Hello, World!", s.toString()); - assertTrue(s instanceof String); - - Comparable b = FALSE ? new Integer(1) : "Hello, World!"; - assertTrue(((Comparable) "Hello, World!").compareTo(b) == 0); - assertTrue(b.compareTo("Hello, World!") == 0); - assertTrue(((Comparable) "A").compareTo(b) < 0); - assertTrue(b.compareTo("A") > 0); - assertTrue(((Comparable) "Z").compareTo(b) > 0); - assertTrue(b.compareTo("Z") < 0); - assertTrue(b instanceof String); - - CharSequence c = FALSE ? new StringBuilder() : "Hello, World!"; - assertEquals('e', c.charAt(1)); - assertEquals(13, c.length()); - assertEquals("ello", c.subSequence(1, 5)); - assertTrue(c instanceof String); - } - - /** - * Ensures that dispatch to JavaScript native arrays that are NOT Java arrays works properly. - */ - public void testNativeJavaScriptArray() { - Object jsoArray = FALSE ? new Object() : JavaScriptObject.createArray(); - assertEquals(JavaScriptObject.class, jsoArray.getClass()); - assertFalse(jsoArray instanceof Object[]); - - Object objectArray = FALSE ? new Object() : new Object[10]; - assertEquals(Object[].class, objectArray.getClass()); - assertTrue(objectArray instanceof Object[]); - - assertFalse(jsoArray.toString().equals(objectArray.toString())); - } - @Override public String toString() { return "com.google.gwt.dev.jjs.test.MiscellaneousTest";
diff --git a/user/test/com/google/gwt/dev/jjs/test/NativeDevirtualizationTest.java b/user/test/com/google/gwt/dev/jjs/test/NativeDevirtualizationTest.java new file mode 100644 index 0000000..967cf62 --- /dev/null +++ b/user/test/com/google/gwt/dev/jjs/test/NativeDevirtualizationTest.java
@@ -0,0 +1,147 @@ +/* + * Copyright 2015 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; + +import com.google.gwt.core.client.JavaScriptObject; +import com.google.gwt.junit.DoNotRunWith; +import com.google.gwt.junit.Platform; +import com.google.gwt.junit.client.GWTTestCase; + +import javaemul.internal.HashCodes; + +/** + * This should probably be refactored at some point. + */ +public class NativeDevirtualizationTest extends GWTTestCase { + + public static final String HELLO_WORLD = "Hello, World!"; + + @Override + public String getModuleName() { + return "com.google.gwt.dev.jjs.CompilerSuite"; + } + + public void testStringDevirtualization() { + String x = "hi"; + assertEquals("hi", x); + assertEquals("hi", x.toString()); + x = new String(); + assertEquals("", x); + x = new String(x); + assertEquals("", x); + x = new String("hi"); + assertEquals("hi", x); + assertEquals('i', x.charAt(1)); + assertEquals("hiyay", x.concat("yay")); + assertEquals("hihi", x + x); + + assertEquals( + "blahabctruefalsenullc27", + ("blah" + String.valueOf(new char[] {'a', 'b', 'c'}) + true + + false + null + 'c' + 27)); + Object s = HELLO_WORLD; + assertEquals(String.class, s.getClass()); + assertEquals(HELLO_WORLD, s.toString()); + assertEquals(HashCodes.hashCodeForString(HELLO_WORLD), s.hashCode()); + + assertTrue(s.equals(HELLO_WORLD)); + assertTrue(HELLO_WORLD.equals(s)); + assertFalse(s.equals("")); + assertFalse("".equals(s)); + assertEquals(HELLO_WORLD, s.toString()); + assertTrue(s instanceof String); + + Comparable b = HELLO_WORLD; + assertEquals(String.class, b.getClass()); + assertEquals(HELLO_WORLD, b.toString()); + assertEquals(HashCodes.hashCodeForString(HELLO_WORLD), b.hashCode()); + + assertTrue(((Comparable) HELLO_WORLD).compareTo(b) == 0); + assertTrue(b.compareTo(HELLO_WORLD) == 0); + assertTrue(((Comparable) "A").compareTo(b) < 0); + assertTrue(b.compareTo("A") > 0); + assertTrue(((Comparable) "Z").compareTo(b) > 0); + assertTrue(b.compareTo("Z") < 0); + assertTrue(b instanceof String); + + CharSequence c = HELLO_WORLD; + assertEquals(String.class, c.getClass()); + assertEquals(HELLO_WORLD, c.toString()); + assertEquals(HashCodes.hashCodeForString(HELLO_WORLD), c.hashCode()); + + assertEquals('e', c.charAt(1)); + assertEquals(13, c.length()); + assertEquals("ello", c.subSequence(1, 5)); + assertTrue(c instanceof String); + } + + public void testBooleanDevirtualization() { + final Boolean FALSE = false; + Object o = FALSE; + assertEquals(Boolean.class, o.getClass()); + assertEquals("false", o.toString()); + + assertFalse((Boolean) o); + assertTrue(o instanceof Boolean); + assertFalse(o instanceof Number); + assertTrue(o instanceof Comparable); + + Comparable b = FALSE; + assertEquals(Boolean.class, b.getClass()); + assertEquals("false", b.toString()); + assertEquals(FALSE.hashCode(), b.hashCode()); + } + + public void testDoubleDevirtualization() { + final Double ONE_POINT_ONE = 1.1d; + Object o = ONE_POINT_ONE; + assertEquals(Double.class, o.getClass()); + assertEquals("1.1", o.toString()); + + assertEquals(1.1d, o); + assertTrue(o instanceof Double); + assertTrue(o instanceof Number); + assertTrue(o instanceof Comparable); + + Comparable b = ONE_POINT_ONE; + assertEquals(Double.class, b.getClass()); + assertEquals("1.1", b.toString()); + assertEquals(ONE_POINT_ONE.hashCode(), b.hashCode()); + assertTrue(b.compareTo((Double) 1.2d) < 0); + + Number c = ONE_POINT_ONE; + assertEquals(Double.class, c.getClass()); + assertEquals("1.1", c.toString()); + assertEquals(ONE_POINT_ONE.hashCode(), c.hashCode()); + assertEquals(1, c.intValue()); + } + + /** + * Ensures that dispatch to JavaScript native arrays that are NOT Java arrays works properly. + */ + @DoNotRunWith(Platform.Devel) + public void testNativeJavaScriptArray() { + Object jsoArray = JavaScriptObject.createArray(10); + assertEquals(JavaScriptObject.class, jsoArray.getClass()); + assertTrue(jsoArray instanceof JavaScriptObject); + + Object objectArray = new Object[10]; + assertEquals(Object[].class, objectArray.getClass()); + assertTrue(objectArray instanceof Object[]); + + assertFalse(jsoArray.toString().equals(objectArray.toString())); + } +}