Make sure native JsConstructors are not prunned even if not used.
JsConstructors for native types provide support for cast and instanceof.
This patch makes native JsTypes and their constructors live if they are
referenced at all (but should not have any effect on codesize).
Bug: #9346
Bug-Link: http://github.com/gwtproject/gwt/issues/9346
Change-Id: I6c1621e41705a0ff84a9fefe8c02d77440ca1936
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 2c44b59..e4cf46a 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
@@ -30,7 +30,6 @@
import com.google.gwt.dev.jjs.ast.JExpression;
import com.google.gwt.dev.jjs.ast.JField;
import com.google.gwt.dev.jjs.ast.JFieldRef;
-import com.google.gwt.dev.jjs.ast.JInstanceOf;
import com.google.gwt.dev.jjs.ast.JInterfaceType;
import com.google.gwt.dev.jjs.ast.JLocal;
import com.google.gwt.dev.jjs.ast.JLocalRef;
@@ -202,9 +201,6 @@
// Rescue any JavaScriptObject type that is the target of a cast.
JType targetType = x.getCastType();
- // Casts to native classes use the native constructor qualified name.
- maybeRescueNativeConstructor(targetType);
-
if (!canBeInstantiatedInJavaScript(targetType)) {
return true;
}
@@ -314,13 +310,6 @@
}
@Override
- public boolean visit(JInstanceOf expression, Context ctx) {
- // Instanceof checks for native classes use the native constructor qualified name.
- maybeRescueNativeConstructor(expression.getTestType());
- return true;
- }
-
- @Override
public boolean visit(JInterfaceType type, Context ctx) {
boolean isReferenced = referencedTypes.contains(type);
boolean isInstantiated = instantiatedTypes.contains(type);
@@ -705,7 +694,8 @@
JDeclaredType declaredType = (JDeclaredType) type;
for (JMethod method : declaredType.getMethods()) {
- if (method.canBeReferencedExternally()) {
+ if (method.canBeReferencedExternally()
+ || declaredType.isJsNative() && method.isJsConstructor()) {
rescue(method);
}
}
@@ -769,13 +759,6 @@
}
}
- private void maybeRescueNativeConstructor(JType type) {
- JConstructor jsConstructor = JjsUtils.getJsNativeConstructorOrNull(type);
- if (jsConstructor != null) {
- rescue(jsConstructor);
- }
- }
-
/**
* The code is very tightly tied to the behavior of
* Pruner.CleanupRefsVisitor. CleanUpRefsVisitor will prune unread
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java b/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
index 6945f5f..702cae1 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
@@ -178,17 +178,9 @@
x.setType(translate(x.getType().getUnderlyingType()));
}
- private void maybeFlowIntoNativeConstructor(JType type) {
- JConstructor jsConstructor = JjsUtils.getJsNativeConstructorOrNull(type);
- if (jsConstructor != null) {
- flowInto(jsConstructor);
- }
- }
-
@Override
public void endVisit(JCastOperation x, Context ctx) {
x.resolve(translate(x.getCastType()));
- maybeFlowIntoNativeConstructor(x.getCastType());
}
@Override
@@ -273,7 +265,6 @@
@Override
public void endVisit(JInstanceOf x, Context ctx) {
x.resolve(translate(x.getTestType()));
- maybeFlowIntoNativeConstructor(x.getTestType());
}
@Override
@@ -1032,22 +1023,24 @@
}
}
- for (JDeclaredType t : types) {
+ for (JDeclaredType type : types) {
/*
* Eagerly instantiate any type that requires devirtualization, i.e. String and
* JavaScriptObject subtypes. That way we don't have to copy the exact semantics of
* ControlFlowAnalyzer.
*/
- if (requiresDevirtualization(t)) {
- instantiate(t);
+ if (requiresDevirtualization(type)) {
+ instantiate(type);
}
/*
* We also flow into the types with JsInterop entry point because our first pass on root types
- * with JsInterop entry points are missing these inner classes.
+ * with JsInterop entry points are missing these inner classes. For native types this ensures
+ * that the constructor is considered reachable as it might be needed later for instanceof
+ * and casts.
*/
- if (t.hasJsInteropEntryPoints()) {
- fullFlowIntoType(t);
+ if (type.hasJsInteropEntryPoints() || type.isJsNative()) {
+ fullFlowIntoType(type);
}
}
}
diff --git a/dev/core/test/com/google/gwt/dev/CompilerTest.java b/dev/core/test/com/google/gwt/dev/CompilerTest.java
index 2b27fb9..fd0f23c 100644
--- a/dev/core/test/com/google/gwt/dev/CompilerTest.java
+++ b/dev/core/test/com/google/gwt/dev/CompilerTest.java
@@ -42,6 +42,7 @@
import java.io.File;
import java.io.IOException;
+import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -1510,18 +1511,8 @@
compileToJs(relinkApplicationDir, "com.foo.SimpleModule",
Lists.<MockResource> newArrayList(emptyEntryPointResource), relinkMinimalRebuildCache, null,
JsOutputOption.OBFUSCATED);
- // Since J2CL requires a @JsMethod in Number.java, String.java, etc. and since JsInterop types
- // are fully traversed (so that correct exports can be regenerated) Number is fully traversed
- // and so shows up in the processed list.
- Set<String> staleTypeNames =
- new HashSet<>(relinkMinimalRebuildCache.getProcessedStaleTypeNames());
- staleTypeNames.remove("java.lang.Boolean");
- staleTypeNames.remove("java.lang.Double");
- staleTypeNames.remove("java.lang.Number");
- staleTypeNames.remove("java.lang.String");
- staleTypeNames.remove("java.lang.Throwable");
// Show that only this little change is stale, not the whole world.
- assertEquals(2, staleTypeNames.size());
+ assertEquals(2, getStaleTypeNames(relinkMinimalRebuildCache).size());
}
public void testIncrementalRecompile_bridgeMethodOverrideChain()
@@ -2544,21 +2535,29 @@
}
if (expectedProcessedStaleTypeNames != null) {
- Set<String> staleTypeNames =
- new HashSet<>(minimalRebuildCache.getProcessedStaleTypeNames());
- // Since J2CL requires a @JsMethod in Number.java, String.java, etc. and since JsInterop
- // types are fully traversed (so that correct exports can be regenerated) Number is fully
- // traversed and so shows up in the processed list.
- staleTypeNames.remove("java.lang.Boolean");
- staleTypeNames.remove("java.lang.Double");
- staleTypeNames.remove("java.lang.Number");
- staleTypeNames.remove("java.lang.String");
- staleTypeNames.remove("java.lang.Throwable");
- assertEquals(expectedProcessedStaleTypeNames, staleTypeNames);
+ assertEquals(expectedProcessedStaleTypeNames, getStaleTypeNames(minimalRebuildCache));
}
return Files.toString(outputJsFile, Charsets.UTF_8);
}
+ private Set<String> getStaleTypeNames(MinimalRebuildCache relinkMinimalRebuildCache) {
+ Set<String> staleTypeNames =
+ new HashSet<>(relinkMinimalRebuildCache.getProcessedStaleTypeNames());
+ // List of JRE types that provide JsInterop entry points and jre native JsTypes. These are
+ // always traversed fully and polute the tests, so they will be removed from stale type
+ // comparisons.
+ staleTypeNames.removeAll(Arrays.asList(
+ "java.lang.Boolean",
+ "java.lang.Double",
+ "java.lang.Number",
+ "java.lang.String",
+ "java.lang.String$NativeFunction",
+ "java.lang.String$NativeString",
+ "java.lang.Throwable",
+ "javaemul.internal.NativeRegExp"));
+ return staleTypeNames;
+ }
+
private String getEntryMethodHolderTypeName(String typeName) {
return "com.google.gwt.lang." +
EntryMethodHolderGenerator.getEntryMethodHolderTypeName(typeName);
