Removes incorrect JStype check from isJso.
In UnifyAst, the isJso check also tries to include
JsType but only looks at immediate interfaces, not
concerete JsType or its super classes. But fixing
that caused issues, which let me investigating its
uses.
It is used in three places:
1. Checking rebind results are not JSOs: this use is
actually incorrect because there is nothing wrong
in JsType being a rebind result. Added a test case to
reproduce the problem.
2. assimulateSourceUnits: This already explicitly
checks for JsTypes and mark them as instantiated.
3. instantiate: This use is to prevent shortcuting
types that are only reference only and doesn't
require devirtualization; JsType doesn't require
devirtualization so check doesn't apply to JsType
in this context.
So I decided to remove this code instead.
Change-Id: I2ea2e7bd47dbe100cb2866e825a7eca720ea9ea8
Review-Link: https://gwt-review.googlesource.com/#/c/11650/
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 5650a98..6191be3 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
@@ -960,11 +960,10 @@
* subtypes. That way we don't have to copy the exact semantics of ControlFlowAnalyzer.
*/
for (JDeclaredType t : types) {
- if (t instanceof JClassType && requiresDevirtualization(t)
- || hasAnyExports(t)) {
+ if (t instanceof JClassType && requiresDevirtualization(t)) {
instantiate(t);
}
- if (isJsType(t)) {
+ if (hasAnyExports(t) || isJsType(t)) {
instantiate(t);
}
}
@@ -1432,19 +1431,7 @@
if (type == null) {
return false;
}
- boolean isJso = type == program.getJavaScriptObject() || isJso(type.getSuperClass());
- if (isJso) {
- return true;
- }
-
- // if any of the superinterfaces as JsInterfaces, we consider this effectively a JSO
- // for instantiability purposes
- for (JInterfaceType intf : type.getImplements()) {
- if (isJsType(intf)) {
- return true;
- }
- }
- return false;
+ return type == program.getJavaScriptObject() || isJso(type.getSuperClass());
}
private boolean isJsType(JDeclaredType intf) {
diff --git a/dev/core/test/com/google/gwt/dev/CompilerTest.java b/dev/core/test/com/google/gwt/dev/CompilerTest.java
index 919e476..4024d40 100644
--- a/dev/core/test/com/google/gwt/dev/CompilerTest.java
+++ b/dev/core/test/com/google/gwt/dev/CompilerTest.java
@@ -28,6 +28,7 @@
import com.google.gwt.dev.javac.testing.impl.MockResource;
import com.google.gwt.dev.jjs.JsOutputOption;
import com.google.gwt.dev.util.Util;
+import com.google.gwt.dev.util.arg.OptionJsInteropMode;
import com.google.gwt.dev.util.arg.SourceLevel;
import com.google.gwt.dev.util.log.PrintWriterTreeLogger;
import com.google.gwt.thirdparty.guava.common.base.Charsets;
@@ -50,8 +51,6 @@
public static final String HELLO_MODULE = "com.google.gwt.sample.hello.Hello";
public static final String HELLO_MODULE_STACKMODE_STRIP =
"com.google.gwt.sample.hello.Hello_stackMode_strip";
- private final Compiler.ArgProcessor argProcessor;
- private final CompilerOptionsImpl options = new CompilerOptionsImpl();
private MockJavaResource packagePrivateParentResource =
JavaResourceBase.createMockJavaResource("com.foo.PackagePrivateParent",
@@ -445,6 +444,39 @@
" public void onModuleLoad() {}",
"}");
+ private MockJavaResource gwtCreateEntryPointResource =
+ JavaResourceBase.createMockJavaResource("com.foo.TestEntryPoint",
+ "package com.foo;",
+ "import com.google.gwt.core.client.EntryPoint;",
+ "import com.google.gwt.core.client.GWT;",
+ "import com.google.gwt.core.client.js.JsType;",
+ "public class TestEntryPoint implements EntryPoint {",
+ " @JsType public static class MyJsType {}",
+ " @JsType public interface MyJsTypeInterface {}",
+ " public static class MyTypeImplementsJsType implements MyJsTypeInterface {}",
+ " @Override",
+ " public void onModuleLoad() {",
+ " GWT.create(MyJsType.class);",
+ " GWT.create(MyTypeImplementsJsType.class);",
+ " }",
+ "}");
+
+ private MockJavaResource brokenGwtCreateEntryPointResource =
+ JavaResourceBase.createMockJavaResource("com.foo.TestEntryPoint",
+ "package com.foo;",
+ "import com.google.gwt.core.client.EntryPoint;",
+ "import com.google.gwt.core.client.GWT;",
+ "import com.google.gwt.core.client.JavaScriptObject;",
+ "public class TestEntryPoint implements EntryPoint {",
+ " public static class MyJso extends JavaScriptObject {",
+ " protected MyJso() {}",
+ " }",
+ " @Override",
+ " public void onModuleLoad() {",
+ " GWT.create(MyJso.class);",
+ " }",
+ "}");
+
private MockJavaResource topResource =
JavaResourceBase.createMockJavaResource("com.foo.top",
"package com.foo;",
@@ -760,10 +792,6 @@
private Set<String> emptySet = stringSet();
- public CompilerTest() {
- argProcessor = new Compiler.ArgProcessor(options);
- }
-
@Override
protected void setUp() throws Exception {
super.setUp();
@@ -774,6 +802,9 @@
}
public void testAllValidArgs() {
+ CompilerOptionsImpl options = new CompilerOptionsImpl();
+ Compiler.ArgProcessor argProcessor = new Compiler.ArgProcessor(options);
+
assertProcessSuccess(argProcessor, new String[] {"-logLevel", "DEBUG", "-style",
"PRETTY", "-ea", "-gen", "myGen",
"-war", "myWar", "-workDir", "myWork", "-extra", "myExtra", "-incremental",
@@ -805,6 +836,9 @@
}
public void testDefaultArgs() {
+ CompilerOptionsImpl options = new CompilerOptionsImpl();
+ Compiler.ArgProcessor argProcessor = new Compiler.ArgProcessor(options);
+
assertProcessSuccess(argProcessor, new String[] {"c.g.g.h.H"});
assertEquals(null, options.getGenDir());
@@ -830,6 +864,9 @@
}
public void testForbiddenArgs() {
+ CompilerOptionsImpl options = new CompilerOptionsImpl();
+ Compiler.ArgProcessor argProcessor = new Compiler.ArgProcessor(options);
+
assertProcessFailure(argProcessor, "Unknown argument", new String[]{"-out", "www"});
assertProcessFailure(argProcessor, "Source level must be one of",
new String[]{"-sourceLevel", "ssss"});
@@ -866,6 +903,25 @@
assertEquals(SourceLevel.JAVA7, SourceLevel.getBestMatchingVersion("1.7b3"));
}
+ public void testGwtCreateJsTypeRebindResult() throws Exception {
+ CompilerOptions compilerOptions = new CompilerOptionsImpl();
+ compilerOptions.setJsInteropMode(OptionJsInteropMode.Mode.JS);
+ compileToJs(compilerOptions, Files.createTempDir(), "com.foo.SimpleModule",
+ Lists.newArrayList(simpleModuleResource, gwtCreateEntryPointResource),
+ new MinimalRebuildCache(), emptySet, JsOutputOption.PRETTY);
+ }
+
+ public void testGwtCreateJsoRebindResult() throws Exception {
+ try {
+ compileToJs(Files.createTempDir(), "com.foo.SimpleModule",
+ Lists.newArrayList(simpleModuleResource, brokenGwtCreateEntryPointResource),
+ new MinimalRebuildCache(), emptySet, JsOutputOption.PRETTY);
+ fail("Compile should have failed");
+ } catch (UnableToCompleteException e) {
+ // success
+ }
+ }
+
public void testDeterministicBuild_Draft_StackModeStrip() throws
UnableToCompleteException, IOException {
assertDeterministicBuild(HELLO_MODULE_STACKMODE_STRIP, 0);
@@ -1625,7 +1681,10 @@
}
}
- assertNotNull(outputJsFile);
+ if (outputJsFile == null) {
+ throw new UnableToCompleteException();
+ }
+
if (expectedProcessedStaleTypeNames != null) {
assertEquals(expectedProcessedStaleTypeNames,
minimalRebuildCache.getProcessedStaleTypeNames());
