Fix a bug in GwtAstBuilder.
The ReferenceMapper object in GwtAstBuilder, which caches name to type
instances, had the wrong lifetime.
That may cause compilation units to be serialized sometimes with non
stub to types outside itself. That resulted in an unified AST with
multiple type instances representing a single type.
Then queries like isInstantiated that are keyed by the type instance
failed and produced incorrect optimizations.
Change-Id: Ie154e7eab257d9c0705a30c2196caec5993657a2
diff --git a/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java b/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
index afb0897..92bd1fe 100644
--- a/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
+++ b/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
@@ -126,7 +126,7 @@
if (!cud.compilationResult().hasErrors()) {
// The above checks might have recorded errors; so we need to check here again.
// So only construct the GWT AST if no JDT errors and no errors from our checks.
- types = astBuilder.process(cud, builder.getSourceMapPath(), jsniMethods, jsniRefs,
+ types = GwtAstBuilder.process(cud, builder.getSourceMapPath(), jsniMethods, jsniRefs,
compilerContext);
}
@@ -189,8 +189,6 @@
*/
private final Map<String, CompiledClass> allValidClasses = Maps.newHashMap();
- private final GwtAstBuilder astBuilder = new GwtAstBuilder();
-
private transient LinkedBlockingQueue<CompilationUnitBuilder> buildQueue;
/**
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JClassLiteral.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JClassLiteral.java
index 6318efd..29c291c 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JClassLiteral.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JClassLiteral.java
@@ -54,6 +54,7 @@
@Override
public JType getType() {
+ assert field != null;
return field.getType();
}
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 90ac909..9caece0 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
@@ -913,9 +913,8 @@
* }
* </pre>
*/
- JLocal arrayVar =
- JProgram.createLocal(info, elementVarName + "$array", collection.getType(), true,
- curMethod.body);
+ JLocal arrayVar = JProgram.createLocal(info, elementVarName + "$array",
+ typeMap.get(x.collection.resolvedType), true, curMethod.body);
JLocal indexVar =
JProgram.createLocal(info, elementVarName + "$index", JPrimitiveType.INT, false,
curMethod.body);
@@ -3597,10 +3596,12 @@
static class CudInfo {
public final CompilationUnitScope scope;
public final int[] separatorPositions;
+ public final CompilationUnitDeclaration cud;
public CudInfo(CompilationUnitDeclaration cud) {
separatorPositions = cud.compilationResult().getLineSeparatorPositions();
scope = cud.scope;
+ this.cud = cud;
}
}
@@ -3769,25 +3770,13 @@
/**
* Builds all the GWT AST nodes that correspond to one Java source file.
*
- * @param cud The compiled form of the Java source from the JDT.
- * @param sourceMapPath the path that should be included in a sourcemap.
- * @param jsniMethods Native methods to add to the AST.
- * @param jsniRefs Map from JSNI references to their JDT definitions.
- * @param compilerContext the compiler context.
* @return All the types seen in this source file.
*/
- public List<JDeclaredType> process(CompilationUnitDeclaration cud, String sourceMapPath,
- Map<MethodDeclaration, JsniMethod> jsniMethods, Map<String, Binding> jsniRefs,
- CompilerContext compilerContext) {
+ private List<JDeclaredType> processImpl() {
+ CompilationUnitDeclaration cud = curCud.cud;
if (cud.types == null) {
return Collections.emptyList();
}
- this.sourceMapPath = sourceMapPath;
- this.jsniRefs = jsniRefs;
- this.jsniMethods = jsniMethods;
- this.compilerContext = compilerContext;
- newTypes = Lists.newArrayList();
- curCud = new CudInfo(cud);
for (TypeDeclaration typeDecl : cud.types) {
createTypes(typeDecl);
@@ -3811,20 +3800,25 @@
// Build the code.
typeDecl.traverse(astVisitor, cud.scope);
}
+ return newTypes;
+ }
- List<JDeclaredType> result = newTypes;
-
- // Clean up.
- typeMap.clearSource();
+ public GwtAstBuilder(CompilationUnitDeclaration cud, String sourceMapPath,
+ Map<MethodDeclaration, JsniMethod> jsniMethods, Map<String, Binding> jsniRefs,
+ CompilerContext compilerContext) {
+ this.sourceMapPath = sourceMapPath;
this.jsniRefs = jsniRefs;
this.jsniMethods = jsniMethods;
- newTypes = null;
- curCud = null;
- javaLangObject = null;
- javaLangString = null;
- javaLangClass = null;
- javaLangThrowable = null;
- return result;
+ this.compilerContext = compilerContext;
+ newTypes = Lists.newArrayList();
+ curCud = new CudInfo(cud);
+ }
+
+ public static List<JDeclaredType> process(CompilationUnitDeclaration cud, String sourceMapPath,
+ Map<MethodDeclaration, JsniMethod> jsniMethods, Map<String, Binding> jsniRefs,
+ CompilerContext compilerContext) {
+ return new GwtAstBuilder(cud, sourceMapPath, jsniMethods, jsniRefs, compilerContext)
+ .processImpl();
}
SourceInfo makeSourceInfo(AbstractMethodDeclaration x) {
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/GwtAstBuilderTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/GwtAstBuilderTest.java
new file mode 100644
index 0000000..90ac2f2
--- /dev/null
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/GwtAstBuilderTest.java
@@ -0,0 +1,224 @@
+/*
+ * 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.dev.jjs.impl;
+
+import com.google.gwt.core.ext.UnableToCompleteException;
+import com.google.gwt.dev.CompilerContext;
+import com.google.gwt.dev.PrecompileTaskOptionsImpl;
+import com.google.gwt.dev.javac.CompilationState;
+import com.google.gwt.dev.javac.CompilationStateBuilder;
+import com.google.gwt.dev.javac.CompilationUnit;
+import com.google.gwt.dev.javac.testing.impl.JavaResourceBase;
+import com.google.gwt.dev.jjs.JavaAstConstructor;
+import com.google.gwt.dev.jjs.ast.Context;
+import com.google.gwt.dev.jjs.ast.JArrayType;
+import com.google.gwt.dev.jjs.ast.JClassLiteral;
+import com.google.gwt.dev.jjs.ast.JDeclaredType;
+import com.google.gwt.dev.jjs.ast.JExpression;
+import com.google.gwt.dev.jjs.ast.JMethod;
+import com.google.gwt.dev.jjs.ast.JNullType;
+import com.google.gwt.dev.jjs.ast.JProgram;
+import com.google.gwt.dev.jjs.ast.JReferenceType;
+import com.google.gwt.dev.jjs.ast.JType;
+import com.google.gwt.dev.jjs.ast.JVariable;
+import com.google.gwt.dev.jjs.ast.JVisitor;
+import com.google.gwt.dev.resource.Resource;
+import com.google.gwt.thirdparty.guava.common.collect.Lists;
+import com.google.gwt.thirdparty.guava.common.collect.Sets;
+
+import java.util.List;
+import java.util.Set;
+
+/**
+ * Test {@link com.google.gwt.dev.jjs.impl.GwtAstBuilder} correctly builds the AST.
+ *
+ * TODO(leafwang): Write tests for all other features.
+ */
+public class GwtAstBuilderTest extends JJSTestBase {
+ Set<Resource> sources = Sets.newLinkedHashSet();
+
+ /**
+ * A Gwt AST verifier, which is used to verify that in Gwt AST, all JReferenceType instances that
+ * are not in current compilation unit are external.
+ */
+ static class CompilationUnitJavaAstVerifier extends JVisitor {
+ /**
+ * Throws an assertion error if a ReferenceType that is not in current compilation unit is not
+ * external.
+ */
+ public static void assertNonExternalOnlyInCurrentCU(CompilationUnit compilationUnit) {
+ CompilationUnitJavaAstVerifier verifier =
+ new CompilationUnitJavaAstVerifier(compilationUnit.getTypes());
+ for (JDeclaredType type : compilationUnit.getTypes()) {
+ verifier.accept(type);
+ }
+ }
+
+ final List<JDeclaredType> typesInCurrentCud;
+
+ final List<String> typeNames;
+
+ public CompilationUnitJavaAstVerifier(List<JDeclaredType> typesInCurrentCud) {
+ super();
+ this.typesInCurrentCud = typesInCurrentCud;
+ this.typeNames = Lists.newArrayList();
+ for (JDeclaredType type : typesInCurrentCud) {
+ typeNames.add(type.getName());
+ }
+ }
+
+ @Override
+ public void endVisit(JClassLiteral x, Context ctx) {
+ /**
+ * class literals only return a meaningful type after ImplementClassLiteralsAsFields has been
+ * run.
+ */
+ if (x.getField() != null) {
+ endVisit((JExpression) x, ctx);
+ }
+ }
+
+ @Override
+ public void endVisit(JExpression x, Context ctx) {
+ if (x.getType() == null) {
+ return;
+ }
+ assertExternal(x.getType().getUnderlyingType());
+ }
+
+ @Override
+ public void endVisit(JMethod x, Context ctx) {
+ assertExternal(x.getType());
+ }
+
+ @Override
+ public void endVisit(JVariable x, Context ctx) {
+ assertExternal(x.getType());
+ }
+
+ private void assertExternal(JType type) {
+ JType typeToCheck = type;
+ if (type instanceof JArrayType) {
+ typeToCheck = (((JArrayType) type).getLeafType());
+ }
+ if (typeToCheck == null || !(typeToCheck instanceof JReferenceType)
+ || typeToCheck.equals(JNullType.INSTANCE)) {
+ return;
+ }
+ if (!typeNames.contains(typeToCheck.getName())) {
+ assert (typeToCheck.isExternal());
+ }
+ }
+ }
+
+ @Override
+ public void setUp() {
+ sources.addAll(sourceOracle.getResources());
+ sources.add(JavaResourceBase.createMockJavaResource("test.DalNavigationTile",
+ "package test;",
+ "public class DalNavigationTile extends DalTile {",
+ "}"
+ ));
+
+ sources.add(JavaResourceBase.createMockJavaResource("test.DalTile",
+ "package test;",
+ "public class DalTile {"
+ + "{ new DalRow().getTiles();"
+ + "}",
+ "}"
+ ));
+
+ sources.add(JavaResourceBase.createMockJavaResource("test.DalGrid",
+ "package test;",
+ "public class DalGrid {",
+ " public DalNavigationTile getNavigationTile() {"
+ + " DalRow row = new DalRow();"
+ + " DalNavigationTile found = null;"
+ + " for (DalTile dalTile : row.getTiles()) {"
+ + " if (dalTile instanceof DalNavigationTile) {"
+ + " found = (DalNavigationTile) dalTile;"
+ + " break;"
+ + " }"
+ + " }"
+ + " return found;"
+ + "}",
+ "}"
+ ));
+
+ sources.add(JavaResourceBase.createMockJavaResource("test.DalRow",
+ "package test;",
+ "public class DalRow {"
+ + " public DalTile[] getTiles() {"
+ + " int length = 5;"
+ + " DalTile[] result = new DalTile[length];"
+ + " for (int i = 0; i < length; i++) {"
+ + " result[i] = new DalTile();"
+ + " }"
+ + " return result;"
+ + " }",
+ "}"
+ ));
+ }
+
+ public void testUniqueArrayTypeInstance() throws UnableToCompleteException {
+ JProgram program = compileProgram("test.DalGrid");
+ Set<String> arrayTypeNames = Sets.newHashSet();
+ for (JArrayType type : program.getAllArrayTypes()) {
+ arrayTypeNames.add(type.getName());
+ }
+ assertEquals(arrayTypeNames.size(), program.getAllArrayTypes().size());
+ }
+
+ public void testNonExternalOnlyInCurrentCud() throws UnableToCompleteException {
+ CompilationState state = buildCompilationState();
+ for (CompilationUnit compilationUnit : state.getCompilationUnits()) {
+ CompilationUnitJavaAstVerifier.assertNonExternalOnlyInCurrentCU(compilationUnit);
+ }
+ }
+
+ private CompilationState buildCompilationState() throws UnableToCompleteException {
+ CompilerContext compilerContext =
+ new CompilerContext.Builder().options(new PrecompileTaskOptionsImpl() {
+ @Override
+ public boolean shouldJDTInlineCompileTimeConstants() {
+ return false;
+ }
+ }).build();
+ compilerContext.getOptions().setSourceLevel(sourceLevel);
+ compilerContext.getOptions().setStrict(true);
+ CompilationState state = CompilationStateBuilder.buildFrom(logger, compilerContext, sources,
+ getAdditionalTypeProviderDelegate());
+ return state;
+ }
+
+ private JProgram compileProgram(String entryType) throws UnableToCompleteException {
+ CompilerContext compilerContext =
+ new CompilerContext.Builder().options(new PrecompileTaskOptionsImpl() {
+ @Override
+ public boolean shouldJDTInlineCompileTimeConstants() {
+ return false;
+ }
+ }).build();
+ compilerContext.getOptions().setSourceLevel(sourceLevel);
+ compilerContext.getOptions().setStrict(true);
+ CompilationState state = CompilationStateBuilder.buildFrom(logger, compilerContext, sources,
+ getAdditionalTypeProviderDelegate());
+ JProgram program = JavaAstConstructor.construct(logger, state, compilerContext.getOptions(),
+ null, entryType, "com.google.gwt.lang.Exceptions");
+ return program;
+ }
+}
