Cleans up JsInterop related code in UnifyAst
This also fixes the problem where we don't always correctly
collect the root types (i.e. in case of non-class-wide JsExport).
This is not a complete cleanup as there are still unnecessary
checks in UnifyAst. I want to reduce the likelihood of behavior
change in this patch.
Change-Id: Ia8e9594e028df7977fd83e4e403dbbc8bcf63a35
Review-Link: https://gwt-review.googlesource.com/#/c/11651/
diff --git a/dev/core/src/com/google/gwt/dev/javac/CachedCompilationUnit.java b/dev/core/src/com/google/gwt/dev/javac/CachedCompilationUnit.java
index 2ea39a4..f9eecd3 100644
--- a/dev/core/src/com/google/gwt/dev/javac/CachedCompilationUnit.java
+++ b/dev/core/src/com/google/gwt/dev/javac/CachedCompilationUnit.java
@@ -45,6 +45,7 @@
private final boolean isError;
private final boolean isGenerated;
private final boolean isSuperSource;
+ private final boolean hasJsInteropRootType;
private transient List<JsniMethod> jsniMethods;
private final long lastModified;
private final MethodArgNamesLookup methodArgNamesLookup;
@@ -64,6 +65,7 @@
String resourceLocation) {
assert unit != null;
this.compiledClasses = CompiledClass.copyForUnit(unit.getCompiledClasses(), this);
+ this.hasJsInteropRootType = unit.hasJsInteropRootType;
this.contentId = unit.getContentId();
this.dependencies = unit.getDependencies();
this.resourcePath = unit.getResourcePath();
@@ -94,6 +96,7 @@
CachedCompilationUnit(CompilationUnit unit, long astToken) {
assert unit != null;
this.compiledClasses = CompiledClass.copyForUnit(unit.getCompiledClasses(), this);
+ this.hasJsInteropRootType = unit.hasJsInteropRootType();
this.contentId = unit.getContentId();
this.dependencies = unit.getDependencies();
this.resourcePath = unit.getResourcePath();
@@ -181,6 +184,11 @@
}
@Override
+ boolean hasJsInteropRootType() {
+ return hasJsInteropRootType;
+ }
+
+ @Override
ContentId getContentId() {
return contentId;
}
diff --git a/dev/core/src/com/google/gwt/dev/javac/CompilationState.java b/dev/core/src/com/google/gwt/dev/javac/CompilationState.java
index c3a1079..2979a4a 100644
--- a/dev/core/src/com/google/gwt/dev/javac/CompilationState.java
+++ b/dev/core/src/com/google/gwt/dev/javac/CompilationState.java
@@ -24,6 +24,9 @@
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
import com.google.gwt.thirdparty.guava.common.annotations.VisibleForTesting;
+import com.google.gwt.thirdparty.guava.common.base.Function;
+import com.google.gwt.thirdparty.guava.common.base.Predicates;
+import com.google.gwt.thirdparty.guava.common.collect.FluentIterable;
import java.util.Collection;
import java.util.Collections;
@@ -166,6 +169,18 @@
return exposedUnits;
}
+ public Iterable<String> getQualifiedJsInteropRootTypesNames() {
+ Function<CompilationUnit, String> toRootTypeName = new Function<CompilationUnit, String>() {
+ @Override
+ public String apply(CompilationUnit compilationUnit) {
+ return compilationUnit.hasJsInteropRootType() ? compilationUnit.getTypeName() : null;
+ }
+ };
+ return FluentIterable.from(unitMap.values())
+ .transform(toRootTypeName)
+ .filter(Predicates.notNull());
+ }
+
public CompilerContext getCompilerContext() {
return compilerContext;
}
diff --git a/dev/core/src/com/google/gwt/dev/javac/CompilationUnit.java b/dev/core/src/com/google/gwt/dev/javac/CompilationUnit.java
index a340edf..f9d2866 100644
--- a/dev/core/src/com/google/gwt/dev/javac/CompilationUnit.java
+++ b/dev/core/src/com/google/gwt/dev/javac/CompilationUnit.java
@@ -445,6 +445,12 @@
abstract CategorizedProblem[] getProblems();
+ /**
+ * Returns true if this compilation unit has any JsInterop root types (i.e. type with an export or
+ * a JsType).
+ */
+ abstract boolean hasJsInteropRootType();
+
private List<String> getJdtClassNames(String topLevelClass) {
List<String> classNames = new ArrayList<String>();
for (CompiledClass cc : getCompiledClasses()) {
diff --git a/dev/core/src/com/google/gwt/dev/javac/CompilationUnitImpl.java b/dev/core/src/com/google/gwt/dev/javac/CompilationUnitImpl.java
index 43c8102..18cf8d3 100644
--- a/dev/core/src/com/google/gwt/dev/javac/CompilationUnitImpl.java
+++ b/dev/core/src/com/google/gwt/dev/javac/CompilationUnitImpl.java
@@ -18,6 +18,8 @@
import com.google.gwt.dev.jjs.ast.JDeclaredType;
import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.util.collect.Lists;
+import com.google.gwt.thirdparty.guava.common.base.Predicate;
+import com.google.gwt.thirdparty.guava.common.collect.Iterables;
import org.eclipse.jdt.core.compiler.CategorizedProblem;
@@ -37,6 +39,7 @@
private final Dependencies dependencies;
private final List<CompiledClass> exposedCompiledClasses;
private final boolean hasErrors;
+ private final boolean hasJsInteropRootType;
private final List<JsniMethod> jsniMethods;
private final MethodArgNamesLookup methodArgs;
private final CategorizedProblem[] problems;
@@ -59,6 +62,11 @@
}
}
this.hasErrors = hasAnyErrors;
+ this.hasJsInteropRootType = Iterables.any(types, new Predicate<JDeclaredType>() {
+ @Override public boolean apply(JDeclaredType type) {
+ return type.hasAnyExports() || type.isOrExtendsJsType();
+ }
+ });
for (CompiledClass cc : compiledClasses) {
cc.initUnit(this);
}
@@ -100,6 +108,11 @@
}
@Override
+ boolean hasJsInteropRootType() {
+ return hasJsInteropRootType;
+ }
+
+ @Override
Dependencies getDependencies() {
return dependencies;
}
diff --git a/dev/core/src/com/google/gwt/dev/javac/CompiledClass.java b/dev/core/src/com/google/gwt/dev/javac/CompiledClass.java
index e0e7e25..e53bbde 100644
--- a/dev/core/src/com/google/gwt/dev/javac/CompiledClass.java
+++ b/dev/core/src/com/google/gwt/dev/javac/CompiledClass.java
@@ -44,7 +44,6 @@
if (in == null) {
return null;
}
- CompiledClass[] orig = new CompiledClass[in.size()];
List<CompiledClass> copy = new ArrayList<CompiledClass>();
Map<CompiledClass, CompiledClass> enclosingClassMap = new HashMap<CompiledClass, CompiledClass>();
diff --git a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
index 185ed0f..3e3f72c 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
@@ -148,6 +148,7 @@
import com.google.gwt.soyc.io.ArtifactsOutputDirectory;
import com.google.gwt.thirdparty.guava.common.annotations.VisibleForTesting;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableMap;
+import com.google.gwt.thirdparty.guava.common.collect.Iterables;
import com.google.gwt.thirdparty.guava.common.collect.Lists;
import com.google.gwt.thirdparty.guava.common.collect.Multimap;
import com.google.gwt.thirdparty.guava.common.collect.Sets;
@@ -158,7 +159,6 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.lang.annotation.Annotation;
import java.lang.management.ManagementFactory;
import java.util.ArrayList;
import java.util.Collection;
@@ -891,8 +891,6 @@
protected RebindPermutationOracle rpo;
protected String[] entryPointTypeNames;
- private static final String JS_EXPORT_ANN = "com.google.gwt.core.client.js.JsExport";
- private static final String JS_TYPE_ANN = "com.google.gwt.core.client.js.JsType";
public Precompiler(RebindPermutationOracle rpo, String[] entryPointTypeNames) {
this.rpo = rpo;
@@ -1036,7 +1034,7 @@
CompilationState compilationState = rpo.getCompilationState();
Memory.maybeDumpMemory("CompStateBuilt");
recordJsoTypes(compilationState.getTypeOracle());
- populateRootTypes(allRootTypes, additionalRootTypes, compilationState.getTypeOracle());
+ populateRootTypes(allRootTypes, additionalRootTypes, compilationState);
String entryMethodHolderTypeName =
buildEntryMethodHolder(rpo.getGeneratorContext(), allRootTypes);
beforeUnifyAst(allRootTypes);
@@ -1132,7 +1130,10 @@
}
private void populateRootTypes(Set<String> allRootTypes, String[] additionalRootTypes,
- TypeOracle typeOracle) {
+ CompilationState compilationState) {
+ if (jprogram.typeOracle.isJsInteropEnabled()) {
+ Iterables.addAll(allRootTypes, compilationState.getQualifiedJsInteropRootTypesNames());
+ }
Collections.addAll(allRootTypes, entryPointTypeNames);
Collections.addAll(allRootTypes, additionalRootTypes);
allRootTypes.addAll(JProgram.CODEGEN_TYPES_SET);
@@ -1141,49 +1142,11 @@
* Add all SingleJsoImpl types that we know about. It's likely that the concrete types are
* never explicitly referenced.
*/
+ TypeOracle typeOracle = compilationState.getTypeOracle();
for (com.google.gwt.core.ext.typeinfo.JClassType singleJsoIntf :
typeOracle.getSingleJsoImplInterfaces()) {
allRootTypes.add(typeOracle.getSingleJsoImpl(singleJsoIntf).getQualifiedSourceName());
}
-
- if (jprogram.typeOracle.isJsInteropEnabled()) {
- // find any types with @JsExport could be entry points as well
- nextType:
- for (com.google.gwt.dev.javac.typemodel.JClassType type :
- typeOracle.getTypes()) {
- for (Annotation ann : type.getAnnotations()) {
- // If the type immediately exports symbols to JS.
- if (ann.annotationType().getName().equals(JS_EXPORT_ANN)) {
- allRootTypes.add(type.getQualifiedSourceName());
- continue nextType;
- }
- }
- if (isJsType(type)) {
- // If the type or any transitive interface is a JS type.
- allRootTypes.add(type.getQualifiedSourceName());
- }
- }
- }
- }
-
- /**
- * Returns true if the type, or any super-interface has a JsType
- * annotation.
- */
- private boolean isJsType(com.google.gwt.dev.javac.typemodel.JClassType type) {
- for (Annotation ann : type.getAnnotations()) {
- if (ann.annotationType().getName().equals(JS_TYPE_ANN)) {
- return true;
- }
- }
-
- for (com.google.gwt.dev.javac.typemodel.JClassType intf : type
- .getImplementedInterfaces()) {
- if (isJsType(intf)) {
- return true;
- }
- }
- return false;
}
private void recordJsoTypes(TypeOracle typeOracle) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
index d50a567..167b15c 100755
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
@@ -309,6 +309,34 @@
return jsInteropType != JsInteropType.NONE;
}
+ public boolean isOrExtendsJsType() {
+ if (isJsType()) {
+ return true;
+ }
+ for (JInterfaceType subIntf : getImplements()) {
+ if (subIntf.isJsType()) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ public boolean hasAnyExports() {
+ for (JMethod method : getMethods()) {
+ if (method.isExported()) {
+ return true;
+ }
+ }
+
+ for (JField field : getFields()) {
+ if (field.isExported()) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
public JsInteropType getJsInteropType() {
return jsInteropType;
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JField.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JField.java
index c1696db..353d23b 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JField.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JField.java
@@ -52,10 +52,6 @@
private String exportName;
private boolean noExport = false;
- public boolean isNoExport() {
- return noExport;
- }
-
public void setNoExport(boolean noExport) {
this.noExport = noExport;
}
@@ -68,6 +64,10 @@
return exportName;
}
+ public boolean isExported() {
+ return exportName != null && !noExport;
+ }
+
private static class ExternalSerializedForm implements Serializable {
private final JDeclaredType enclosingType;
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
index aed3155..26faa37 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
@@ -68,6 +68,10 @@
return exportName;
}
+ public boolean isExported() {
+ return exportName != null && !noExport;
+ }
+
public void setJsProperty(boolean jsProperty) {
this.jsProperty = jsProperty;
}
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 5e42e85..620e408 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
@@ -256,11 +256,11 @@
}
public boolean isExportedField(JField field) {
- return isJsInteropEnabled() && field.getExportName() != null && !field.isNoExport();
+ return isJsInteropEnabled() && field.isExported();
}
public boolean isExportedMethod(JMethod method) {
- return isJsInteropEnabled() && method.getExportName() != null && !method.isNoExport();
+ return isJsInteropEnabled() && method.isExported();
}
public boolean isJsInteropEnabled() {
@@ -477,11 +477,6 @@
private final Map<String, String> jsoByInterface = Maps.newHashMap();
/**
- * A set of all JsTypes.
- */
- private final Set<JInterfaceType> jsInterfaces = Sets.newIdentityHashSet();
-
- /**
* A mapping from the type name to the actual type instance.
*/
private Map<String, JReferenceType> referenceTypesByName = Maps.newHashMap();
@@ -781,15 +776,7 @@
recordImmediateTypeRelations(moduleDeclaredTypes);
computeExtendedTypeRelations();
- jsInterfaces.clear();
-
for (JDeclaredType type : declaredTypes) {
-
- if (type instanceof JInterfaceType) {
- if (((JInterfaceType) type).isJsType()) {
- jsInterfaces.add((JInterfaceType) type);
- }
- }
// first time through, record all exported methods
for (JMethod method : type.getMethods()) {
if (isExportedMethod(method)) {
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 6191be3..d7da42e 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
@@ -767,7 +767,7 @@
}
rootTypeBinaryNames.add(rootType.getName());
- if (jsInteropEnabled && (isJsType(rootType) || hasAnyExports(rootType))) {
+ if (jsInteropEnabled && (rootType.hasAnyExports() || rootType.isOrExtendsJsType())) {
fullFlowIntoType(rootType);
}
}
@@ -963,31 +963,12 @@
if (t instanceof JClassType && requiresDevirtualization(t)) {
instantiate(t);
}
- if (hasAnyExports(t) || isJsType(t)) {
+ if (jsInteropEnabled && (t.hasAnyExports() || t.isOrExtendsJsType())) {
instantiate(t);
}
}
}
- private boolean hasAnyExports(JDeclaredType t) {
- if (!jsInteropEnabled) {
- return false;
- }
-
- for (JMethod method : t.getMethods()) {
- if (program.typeOracle.isExportedMethod(method)) {
- return true;
- }
- }
-
- for (JField field : t.getFields()) {
- if (program.typeOracle.isExportedField(field)) {
- return true;
- }
- }
- return false;
- }
-
private boolean canAccessSuperMethod(JDeclaredType type, JMethod method) {
if (method.isPrivate()) {
return false;
@@ -1311,59 +1292,60 @@
if (program.isReferenceOnly(type) && !requiresDevirtualization(type)) {
return;
}
+ if (instantiatedTypes.contains(type)) {
+ return;
+ }
if (type.isExternal()) {
assert errorsFound;
return;
}
- if (!instantiatedTypes.contains(type)) {
- instantiatedTypes.add(type);
- if (type.getSuperClass() != null) {
- instantiate(translate(type.getSuperClass()));
- }
- for (JInterfaceType intf : type.getImplements()) {
- instantiate(translate(intf));
- }
- staticInitialize(type);
- boolean isJsType = isJsType(type);
+ instantiatedTypes.add(type);
+ if (type.getSuperClass() != null) {
+ instantiate(translate(type.getSuperClass()));
+ }
+ for (JInterfaceType intf : type.getImplements()) {
+ instantiate(translate(intf));
+ }
+ staticInitialize(type);
+ boolean isJsType = jsInteropEnabled && type.isOrExtendsJsType();
- if (!type.isAbstract()) {
- // this is a concrete type, add delegation methods for any unimplemented defender methods
- maybeImplementDefenderMethods(type);
- }
+ if (!type.isAbstract()) {
+ // this is a concrete type, add delegation methods for any unimplemented defender methods
+ maybeImplementDefenderMethods(type);
+ }
- // Flow into any reachable virtual methods.
- for (JMethod method : type.getMethods()) {
- if (method.canBePolymorphic()) {
- if (isJsType) {
- // Fake a call into the method to keep it around
- flowInto(method);
- continue;
- }
- String signature = method.getSignature();
- if (virtualMethodsLive.contains(signature)) {
- assert !virtualMethodsPending.containsKey(signature);
- flowInto(method);
- } else {
- List<JMethod> pending = virtualMethodsPending.get(signature);
- if (pending == null) {
- pending = Lists.create(method);
- } else {
- pending = Lists.add(pending, method);
- }
- virtualMethodsPending.put(signature, pending);
- }
- } else if (program.typeOracle.isExportedMethod(method) &&
- (method.isStatic() || method.isConstructor())) {
- // rescue any @JsExport methods
+ // Flow into any reachable virtual methods.
+ for (JMethod method : type.getMethods()) {
+ if (method.canBePolymorphic()) {
+ if (isJsType) {
+ // Fake a call into the method to keep it around
flowInto(method);
+ continue;
}
+ String signature = method.getSignature();
+ if (virtualMethodsLive.contains(signature)) {
+ assert !virtualMethodsPending.containsKey(signature);
+ flowInto(method);
+ } else {
+ List<JMethod> pending = virtualMethodsPending.get(signature);
+ if (pending == null) {
+ pending = Lists.create(method);
+ } else {
+ pending = Lists.add(pending, method);
+ }
+ virtualMethodsPending.put(signature, pending);
+ }
+ } else if (program.typeOracle.isExportedMethod(method)
+ && (method.isStatic() || method.isConstructor())) {
+ // rescue any @JsExport methods
+ flowInto(method);
}
+ }
- for (JField field : type.getFields()) {
- if (field.isStatic() && program.typeOracle.isExportedField(field)) {
- flowInto(field);
- }
+ for (JField field : type.getFields()) {
+ if (field.isStatic() && program.typeOracle.isExportedField(field)) {
+ flowInto(field);
}
}
}
@@ -1434,22 +1416,6 @@
return type == program.getJavaScriptObject() || isJso(type.getSuperClass());
}
- private boolean isJsType(JDeclaredType intf) {
- if (!jsInteropEnabled) {
- return false;
- }
-
- if (intf.isJsType()) {
- return true;
- }
- for (JInterfaceType subIntf : intf.getImplements()) {
- if (isJsType(subIntf)) {
- return true;
- }
- }
- return false;
- }
-
/**
* Main loop: run through the queue doing deferred resolution. We could have
* made this entirely recursive, but a work queue uses much less max stack.
diff --git a/dev/core/test/com/google/gwt/dev/javac/MockCompilationUnit.java b/dev/core/test/com/google/gwt/dev/javac/MockCompilationUnit.java
index c22303b..75f65c9 100644
--- a/dev/core/test/com/google/gwt/dev/javac/MockCompilationUnit.java
+++ b/dev/core/test/com/google/gwt/dev/javac/MockCompilationUnit.java
@@ -122,6 +122,11 @@
}
@Override
+ boolean hasJsInteropRootType() {
+ return false;
+ }
+
+ @Override
ContentId getContentId() {
return contentId;
}
diff --git a/user/test/com/google/gwt/core/client/interop/JsExportTest.java b/user/test/com/google/gwt/core/client/interop/JsExportTest.java
index 57c6475..afaf108 100644
--- a/user/test/com/google/gwt/core/client/interop/JsExportTest.java
+++ b/user/test/com/google/gwt/core/client/interop/JsExportTest.java
@@ -55,6 +55,16 @@
assertTrue(MyClassExportsMethod.calledFromFoo);
}
+ public void testMethodExport_notReferencedFromJava() {
+ // Exported by MyClassExportsMethodWithoutReference which is not referenced by Java. This
+ // ensures that we correctly collect root types.
+ assertEquals(42, onlyCalledFromJs());
+ }
+
+ private native int onlyCalledFromJs() /*-{
+ return $wnd.onlyCalledFromJs();
+ }-*/;
+
public void testClinit() {
ScriptInjector.fromString("new $wnd.MyClassExportsMethodWithClinit();").inject();
assertEquals(23, MyClassExportsMethodWithClinit.magicNumber);
diff --git a/user/test/com/google/gwt/core/client/interop/MyClassExportsMethodWithoutReference.java b/user/test/com/google/gwt/core/client/interop/MyClassExportsMethodWithoutReference.java
new file mode 100644
index 0000000..27ab623
--- /dev/null
+++ b/user/test/com/google/gwt/core/client/interop/MyClassExportsMethodWithoutReference.java
@@ -0,0 +1,28 @@
+/*
+ * 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.core.client.interop;
+
+import com.google.gwt.core.client.js.JsExport;
+
+/**
+ * A test class exports a method but isn't referenced from any Java code.
+ */
+public class MyClassExportsMethodWithoutReference {
+ @JsExport("onlyCalledFromJs")
+ public static int callMe() {
+ return 42;
+ }
+}
