Fix method call tightening involving native JsTypes.
Makes sure that polymorphic methods on native types are
always considered polymorphic dispatch.
This patch removed the synthetic getClass in JsTypes and
makes sure that object methods on native types are properly
devirtualized.
Change-Id: I42508fd881761d49ce71201db688dc38bd744c62
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JClassType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JClassType.java
index 3aa02d0..b7b01d7 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JClassType.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JClassType.java
@@ -66,7 +66,9 @@
@Override
public final JMethod getInitMethod() {
- assert getMethods().size() > 1;
+ if (getMethods().size() <= GwtAstBuilder.INIT_METHOD_INDEX) {
+ return null;
+ }
JMethod init = this.getMethods().get(GwtAstBuilder.INIT_METHOD_INDEX);
if (!init.getName().equals(GwtAstBuilder.INIT_NAME_METHOD_NAME)) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JReferenceType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JReferenceType.java
index 886c80d..57fc036 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JReferenceType.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JReferenceType.java
@@ -120,10 +120,26 @@
private transient AnalysisDecoratedTypePool analysisDecoratedTypePool = null;
enum AnalysisResult {
- NULLABLE_NOT_EXACT,
- NOT_NULLABLE_NOT_EXACT,
- NULLABLE_EXACT,
- NOT_NULLABLE_EXACT;
+ NULLABLE_NOT_EXACT(true, false),
+ NOT_NULLABLE_NOT_EXACT(false, false),
+ NULLABLE_EXACT(true, true),
+ NOT_NULLABLE_EXACT(false, true);
+
+ private final boolean isNullable;
+ private final boolean isExact;
+
+ AnalysisResult(boolean isNullable, boolean isExact) {
+ this.isNullable = isNullable;
+ this.isExact = isExact;
+ }
+
+ private boolean isNullable() {
+ return isNullable;
+ }
+
+ private boolean isExact() {
+ return isExact;
+ }
}
/**
* A reference type decorated with the result of static analysis. Only two analysis properties
@@ -278,16 +294,15 @@
@Override
public final boolean canBeNull() {
- return getAnalysisResult() == AnalysisResult.NULLABLE_EXACT ||
- getAnalysisResult() == AnalysisResult.NULLABLE_NOT_EXACT;
+ return getAnalysisResult().isNullable();
}
@Override
public final boolean canBeSubclass() {
- boolean canBeSubclass = getAnalysisResult() == AnalysisResult.NULLABLE_NOT_EXACT ||
- getAnalysisResult() == AnalysisResult.NOT_NULLABLE_NOT_EXACT;
- assert canBeSubclass || !isJsoType() : "A JSO type can never be EXACT but " + name + " is.";
- return canBeSubclass;
+ boolean exact = getAnalysisResult().isExact();
+ assert !exact || canBeStrengthenedToExactType() :
+ "A JSO or native type can never be EXACT but " + name + " is.";
+ return !exact;
}
@Override
@@ -343,10 +358,21 @@
throw new AssertionError("Unknown AnalysisResult " + getAnalysisResult().toString());
}
+ private boolean canBeStrengthenedToExactType() {
+ return !isJsoType() && !canBeImplementedExternally();
+ }
+
+ private boolean canBeStrengthenedToNonNull() {
+ // JSOs can not be strengthened because there is code that assumes that null is a JSO, and
+ // instance methods calls never throw NPE on null.
+ // Some methods like JavaScriptObject.cast() will confuse the compiler, due to modeling the
+ // return type as non-null.
+ return !isJsoType();
+ }
+
@Override
public JReferenceType strengthenToNonNull() {
- if (isJsoType()) {
- // JSOs can not be strengthened.
+ if (!canBeStrengthenedToNonNull()) {
return this;
}
switch (getAnalysisResult()) {
@@ -364,8 +390,7 @@
}
public JReferenceType strengthenToExact() {
- if (isJsoType()) {
- // JSOs can not be strengthened.
+ if (!canBeStrengthenedToExactType()) {
return this;
}
switch (getAnalysisResult()) {
@@ -404,7 +429,7 @@
}
AnalysisResult getAnalysisResult() {
- if (isFinal() && !isJsoType()) {
+ if (isFinal() && canBeStrengthenedToExactType()) {
return AnalysisResult.NULLABLE_EXACT;
}
return AnalysisResult.NULLABLE_NOT_EXACT;
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ArrayNormalizer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ArrayNormalizer.java
index 687a434..226bfb8 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ArrayNormalizer.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ArrayNormalizer.java
@@ -221,6 +221,7 @@
// There is no need to check as the static check already proved the cast is correct.
return null;
}
+ // TODO(rluble): native[] setCheck can also be omitted.
return arrayRef;
}
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 b3ec75d..dd5e9ec 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
@@ -262,8 +262,6 @@
public static final int CLINIT_METHOD_INDEX = 0;
public static final int INIT_METHOD_INDEX = 1;
public static final int GET_CLASS_METHOD_INDEX = 2;
- public static final int VALUE_OF_METHOD_INDEX = 3;
- public static final int VALUES_METHOD_INDEX = 4;
/**
* Visit the JDT AST and produce our own AST. By the end of this pass, the
@@ -2907,7 +2905,7 @@
JMethod method = type.getMethods().get(GET_CLASS_METHOD_INDEX);
assert (GET_CLASS_METHOD_NAME.equals(method.getName()));
SourceInfo info = method.getSourceInfo();
- if (type.isJsoType() || type.isJsNative()) {
+ if (type.isJsoType()) {
// Native types and JSOs get a synthetic get class that return JavaScriptObject.class.
//
// return Cast.getClass(this)
@@ -3055,8 +3053,10 @@
private void processEnumType(JEnumType type) {
// $clinit, $init, getClass, valueOf, values
- JMethod valueOfMethod = type.getMethods().get(VALUE_OF_METHOD_INDEX);
- JMethod valuesMethod = type.getMethods().get(VALUES_METHOD_INDEX);
+ JMethod valueOfMethod =
+ type.getMethods().get(getEnumMethodsStartIndex(type) + VALUE_OF_METHOD_OFFSET);
+ JMethod valuesMethod =
+ type.getMethods().get(getEnumMethodsStartIndex(type) + VALUES_METHOD_OFFSET);
{
assert VALUE_OF_METHOD_NAME.equals(valueOfMethod.getName());
writeEnumValueOfMethod(type, valueOfMethod, valuesMethod);
@@ -3932,7 +3932,8 @@
if (type instanceof JEnumType) {
{
- assert type.getMethods().size() == VALUE_OF_METHOD_INDEX;
+ assert type.getMethods().size()
+ == getEnumMethodsStartIndex(type) + VALUE_OF_METHOD_OFFSET;
MethodBinding valueOfBinding =
binding.getExactMethod(VALUE_OF_,
new TypeBinding[]{x.scope.getJavaLangString()}, curCud.scope);
@@ -3940,7 +3941,9 @@
createMethodFromBinding(info, valueOfBinding, new String[] {"name"});
}
{
- assert type.getMethods().size() == VALUES_METHOD_INDEX;
+ assert type.getMethods().size()
+ == getEnumMethodsStartIndex(type) + VALUES_METHOD_OFFSET;
+
MethodBinding valuesBinding = binding.getExactMethod(VALUES_, NO_TYPES, curCud.scope);
assert valuesBinding != null;
createMethodFromBinding(info, valuesBinding, null);
@@ -3969,10 +3972,23 @@
}
}
- private boolean isSyntheticGetClassNeeded(TypeDeclaration typeDeclaration, JDeclaredType type) {
+ private static boolean isSyntheticGetClassNeeded(
+ TypeDeclaration typeDeclaration, JDeclaredType type) {
// TODO(rluble): We should check whether getClass is implemented by type and only
// instead of blacklisting.
- return type.getSuperClass() != null && !JdtUtil.isJsoSubclass(typeDeclaration.binding);
+ return type.getSuperClass() != null && !JdtUtil.isJsoSubclass(typeDeclaration.binding)
+ && !type.isJsNative();
+ }
+
+ private static final int VALUE_OF_METHOD_OFFSET = 0;
+ private static final int VALUES_METHOD_OFFSET = 1;
+
+ private static int getEnumMethodsStartIndex(JType type) {
+ assert type instanceof JEnumType;
+ if (type.isJsNative()) {
+ return GET_CLASS_METHOD_INDEX;
+ }
+ return GET_CLASS_METHOD_INDEX + 1;
}
private void createMethod(AbstractMethodDeclaration x) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java b/dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java
index 96c9de4..c5e8220 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java
@@ -125,9 +125,7 @@
JMethod mostSpecificOverride =
program.typeOracle.findMostSpecificOverride(underlyingType, original);
- if (mostSpecificOverride == original || mostSpecificOverride.isJsNative()) {
- // Do not tighten if the target method is a native JsMethod; this is necessary for the
- // JSO trampoline to be invoked on getClass().
+ if (mostSpecificOverride == original) {
return methodCall;
}
JMethodCall newCall = new JMethodCall(
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
index 8c54cea..6227e40 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
@@ -437,8 +437,7 @@
// If possible, try to use a narrower cast
JReferenceType tighterType = getSingleConcreteType(toType);
if (tighterType != null && tighterType != toType) {
- JCastOperation newOp = new JCastOperation(x.getSourceInfo(), tighterType, x.getExpr());
- ctx.replaceMe(newOp);
+ ctx.replaceMe(new JCastOperation(x.getSourceInfo(), tighterType, x.getExpr()));
}
}
diff --git a/user/test/com/google/gwt/core/interop/NativeJsTypeTest.java b/user/test/com/google/gwt/core/interop/NativeJsTypeTest.java
index b96a6eb..26b4110 100644
--- a/user/test/com/google/gwt/core/interop/NativeJsTypeTest.java
+++ b/user/test/com/google/gwt/core/interop/NativeJsTypeTest.java
@@ -51,6 +51,18 @@
interface MyNativeJsTypeInterface {
}
+ @JsType(isNative = true, namespace = JsPackage.GLOBAL, name = "Object")
+ static class NativeObject implements MyNativeJsTypeInterface {
+ }
+
+ @JsType(isNative = true, namespace = JsPackage.GLOBAL, name = "Object")
+ final static class FinalNativeObject implements MyNativeJsTypeInterface {
+ }
+
+ @JsType(isNative = true)
+ interface MyNativeJsTypeInterfaceOnlyOneConcreteImplementor {
+ }
+
public void testClassLiterals() {
assertEquals(JavaScriptObject.class, MyNativeJsType.class);
assertEquals(JavaScriptObject.class, MyNativeJsTypeInterface.class);
@@ -58,10 +70,45 @@
assertEquals(JavaScriptObject.class, MyNativeJsTypeInterface[].class);
assertEquals(JavaScriptObject.class, MyNativeJsType[][].class);
assertEquals(JavaScriptObject.class, MyNativeJsTypeInterface[][].class);
+ }
- Object nativeObject = createNativeObjectWithoutToString();
- assertEquals(JavaScriptObject.class, nativeObject.getClass());
- assertEquals(JavaScriptObject.class, ((MyNativeJsTypeInterface) nativeObject).getClass());
+ public void testGetClass() {
+ Object object = createNativeObjectWithoutToString();
+ assertEquals(JavaScriptObject.class, object.getClass());
+
+ MyNativeJsTypeInterface nativeInterface =
+ (MyNativeJsTypeInterface) createNativeObjectWithoutToString();
+ assertEquals(JavaScriptObject.class, nativeInterface.getClass());
+
+ // Test that the dispatch to getClass in not messed up by incorrectly marking nativeObject1 as
+ // exact and inlining Object.getClass() implementation.
+ NativeObject nativeObject1 = new NativeObject();
+ assertEquals(JavaScriptObject.class, nativeObject1.getClass());
+
+ // Test that the dispatch to getClass in not messed up by incorrectly marking nativeObject2 as
+ // exact and inlining Object.getClass() implementation.
+ FinalNativeObject nativeObject2 = createNativeObject();
+ assertEquals(JavaScriptObject.class, nativeObject2.getClass());
+ }
+
+ @JsType(isNative = true, namespace = JsPackage.GLOBAL, name = "Object")
+ final static class AnotherFinalNativeObject implements MyNativeJsTypeInterface {
+ }
+
+ private static boolean same(Object thisObject, Object thatObject) {
+ return thisObject == thatObject;
+ }
+
+ public void testEqualityOptimization() {
+ // Makes sure that == does not get optimized away due to static class incompatibility.
+
+ FinalNativeObject finalNativeObject = new FinalNativeObject();
+
+ AnotherFinalNativeObject anotherFinalNativeObject =
+ (AnotherFinalNativeObject) (Object) finalNativeObject;
+ // DeadCodeElimination could optimize statically to false due to type incompatibility, which
+ // could happen if both variables were marked as exact.
+ assertTrue(same(anotherFinalNativeObject, finalNativeObject));
}
public void testToString() {
@@ -75,6 +122,10 @@
assertEquals("", nativeArray.toString());
}
+ private static native FinalNativeObject createNativeObject() /*-{
+ return {};
+ }-*/;
+
private static native MyNativeJsType createNativeObjectWithToString() /*-{
return {toString: function() { return "Native type"; } };
}-*/;
diff --git a/user/test/com/google/gwt/dev/jjs/OptimizedOnlyCompilerSuite.java b/user/test/com/google/gwt/dev/jjs/OptimizedOnlyCompilerSuite.java
index f37d6e1..0a69664 100644
--- a/user/test/com/google/gwt/dev/jjs/OptimizedOnlyCompilerSuite.java
+++ b/user/test/com/google/gwt/dev/jjs/OptimizedOnlyCompilerSuite.java
@@ -18,6 +18,7 @@
import com.google.gwt.dev.jjs.optimized.ArrayListOptimizationTest;
import com.google.gwt.dev.jjs.optimized.ArrayStoreOptimizationTest;
import com.google.gwt.dev.jjs.optimized.CastOptimizationTest;
+import com.google.gwt.dev.jjs.optimized.JsOverlayMethodOptimizationTest;
import com.google.gwt.dev.jjs.optimized.SpecializationTest;
import com.google.gwt.dev.jjs.test.HasNoSideEffectsTest;
import com.google.gwt.dev.jjs.test.RunAsyncContentTest;
@@ -37,6 +38,7 @@
suite.addTestSuite(ArrayListOptimizationTest.class);
suite.addTestSuite(ArrayStoreOptimizationTest.class);
suite.addTestSuite(CastOptimizationTest.class);
+ suite.addTestSuite(JsOverlayMethodOptimizationTest.class);
suite.addTestSuite(SpecializationTest.class);
suite.addTestSuite(HasNoSideEffectsTest.class);
// RunAsyncContentTest relies in string interning for its assertions which is now always off
diff --git a/user/test/com/google/gwt/dev/jjs/optimized/JsOverlayMethodOptimizationTest.java b/user/test/com/google/gwt/dev/jjs/optimized/JsOverlayMethodOptimizationTest.java
new file mode 100644
index 0000000..c4f8acf
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/optimized/JsOverlayMethodOptimizationTest.java
@@ -0,0 +1,66 @@
+/*
+ * Copyright 2014 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.optimized;
+
+import com.google.gwt.junit.DoNotRunWith;
+import com.google.gwt.junit.Platform;
+
+import jsinterop.annotations.JsOverlay;
+import jsinterop.annotations.JsPackage;
+import jsinterop.annotations.JsType;
+
+/**
+ * Tests for JsOverlay methods are inlined by the Java optimization passes..
+ */
+@DoNotRunWith(Platform.Devel)
+public class JsOverlayMethodOptimizationTest extends OptimizationTestBase {
+
+ @JsType(isNative = true, namespace = JsPackage.GLOBAL, name = "Object")
+ private static class NativeType {
+ private native int nativeMethod();
+
+ @JsOverlay
+ final boolean contains(String s1, String s2) {
+ return s1.contains(s2);
+ }
+
+ @JsOverlay
+ final int alwaysCallNative() {
+ // NativeType.contains should get inlined by Java passes so the code below should be
+ // statically evaluated to return "this".
+ return contains("1234","1") ? this.nativeMethod() : 0;
+ }
+ }
+
+ public static int callAlwaysCallNative(NativeType obj) {
+ return obj.alwaysCallNative();
+ }
+
+ private static native String getGeneratedFunctionDefinition() /*-{
+ return function() {
+ @JsOverlayMethodOptimizationTest::callAlwaysCallNative(*)({});
+ }.toString();
+ }-*/;
+
+ /**
+ * Tests whether JsOverlay is inlined by the Java optimization passes rather than being just
+ * devirtualized.
+ */
+ public void testJsOverlayIsInlined() throws Exception {
+ String functionDef = getGeneratedFunctionDefinition();
+ assertFunctionMatches(functionDef, "({}.nativeMethod())");
+ }
+}
