Skip to content

Commit

Permalink
Fix NPE in SDM due to Boolean/Double/String devirtualization.
Browse files Browse the repository at this point in the history 
Devirtualization of these types requires that their methods be traversed
and available for rewriting.

Bug: #9424
Bug-Link: #9424
Change-Id: I3addaae1de1e178c5b1276c7804fbf29686ff8f1
  • Loading branch information
@rluble
rluble committed Sep 19, 2016
1 parent 154bc52 commit 9b907f2
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 17 deletions.
6 changes: 3 additions & 3 deletions dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
View file
Expand Up @@ -328,9 +328,6 @@ private PermutationResult compilePermutation(Permutation permutation, UnifiedAst
// TODO(stalcup): hide metrics gathering in a callback or subclass
logger.log(TreeLogger.INFO, "Compiling permutation " + permutationId + "...");

// Rewrite calls to from boxed constructor types to specialized unboxed methods
RewriteConstructorCallsForUnboxedTypes.exec(jprogram);

// (2) Transform unresolved Java AST to resolved Java AST
ResolvePermutationDependentValues
.exec(jprogram, properties, permutation.getPropertyAndBindingInfos());
Expand All @@ -351,6 +348,9 @@ private PermutationResult compilePermutation(Permutation permutation, UnifiedAst
// record references to runtime classes like LongLib).
maybeRecordReferencesAndControlFlow(false);

// Rewrite calls to from boxed constructor types to specialized unboxed methods
RewriteConstructorCallsForUnboxedTypes.exec(jprogram);

// Replace compile time constants by their values.
// TODO(rluble): eventually move to normizeSemantics.
CompileTimeConstantsReplacer.exec(jprogram);
Expand Down
20 changes: 17 additions & 3 deletions dev/core/src/com/google/gwt/dev/jjs/impl/RewriteConstructorCallsForUnboxedTypes.java
View file
Expand Up @@ -23,10 +23,14 @@
import com.google.gwt.dev.jjs.ast.JModVisitor;
import com.google.gwt.dev.jjs.ast.JNewInstance;
import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.jjs.ast.JType;
import com.google.gwt.dev.jjs.ast.js.JsniMethodRef;
import com.google.gwt.dev.util.log.speedtracer.CompilerEventType;
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.base.Function;
import com.google.gwt.thirdparty.guava.common.base.Joiner;
import com.google.gwt.thirdparty.guava.common.collect.Iterables;

import java.util.HashMap;
import java.util.Map;
Expand All @@ -49,7 +53,7 @@ public RewriteConstructorCallsForUnboxedTypes(JProgram program) {
createMethodsByType.put(unboxedType, createMethods);
for (JMethod method : unboxedType.getMethods()) {
if (method.getName().startsWith(NATIVE_TYPE_CREATEMETHOD_PREFIX)) {
createMethods.put(method.getOriginalParamTypes().toString(), method);
createMethods.put(getParametersAsString(method), method);
}
}
}
Expand All @@ -67,7 +71,7 @@ public void endVisit(JNewInstance x, Context ctx) {
JMethod createMethod =
createMethodsByType
.get(ctor.getEnclosingType())
.get(ctor.getOriginalParamTypes().toString());
.get(getParametersAsString(ctor));
assert createMethod != null;

JMethodCall createCall = new JMethodCall(x.getSourceInfo(), null, createMethod);
Expand All @@ -84,7 +88,7 @@ public void endVisit(JsniMethodRef x, Context ctx) {
JMethod createMethod =
createMethodsByType
.get(ctor.getEnclosingType())
.get(ctor.getOriginalParamTypes().toString());
.get(getParametersAsString(ctor));
assert createMethod != null;

JsniMethodRef newJsniMethodRef = new JsniMethodRef(x.getSourceInfo(),
Expand All @@ -93,6 +97,16 @@ public void endVisit(JsniMethodRef x, Context ctx) {
}
}

private static String getParametersAsString(JMethod method) {
return Joiner.on(",").join(Iterables.transform(method.getOriginalParamTypes(),
new Function<JType, String>() {
@Override
public String apply(JType type) {
return type.getJsniSignatureName();
}
}));
}

private static final String NAME = RewriteConstructorCallsForUnboxedTypes.class
.getSimpleName();

Expand Down
23 changes: 14 additions & 9 deletions dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
View file
Expand Up @@ -825,12 +825,13 @@ public void exec() throws UnableToCompleteException {
// visitor execution after unification. Since we don't want those fields are methods to be
// prematurely pruned here we defensively trace them now.
for (JClassType type : program.codeGenTypes) {
for (JMethod method : type.getMethods()) {
flowInto(method);
}
for (JField field : type.getFields()) {
flowInto(field);
}
flowInto(type);
}

// Make sure that the rewriting pass for the types that are represented as natives have the
// needed members available.
for (JDeclaredType type : program.getRepresentedAsNativeTypes()) {
flowInto(type);
}

if (incrementalCompile) {
Expand Down Expand Up @@ -1124,12 +1125,16 @@ private void fullFlowIntoType(JDeclaredType type) {
// attempt is shorter.
processedStaleTypeNames.add(typeName);
instantiate(type);
for (JField field : type.getFields()) {
flowInto(field);
}
flowInto(type);
}

private void flowInto(JDeclaredType type) {
for (JMethod method : type.getMethods()) {
flowInto(method);
}
for (JField field : type.getFields()) {
flowInto(field);
}
}

private void flowInto(JField field) {
Expand Down
67 changes: 65 additions & 2 deletions dev/core/test/com/google/gwt/dev/CompilerTest.java
View file
Expand Up @@ -1047,6 +1047,7 @@ public void testReferenceThroughJsFunction() throws Exception {
public void testChangeJsNamespaceOnMethod() throws Exception {
CompilerOptions compilerOptions = new CompilerOptionsImpl();
compilerOptions.setUseDetailedTypeIds(true);
compilerOptions.setGenerateJsInteropExports(true);

MockJavaResource jsNamespaceFooResource =
JavaResourceBase.createMockJavaResource(
Expand Down Expand Up @@ -1086,6 +1087,7 @@ public void testChangeJsNamespaceOnMethod() throws Exception {
public void testChangeJsNamespaceOnClass() throws Exception {
CompilerOptions compilerOptions = new CompilerOptionsImpl();
compilerOptions.setUseDetailedTypeIds(true);
compilerOptions.setGenerateJsInteropExports(true);

MockJavaResource jsNamespaceFooResource =
JavaResourceBase.createMockJavaResource(
Expand Down Expand Up @@ -1124,6 +1126,7 @@ public void testChangeJsNamespaceOnClass() throws Exception {
public void testChangeJsFunction() throws Exception {
CompilerOptions compilerOptions = new CompilerOptionsImpl();
compilerOptions.setUseDetailedTypeIds(true);
compilerOptions.setGenerateJsInteropExports(true);

MockJavaResource jsFunctionIFooResource =
JavaResourceBase.createMockJavaResource(
Expand Down Expand Up @@ -1170,6 +1173,7 @@ public void testChangeJsFunction() throws Exception {
public void testChangeJsProperty() throws Exception {
CompilerOptions compilerOptions = new CompilerOptionsImpl();
compilerOptions.setUseDetailedTypeIds(true);
compilerOptions.setGenerateJsInteropExports(true);

MockJavaResource jsPropertyIFooResource =
JavaResourceBase.createMockJavaResource(
Expand Down Expand Up @@ -1222,6 +1226,7 @@ public void testChangeJsProperty() throws Exception {
public void testChangeJsType() throws Exception {
CompilerOptions compilerOptions = new CompilerOptionsImpl();
compilerOptions.setUseDetailedTypeIds(true);
compilerOptions.setGenerateJsInteropExports(true);

MockJavaResource jsTypeFooResource =
JavaResourceBase.createMockJavaResource(
Expand Down Expand Up @@ -1256,6 +1261,7 @@ public void testChangeJsType() throws Exception {
public void testChangeJsTypeNative() throws Exception {
CompilerOptions compilerOptions = new CompilerOptionsImpl();
compilerOptions.setUseDetailedTypeIds(true);
compilerOptions.setGenerateJsInteropExports(true);

MockJavaResource nativeFooResource =
JavaResourceBase.createMockJavaResource(
Expand Down Expand Up @@ -1295,6 +1301,7 @@ public void testChangeJsTypeNative() throws Exception {
public void testChangeJsIgnore() throws Exception {
CompilerOptions compilerOptions = new CompilerOptionsImpl();
compilerOptions.setUseDetailedTypeIds(true);
compilerOptions.setGenerateJsInteropExports(true);

MockJavaResource jsIgnoreFooResource =
JavaResourceBase.createMockJavaResource(
Expand Down Expand Up @@ -1329,6 +1336,7 @@ public void testJsInteropNameCollision() throws Exception {
MinimalRebuildCache minimalRebuildCache = new MinimalRebuildCache();
File applicationDir = Files.createTempDir();
CompilerOptions compilerOptions = new CompilerOptionsImpl();
compilerOptions.setGenerateJsInteropExports(true);

// Simple compile with one dialog.alert() export succeeds.
compileToJs(compilerOptions, applicationDir, "com.foo.SimpleModule", Lists.newArrayList(
Expand Down Expand Up @@ -1590,7 +1598,6 @@ public void testIncrementalRecompile_withErrors()
"<module>",
" <source path=''/>",
" <entry-point class='com.foo.ErrorsEntryPoint'/>",
" <add-linker name='xsiframe'/>",
"</module>");

MockJavaResource entryPointResource =
Expand Down Expand Up @@ -1652,6 +1659,63 @@ public void testIncrementalRecompile_withErrors()
}
}

public void testIncrementalRecompile_representedAsNative()
throws UnableToCompleteException, IOException, InterruptedException {
MockResource moduleResource =
JavaResourceBase.createMockResource(
"com/foo/RepresentedAsNative.gwt.xml",
"<module>",
" <source path=''/>",
" <entry-point class='com.foo.RepresentedAsNativeEntryPoint'/>",
"</module>");

MockResource entryPointResource =
JavaResourceBase.createMockJavaResource(
"com.foo.RepresentedAsNativeEntryPoint",
"package com.foo;",
"import com.google.gwt.core.client.EntryPoint;",
"public class RepresentedAsNativeEntryPoint implements EntryPoint {",
" public void onModuleLoad() {",
" Double d = new Double(1d);",
" }",
"}");

MockResource modifiedEntryPointResource =
JavaResourceBase.createMockJavaResource(
"com.foo.RepresentedAsNativeEntryPoint",
"package com.foo;",
"import com.google.gwt.core.client.EntryPoint;",
"public class RepresentedAsNativeEntryPoint implements EntryPoint {",
" public void onModuleLoad() {",
" Double d = new Double(\"1\");",
" }",
"}");

PrintWriterTreeLogger logger = new PrintWriterTreeLogger();
logger.setMaxDetail(TreeLogger.ERROR);

MinimalRebuildCache minimalRebuildCache = new MinimalRebuildCache();
File applicationDir = Files.createTempDir();
CompilerOptions compilerOptions = new CompilerOptionsImpl();
compilerOptions.setUseDetailedTypeIds(true);
compilerOptions.setSourceLevel(SourceLevel.JAVA8);
compilerOptions.setGenerateJsInteropExports(false);

// Compile the application with no errors.
compileToJs(logger, compilerOptions, applicationDir, "com.foo.RepresentedAsNative",
Lists.newArrayList(moduleResource, entryPointResource), minimalRebuildCache,
emptySet, JsOutputOption.OBFUSCATED);

// Recompile but now the changed file has an error
compileToJs(logger, compilerOptions, applicationDir, "com.foo.RepresentedAsNative",
Lists.newArrayList(modifiedEntryPointResource),
minimalRebuildCache,
stringSet(
"com.foo.RepresentedAsNativeEntryPoint",
getEntryMethodHolderTypeName("com.foo.RepresentedAsNative")),
JsOutputOption.OBFUSCATED);
}

public void testIncrementalRecompile_functionSignatureChange() throws UnableToCompleteException,
IOException, InterruptedException {
// Not testing recompile equality with Pretty/Obfuscated output since the JsIncrementalNamer's
Expand Down Expand Up @@ -2570,7 +2634,6 @@ private String compileToJs(TreeLogger logger, CompilerOptions compilerOptions, F
// Setup options to perform a per-file compile, output to this new application directory and
// compile the given module.
compilerOptions.setIncrementalCompileEnabled(true);
compilerOptions.setGenerateJsInteropExports(true);
compilerOptions.setWarDir(applicationDir);
compilerOptions.setModuleNames(ImmutableList.of(moduleName));
compilerOptions.setOutput(output);
Expand Down

0 comments on commit 9b907f2

Please sign in to comment.