Skip to content

Commit

Permalink
Remove string based JavaScript instanceof.
Browse files Browse the repository at this point in the history
Change-Id: I1cbf5feb30cefc812d03ea7bac0877cef44eceae
  • Loading branch information
rluble authored and Gerrit Code Review committed Jan 12, 2016
1 parent f7355dd commit dca53da
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 15 deletions.
19 changes: 19 additions & 0 deletions dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
Expand Up @@ -30,6 +30,7 @@
import com.google.gwt.dev.jjs.ast.JExpression; import com.google.gwt.dev.jjs.ast.JExpression;
import com.google.gwt.dev.jjs.ast.JField; import com.google.gwt.dev.jjs.ast.JField;
import com.google.gwt.dev.jjs.ast.JFieldRef; import com.google.gwt.dev.jjs.ast.JFieldRef;
import com.google.gwt.dev.jjs.ast.JInstanceOf;
import com.google.gwt.dev.jjs.ast.JInterfaceType; import com.google.gwt.dev.jjs.ast.JInterfaceType;
import com.google.gwt.dev.jjs.ast.JLocal; import com.google.gwt.dev.jjs.ast.JLocal;
import com.google.gwt.dev.jjs.ast.JLocalRef; import com.google.gwt.dev.jjs.ast.JLocalRef;
Expand Down Expand Up @@ -200,6 +201,10 @@ public boolean visit(JBinaryOperation x, Context ctx) {
public boolean visit(JCastOperation x, Context ctx) { public boolean visit(JCastOperation x, Context ctx) {
// Rescue any JavaScriptObject type that is the target of a cast. // Rescue any JavaScriptObject type that is the target of a cast.
JType targetType = x.getCastType(); JType targetType = x.getCastType();

// Casts to native classes use the native constructor qualified name.
maybeRescueNativeConstructor(targetType);

if (!canBeInstantiatedInJavaScript(targetType)) { if (!canBeInstantiatedInJavaScript(targetType)) {
return true; return true;
} }
Expand Down Expand Up @@ -308,6 +313,13 @@ public boolean visit(JFieldRef ref, Context ctx) {
return true; return true;
} }


@Override
public boolean visit(JInstanceOf expression, Context ctx) {
// Instanceof checks for native classes use the native constructor qualified name.
maybeRescueNativeConstructor(expression.getTestType());
return true;
}

@Override @Override
public boolean visit(JInterfaceType type, Context ctx) { public boolean visit(JInterfaceType type, Context ctx) {
boolean isReferenced = referencedTypes.contains(type); boolean isReferenced = referencedTypes.contains(type);
Expand Down Expand Up @@ -752,6 +764,13 @@ private void rescue(JVariable var) {
} }
} }


private void maybeRescueNativeConstructor(JType type) {
JConstructor jsConstructor = JjsUtils.getJsNativeConstructorOrNull(type);
if (jsConstructor != null) {
rescue(jsConstructor);
}
}

/** /**
* The code is very tightly tied to the behavior of * The code is very tightly tied to the behavior of
* Pruner.CleanupRefsVisitor. CleanUpRefsVisitor will prune unread * Pruner.CleanupRefsVisitor. CleanUpRefsVisitor will prune unread
Expand Down
Expand Up @@ -1097,6 +1097,10 @@ public JsNode transformCastMap(JCastMap castMap) {
@Override @Override
public JsNameRef transformJsniMethodRef(JsniMethodRef jsniMethodRef) { public JsNameRef transformJsniMethodRef(JsniMethodRef jsniMethodRef) {
JMethod method = jsniMethodRef.getTarget(); JMethod method = jsniMethodRef.getTarget();
if (method.isJsNative()) {
// Construct Constructor.prototype.jsname or Constructor.
return createJsQualifier(method.getQualifiedJsName(), jsniMethodRef.getSourceInfo());
}
return names.get(method).makeRef(jsniMethodRef.getSourceInfo()); return names.get(method).makeRef(jsniMethodRef.getSourceInfo());
} }


Expand Down
Expand Up @@ -34,6 +34,7 @@
import com.google.gwt.dev.jjs.ast.JRuntimeTypeReference; import com.google.gwt.dev.jjs.ast.JRuntimeTypeReference;
import com.google.gwt.dev.jjs.ast.JType; import com.google.gwt.dev.jjs.ast.JType;
import com.google.gwt.dev.jjs.ast.RuntimeConstants; import com.google.gwt.dev.jjs.ast.RuntimeConstants;
import com.google.gwt.dev.jjs.ast.js.JsniMethodRef;
import com.google.gwt.thirdparty.guava.common.collect.Maps; import com.google.gwt.thirdparty.guava.common.collect.Maps;


import java.util.EnumSet; import java.util.EnumSet;
Expand Down Expand Up @@ -265,8 +266,12 @@ private JMethodCall implementCastOrInstanceOfOperation(SourceInfo sourceInfo,
targetType))); targetType)));
} }
if (method.getParams().size() == 3) { if (method.getParams().size() == 3) {
call.addArg(program.getStringLiteral(sourceInfo, JDeclaredType declaredType = (JDeclaredType) targetType;
((JDeclaredType) targetType).getQualifiedJsName()));
JMethod jsConstructor = JjsUtils.getJsNativeConstructorOrNull(declaredType);
assert jsConstructor != null && declaredType.isJsNative();
call.addArg(new JsniMethodRef(sourceInfo, declaredType.getQualifiedJsName(), jsConstructor,
program.getJavaScriptObject()));
} }
return call; return call;
} }
Expand Down
9 changes: 9 additions & 0 deletions dev/core/src/com/google/gwt/dev/jjs/impl/JjsPredicates.java
Expand Up @@ -15,13 +15,22 @@
*/ */
package com.google.gwt.dev.jjs.impl; package com.google.gwt.dev.jjs.impl;


import com.google.gwt.dev.jjs.ast.HasJsInfo.JsMemberType;
import com.google.gwt.dev.jjs.ast.JMember; import com.google.gwt.dev.jjs.ast.JMember;
import com.google.gwt.dev.jjs.ast.JMethod;
import com.google.gwt.thirdparty.guava.common.base.Predicate; import com.google.gwt.thirdparty.guava.common.base.Predicate;


/** /**
* General predicates for Java AST nodes. * General predicates for Java AST nodes.
*/ */
public class JjsPredicates { public class JjsPredicates {
public static final Predicate<JMethod> IS_JS_CONSTRUCTOR =
new Predicate<JMethod>() {
@Override
public boolean apply(JMethod method) {
return method.isConstructor() && method.getJsMemberType() == JsMemberType.CONSTRUCTOR;
}
};
public static Predicate<JMember> IS_SYNTHETIC = public static Predicate<JMember> IS_SYNTHETIC =
new Predicate<JMember>() { new Predicate<JMember>() {
@Override @Override
Expand Down
13 changes: 13 additions & 0 deletions dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java
Expand Up @@ -303,6 +303,19 @@ public static JMethod createSyntheticAbstractStub(JDeclaredType type, JMethod su
return createEmptyMethodFromExample(type, superTypeMethod, true); return createEmptyMethodFromExample(type, superTypeMethod, true);
} }


/**
* Returns a native constructor of a native JsType class.
*/
public static JConstructor getJsNativeConstructorOrNull(JType type) {
if (!type.isJsNative() || !(type.getUnderlyingType() instanceof JClassType)) {
return null;
}
JMethod jsConstructor = Iterables.getFirst(Iterables.filter(
((JClassType) type).getMethods(), JjsPredicates.IS_JS_CONSTRUCTOR), null);
assert jsConstructor != null;
return (JConstructor) jsConstructor;
}

/** /**
* Returns a description for a type suitable for reporting errors to the users. * Returns a description for a type suitable for reporting errors to the users.
*/ */
Expand Down
9 changes: 9 additions & 0 deletions dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
Expand Up @@ -179,9 +179,17 @@ public void endVisit(JBinaryOperation x, Context ctx) {
x.setType(translate(x.getType().getUnderlyingType())); x.setType(translate(x.getType().getUnderlyingType()));
} }


private void maybeFlowIntoNativeConstructor(JType type) {
JConstructor jsConstructor = JjsUtils.getJsNativeConstructorOrNull(type);
if (jsConstructor != null) {
flowInto(jsConstructor);
}
}

@Override @Override
public void endVisit(JCastOperation x, Context ctx) { public void endVisit(JCastOperation x, Context ctx) {
x.resolve(translate(x.getCastType())); x.resolve(translate(x.getCastType()));
maybeFlowIntoNativeConstructor(x.getCastType());
} }


@Override @Override
Expand Down Expand Up @@ -266,6 +274,7 @@ public void endVisit(JFieldRef x, Context ctx) {
@Override @Override
public void endVisit(JInstanceOf x, Context ctx) { public void endVisit(JInstanceOf x, Context ctx) {
x.resolve(translate(x.getTestType())); x.resolve(translate(x.getTestType()));
maybeFlowIntoNativeConstructor(x.getTestType());
} }


@Override @Override
Expand Down
Expand Up @@ -127,7 +127,7 @@ static Object castToFunction(Object src) {
/** /**
* A dynamic cast that optionally checks for JsType prototypes. * A dynamic cast that optionally checks for JsType prototypes.
*/ */
static Object castToNative(Object src, JavaScriptObject dstId, String jsType) { static Object castToNative(Object src, JavaScriptObject dstId, JavaScriptObject jsType) {
// TODO(goktug): Remove canCast after new JsInterop semantics. // TODO(goktug): Remove canCast after new JsInterop semantics.
checkType(src == null || canCast(src, dstId) || jsinstanceOf(src, jsType)); checkType(src == null || canCast(src, dstId) || jsinstanceOf(src, jsType));
return src; return src;
Expand Down Expand Up @@ -163,7 +163,7 @@ static boolean instanceOfNativeArray(Object src) {
return isArray(src); return isArray(src);
} }


static boolean instanceOfNative(Object src, JavaScriptObject dstId, String jsType) { static boolean instanceOfNative(Object src, JavaScriptObject dstId, JavaScriptObject jsType) {
// TODO(goktug): Remove instanceof after new JsInterop semantics. // TODO(goktug): Remove instanceof after new JsInterop semantics.
return instanceOf(src, dstId) || jsinstanceOf(src, jsType); return instanceOf(src, dstId) || jsinstanceOf(src, jsType);
} }
Expand Down Expand Up @@ -234,16 +234,8 @@ static native boolean jsEquals(Object a, Object b) /*-{
* Determine if object is an instanceof jsType regardless of window or frame. * Determine if object is an instanceof jsType regardless of window or frame.
*/ */
@HasNoSideEffects @HasNoSideEffects
private static native boolean jsinstanceOf(Object obj, String jsTypeStr) /*-{ private static native boolean jsinstanceOf(Object obj, JavaScriptObject jsType) /*-{
if (!obj) { return obj && jsType && obj instanceof jsType;
return false;
}
var jsType = $wnd;
for (var i = 0, parts = jsTypeStr.split("."), l = parts.length; i < l ; i++) {
jsType = jsType && jsType[parts[i]];
}
return jsType && obj instanceof jsType;
}-*/; }-*/;


static native boolean jsNotEquals(Object a, Object b) /*-{ static native boolean jsNotEquals(Object a, Object b) /*-{
Expand Down
11 changes: 10 additions & 1 deletion user/test/com/google/gwt/core/interop/JsTypeArrayTest.java
Expand Up @@ -15,15 +15,23 @@
*/ */
package com.google.gwt.core.interop; package com.google.gwt.core.interop;


import com.google.gwt.core.client.ScriptInjector;
import com.google.gwt.junit.client.GWTTestCase; import com.google.gwt.junit.client.GWTTestCase;


import jsinterop.annotations.JsPackage;
import jsinterop.annotations.JsProperty; import jsinterop.annotations.JsProperty;
import jsinterop.annotations.JsType; import jsinterop.annotations.JsType;


/** /**
* Tests JsType with array functionality. * Tests JsType with array functionality.
*/ */
public class JsTypeArrayTest extends GWTTestCase { public class JsTypeArrayTest extends GWTTestCase {
@Override
protected void gwtSetUp() throws Exception {
ScriptInjector.fromString("function JsTypeArrayTest_SimpleJsTypeReturnForMultiDimArray() {}")
.setWindow(ScriptInjector.TOP_WINDOW)
.inject();
}


@Override @Override
public String getModuleName() { public String getModuleName() {
Expand Down Expand Up @@ -104,7 +112,8 @@ private native void fillArrayParam(SimpleJsTypeAsAParamHolder holder) /*-{
holder.setArrayParam([{}, {}]); holder.setArrayParam([{}, {}]);
}-*/; }-*/;


@JsType(isNative = true) @JsType(isNative = true, namespace = JsPackage.GLOBAL,
name = "JsTypeArrayTest_SimpleJsTypeReturnForMultiDimArray")
static class SimpleJsTypeReturnForMultiDimArray { static class SimpleJsTypeReturnForMultiDimArray {
@JsProperty public native int getId(); @JsProperty public native int getId();
} }
Expand Down

0 comments on commit dca53da

Please sign in to comment.