Skip to content

Commit

Permalink
Update J2clEqualitySameRewriterPass to rewrite Equality.same when:
Browse files Browse the repository at this point in the history 
1. At least one of the arguments is Object.
2. Two arguments are the same type.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=126809919
  • Loading branch information
@nanj01 @blickly
nanj01 authored and blickly committed Jul 7, 2016
1 parent d5f6f30 commit bb012f3
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 20 deletions.
58 changes: 49 additions & 9 deletions src/com/google/javascript/jscomp/J2clEqualitySameRewriterPass.java
View file
Expand Up @@ -18,13 +18,20 @@
import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
import com.google.javascript.rhino.IR; import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.jstype.JSType;


/** /**
* An optimization pass to re-write J2CL Equality.$same. * An optimization pass to re-write J2CL Equality.$same.
*/ */
public class J2clEqualitySameRewriterPass extends AbstractPostOrderCallback public class J2clEqualitySameRewriterPass extends AbstractPostOrderCallback
implements CompilerPass { implements CompilerPass {


/** Whether to use "==" or "===". */
private static enum Eq {
DOUBLE,
TRIPLE
}

private final AbstractCompiler compiler; private final AbstractCompiler compiler;


J2clEqualitySameRewriterPass(AbstractCompiler compiler) { J2clEqualitySameRewriterPass(AbstractCompiler compiler) {
Expand All @@ -46,24 +53,40 @@ public void visit(NodeTraversal t, Node node, Node parent) {
private void trySubstituteEqualitySame(Node callNode) { private void trySubstituteEqualitySame(Node callNode) {
Node firstExpr = callNode.getSecondChild(); Node firstExpr = callNode.getSecondChild();
Node secondExpr = callNode.getLastChild(); Node secondExpr = callNode.getLastChild();
if (!NodeUtil.isLiteralValue(firstExpr, true) && !NodeUtil.isLiteralValue(secondExpr, true)) {
if (NodeUtil.isNullOrUndefined(firstExpr) || NodeUtil.isNullOrUndefined(secondExpr)) {
// At least one side is null or undefined so no coercion danger.
rewriteToEq(callNode, firstExpr, secondExpr, Eq.DOUBLE);
return;
}

if (NodeUtil.isLiteralValue(firstExpr, true) || NodeUtil.isLiteralValue(secondExpr, true)) {
// There is a coercion danger but since at least one side is not null, we can use === that
// will not trigger any coercion.
rewriteToEq(callNode, firstExpr, secondExpr, Eq.TRIPLE);
return; return;
} }


// At least one is literal value. So we can replace w/ a simpler form. // This part requires "--use_types_for_optimization" to be enabled, otherwise it will not do
// any optimization.
JSType firstType = getTypeRestrictByNotNullOrUndefined(firstExpr);
JSType secondType = getTypeRestrictByNotNullOrUndefined(secondExpr);
if (isObjectType(firstType) || isObjectType(secondType) || sameType(firstType, secondType)) {
// Typeof is same for both so no coersion danger.
rewriteToEq(callNode, firstExpr, secondExpr, Eq.DOUBLE);
return;
}
}

private void rewriteToEq(Node callNode, Node firstExpr, Node secondExpr, Eq eq) {
firstExpr.detachFromParent(); firstExpr.detachFromParent();
secondExpr.detachFromParent(); secondExpr.detachFromParent();
Node replacement = asEqOperation(firstExpr, secondExpr); Node replacement =
eq == Eq.DOUBLE ? IR.eq(firstExpr, secondExpr) : IR.sheq(firstExpr, secondExpr);
callNode.getParent().replaceChild(callNode, replacement.useSourceInfoIfMissingFrom(callNode)); callNode.getParent().replaceChild(callNode, replacement.useSourceInfoIfMissingFrom(callNode));
compiler.reportCodeChange(); compiler.reportCodeChange();
} }


private Node asEqOperation(Node firstExpr, Node secondExpr) {
return (NodeUtil.isNullOrUndefined(firstExpr) || NodeUtil.isNullOrUndefined(secondExpr))
? IR.eq(firstExpr, secondExpr)
: IR.sheq(firstExpr, secondExpr);
}

private static boolean isEqualitySameCall(Node node) { private static boolean isEqualitySameCall(Node node) {
return node.isCall() && isEqualitySameMethodName(node.getFirstChild().getQualifiedName()); return node.isCall() && isEqualitySameMethodName(node.getFirstChild().getQualifiedName());
} }
Expand All @@ -73,4 +96,21 @@ private static boolean isEqualitySameMethodName(String fnName) {
return fnName != null return fnName != null
&& (fnName.endsWith("Equality$$0same") || fnName.endsWith("Equality.$same")); && (fnName.endsWith("Equality$$0same") || fnName.endsWith("Equality.$same"));
} }

private static JSType getTypeRestrictByNotNullOrUndefined(Node node) {
JSType jsType = node.getJSType();
return jsType == null ? null : jsType.restrictByNotNullOrUndefined();
}

private static boolean isObjectType(JSType jsType) {
return !isUnknownType(jsType) && jsType.isObject();
}

private static boolean sameType(JSType jsType1, JSType jsType2) {
return !isUnknownType(jsType1) && !isUnknownType(jsType2) && jsType1.equals(jsType2);
}

private static boolean isUnknownType(JSType jsType) {
return jsType == null || jsType.isUnknownType() || jsType.isNoType();
}
} }
73 changes: 62 additions & 11 deletions test/com/google/javascript/jscomp/J2clEqualitySameRewriterPassTest.java
View file
Expand Up @@ -16,6 +16,16 @@
package com.google.javascript.jscomp; package com.google.javascript.jscomp;


public class J2clEqualitySameRewriterPassTest extends CompilerTestCase { public class J2clEqualitySameRewriterPassTest extends CompilerTestCase {
private static final String EXTERN = "Equality.$same = function(a, b) {};";

public J2clEqualitySameRewriterPassTest() {
super(EXTERN);
}

@Override
public void setUp() {
enableTypeCheck();
}


@Override @Override
protected CompilerPass getProcessor(Compiler compiler) { protected CompilerPass getProcessor(Compiler compiler) {
Expand All @@ -25,14 +35,31 @@ protected CompilerPass getProcessor(Compiler compiler) {
public void testRewriteEqualitySame() { public void testRewriteEqualitySame() {
test( test(
LINE_JOINER.join( LINE_JOINER.join(
"Equality$$0same(0, '');", "Equality.$same(0, '');",
"var a = 'ABC';", "var a = 'ABC';",
"Equality$$0same(a, 'ABC');", "Equality.$same(a, 'ABC');",
"var b = 5;", "var b = 5;",
"Equality$$0same(b, 5);", "Equality.$same(b, 5);",
"Equality$$0same(b, []);", "Equality.$same(b, []);",
"Equality$$0same(b, null);", "Equality.$same(b, null);",
"Equality$$0same(null, b);"), "Equality.$same(null, b);",
"/** @type {number|undefined} */",
"var num1 = 5;",
"/** @type {?number} */",
"var num2 = 5;",
"Equality.$same(num1, num2);",
"/** @type {string} */",
"var str1 = '';",
"/** @type {string|undefined} */",
"var str2 = 'abc';",
"Equality.$same(str1, str2);",
"/** @type {!Object} */",
"var obj1 = {};",
"/** @type {Object} */",
"var obj2 = null;",
"Equality.$same(obj1, obj2);",
"Equality.$same(obj1, str2);",
"Equality.$same(obj1, num2);"),
LINE_JOINER.join( LINE_JOINER.join(
"0 === '';", "0 === '';",
"var a = 'ABC';", "var a = 'ABC';",
Expand All @@ -41,14 +68,38 @@ public void testRewriteEqualitySame() {
"b === 5;", "b === 5;",
"b === [];", "b === [];",
"b == null;", "b == null;",
"null == b;")); "null == b;",
"/** @type {number|undefined} */",
"var num1 = 5;",
"/** @type {?number} */",
"var num2 = 5;",
"num1 == num2;",
"/** @type {string} */",
"var str1 = '';",
"/** @type {string|undefined} */",
"var str2 = 'abc';",
"str1 == str2;",
"/** @type {!Object} */",
"var obj1 = {};",
"/** @type {Object} */",
"var obj2 = null;",
"obj1 == obj2;",
"obj1 == str2;",
"obj1 == num2;"));
} }


public void testRewriteEqualitySame_avoidNonLiterals() { public void testNotRewriteEqualitySame() {
testSame( testSame(
LINE_JOINER.join( LINE_JOINER.join(
"Equality$$0same(c, d);", "Equality.$same(c, d);",
"var a = 5, b = 5;", "/** @type {number} */",
"Equality$$0same(a, b);")); "var num = 5",
"/** @type {string} */",
"var str = 'ABC';",
"/** @type {*} */",
"var unknown = null;",
"Equality.$same(num, str);",
"Equality.$same(num, unknown);",
"Equality.$same(str, unknown);"));
} }
} }

0 comments on commit bb012f3

Please sign in to comment.