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; + } +}