Skip to content

Commit

Permalink
Rescue types represented as natives when supertypes cross JS boundaries.
Browse files Browse the repository at this point in the history 
Bug: #9482
Bug-Link: #9482
Change-Id: I81df1a665a843d0cf11334c6ef47f6490d4b8291
  • Loading branch information
@rluble
rluble committed Jan 26, 2017
1 parent 93e1462 commit 382766e
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 23 deletions.
33 changes: 27 additions & 6 deletions dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
View file
Expand Up @@ -60,8 +60,10 @@
import com.google.gwt.dev.js.ast.JsNameRef;
import com.google.gwt.dev.js.ast.JsVisitor;
import com.google.gwt.thirdparty.guava.common.collect.ArrayListMultimap;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableMultimap;
import com.google.gwt.thirdparty.guava.common.collect.ListMultimap;
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;

import java.util.List;
Expand Down Expand Up @@ -517,10 +519,11 @@ private void rescueByTypeCoercion(JType targetType, JType expressionType) {
}

private boolean canBeInstantiatedInJavaScript(JType type) {
// Technically, JsType/JsFunction are also instantiatable in JavaScript but we don't track
// them using similar to JSO as if we do that then after cast normalization, they got pruned.
// Technically, JsType/JsFunction are also instantiable in JavaScript but we don't track them
// like a JSO. The JSO liveness tracking mechanism has very quirky semantics and using it to
// track JsType/JsFunctions might result on incorrect over-pruning.
if (program.typeOracle.canBeJavaScriptObject(type)
|| program.isRepresentedAsNativeJsPrimitive(type)) {
|| representedAsNativeTypesBySupertype.containsKey(type.getUnderlyingType())) {
return true;
}

Expand Down Expand Up @@ -596,10 +599,16 @@ private void maybeRescueJavaScriptObjectPassingIntoJava(JType type) {
if (!canBeInstantiatedInJavaScript(type)) {
return;
}
rescue((JReferenceType) type, true);

JReferenceType underlyingType = (JReferenceType) type.getUnderlyingType();
for (JReferenceType representedAsNativeType :
representedAsNativeTypesBySupertype.get(underlyingType)) {
rescue(representedAsNativeType, true);
}
rescue(underlyingType, true);
if (program.typeOracle.isSingleJsoImpl(type)) {
// Cast of JSO into SingleJso interface, rescue the implementor if exists
JClassType singleJsoImpl = program.typeOracle.getSingleJsoImpl((JReferenceType) type);
JClassType singleJsoImpl = program.typeOracle.getSingleJsoImpl(underlyingType);
rescue(singleJsoImpl, true);
}
}
Expand Down Expand Up @@ -935,6 +944,7 @@ private boolean isTypeInstantiatedOrJso(JDeclaredType type) {
private final RescueVisitor rescuer;
private final JMethod runAsyncOnSuccess;
private JMethod stringValueOfChar = null;
private final Multimap<JType, JDeclaredType> representedAsNativeTypesBySupertype;

public ControlFlowAnalyzer(ControlFlowAnalyzer cfa) {
program = cfa.program;
Expand All @@ -955,15 +965,26 @@ public ControlFlowAnalyzer(ControlFlowAnalyzer cfa) {
ArrayListMultimap.create(cfa.argumentsToRescueIfParameterRead);
}
rescuer = new RescueVisitor();
representedAsNativeTypesBySupertype = cfa.representedAsNativeTypesBySupertype;
}

public ControlFlowAnalyzer(JProgram program) {
public ControlFlowAnalyzer(final JProgram program) {
this.program = program;
asyncFragmentOnLoad = program.getIndexedMethod(RuntimeConstants.ASYNC_FRAGMENT_LOADER_ON_LOAD);
runAsyncOnSuccess = program.getIndexedMethod(RuntimeConstants.RUN_ASYNC_CALLBACK_ON_SUCCESS);
getClassField = program.getIndexedField(RuntimeConstants.OBJECT_CLAZZ);
getClassMethod = program.getIndexedMethod(RuntimeConstants.OBJECT_GET_CLASS);
rescuer = new RescueVisitor();

ImmutableMultimap.Builder<JType, JDeclaredType> representedAsNativeTypeBySuperTypeBuilder =
ImmutableMultimap.builder();
for (JDeclaredType type : program.getRepresentedAsNativeTypes()) {
representedAsNativeTypeBySuperTypeBuilder.put(type, type);
for (JDeclaredType superType : JjsUtils.getSupertypes(type)) {
representedAsNativeTypeBySuperTypeBuilder.put(superType, type);
}
}
representedAsNativeTypesBySupertype = representedAsNativeTypeBySuperTypeBuilder.build();
}

/**
Expand Down
26 changes: 26 additions & 0 deletions dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java
View file
Expand Up @@ -73,11 +73,13 @@
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.Sets;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;

/**
* General utilities related to Java AST manipulation.
Expand Down Expand Up @@ -568,6 +570,30 @@ public static JMethodCall getThisOrSuperConstructorCall(
return null;
}

/**
* Gets all the supertypes of {@code type}.
*/

public static Set<JDeclaredType> getSupertypes(JDeclaredType type) {
Set<JDeclaredType> superTypes = Sets.newHashSet();
addAllSuperTypes(type, superTypes);
return superTypes;
}

/**
* Adds all the supertypes of {@code type} to {@code types}.
*/
public static void addAllSuperTypes(JDeclaredType type, Set<JDeclaredType> types) {
if (type.getSuperClass() != null) {
types.add(type.getSuperClass());
addAllSuperTypes(type.getSuperClass(), types);
}
for (JInterfaceType interfaceType : type.getImplements()) {
types.add(interfaceType);
addAllSuperTypes(interfaceType, types);
}
}

/**
* Returns the JsConstructor for a class or null if it does not have any.
*/
Expand Down
14 changes: 1 addition & 13 deletions dev/core/src/com/google/gwt/dev/jjs/impl/JsniRestrictionChecker.java
View file
Expand Up @@ -17,7 +17,6 @@
import com.google.gwt.core.ext.UnableToCompleteException;
import com.google.gwt.dev.jjs.ast.Context;
import com.google.gwt.dev.jjs.ast.JDeclaredType;
import com.google.gwt.dev.jjs.ast.JInterfaceType;
import com.google.gwt.dev.jjs.ast.JMethod;
import com.google.gwt.dev.jjs.ast.JMethodBody;
import com.google.gwt.dev.jjs.ast.JProgram;
Expand Down Expand Up @@ -49,7 +48,7 @@ private void checkProgram(TreeLogger logger, final JProgram program)
throws UnableToCompleteException {
final Set<JDeclaredType> typesRequiringTrampolineDispatch = Sets.newHashSet();
for (JDeclaredType type : program.getRepresentedAsNativeTypes()) {
collectAllSuperTypes(type, typesRequiringTrampolineDispatch);
JjsUtils.addAllSuperTypes(type, typesRequiringTrampolineDispatch);
}
new JVisitor() {
@Override
Expand Down Expand Up @@ -152,17 +151,6 @@ private boolean isJsoInterface(JDeclaredType type) {
}
}

private static void collectAllSuperTypes(JDeclaredType type, Set<JDeclaredType> allSuperTypes) {
if (type.getSuperClass() != null) {
allSuperTypes.add(type.getSuperClass());
collectAllSuperTypes(type.getSuperClass(), allSuperTypes);
}
for (JInterfaceType interfaceType : type.getImplements()) {
allSuperTypes.add(interfaceType);
collectAllSuperTypes(interfaceType, allSuperTypes);
}
}

private static boolean isNonStaticJsoClassDispatch(JMethod method, JDeclaredType enclosingType) {
return !method.isStatic() && enclosingType.isJsoType();
}
Expand Down
43 changes: 39 additions & 4 deletions dev/core/test/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzerTest.java
View file
Expand Up @@ -61,7 +61,7 @@ public void assertOnlyInstantiatedTypes(String... expectedTypes) {
Set<JType> expectedSet = Sets.newHashSet();
for (String expectedType : expectedTypes) {
JType type = findType(program, expectedType);
assertNotNull(type);
assertNotNull("Type " + expectedType + " not instantiated.", type);
expectedSet.add(type);
}
assertEquals(expectedSet, cfa.getInstantiatedTypes());
Expand Down Expand Up @@ -264,8 +264,9 @@ public void testRescueJavaScriptObjectReturnedFromFieldReference() throws Except
);
analyzeSnippet("Test.field.hashCode();").assertOnlyInstantiatedTypes(
"JsoIntf", "SingleJso", "JavaScriptObject", "Object",
// These are all live because of the method Test.toString can be referenced externally.
"String", "Comparable", "CharSequence", "Serializable");
// These are all live because of the method Test.toString and equals can be referenced
// externally
"String", "Comparable", "CharSequence", "Serializable", "Boolean", "Number", "Double");

addOrReplaceResource("test.Test",
"package test;",
Expand All @@ -277,8 +278,42 @@ public void testRescueJavaScriptObjectReturnedFromFieldReference() throws Except
);
analyzeSnippet("Test.field.hashCode();").assertOnlyInstantiatedTypes(
"JsoIntf", "SingleJso", "JavaScriptObject", "Object",
// These are all live because of the method Test.toString can be referenced externally.
// These are all live because of the method Test.toString and equals can be referenced
// externally
"String", "Comparable", "CharSequence", "Serializable", "Boolean", "Number", "Double");
}

/**
* Tests that the JavaScriptObject type gets rescued in the three specific
* circumstances where values can pass from JS into Java.
*/
public void testRescueRepresentedAsNative() throws Exception {
addOrReplaceResource("test.Test",
"package test;",
"import jsinterop.annotations.JsMethod;",
"public class Test {",
" @JsMethod",
" public static native Object getObject();",
" @JsMethod",
" public static native Number getNumber();",
" @JsMethod",
" public static native Comparable getComparable();",
" @JsMethod",
" public static native Double getDouble();",
"}"
);

addSnippetImport("test.Test");

analyzeSnippet("Test.getObject();").assertOnlyInstantiatedTypes(
"Object", "Boolean", "Number", "Double",
"String", "Comparable", "CharSequence", "Serializable");
analyzeSnippet("Test.getNumber();").assertOnlyInstantiatedTypes(
"Object", "Number", "Double", "Serializable");
analyzeSnippet("Test.getComparable();").assertOnlyInstantiatedTypes(
"Object", "String", "Comparable", "CharSequence", "Serializable");
analyzeSnippet("Test.getDouble();").assertOnlyInstantiatedTypes(
"Object", "Number", "Double", "Serializable");
}

private void addOrReplaceResource(String qualifiedTypeName, final String... source) {
Expand Down

0 comments on commit 382766e

Please sign in to comment.