Skip to content

Commit

Permalink
Extend the peephole optimization to turn "obj == null" into "!obj":
Browse files Browse the repository at this point in the history 
- Use similar logic for numbers being compared to zero.
- Also do the conversion for arbitrary expressions.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=125973280
  • Loading branch information
@blickly
michaelthomas authored and blickly committed Jun 27, 2016
1 parent 48f4e47 commit a24991e
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 25 deletions.
106 changes: 87 additions & 19 deletions src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java
View file
Expand Up @@ -106,6 +106,12 @@ public Node optimizeSubtree(Node node) {
case BLOCK:
return tryReplaceIf(node);

case EQ:
case NE:
case SHEQ:
case SHNE:
return tryReplaceComparisonWithCoercion(node, true /* booleanResult */);

default:
return node; //Nothing changed
}
Expand Down Expand Up @@ -995,8 +1001,6 @@ private Node performCoercionSubstitutions(Node n) {
return n;
}

Node parent = n.getParent();

switch (n.getType()) {
case OR:
case AND:
Expand All @@ -1008,27 +1012,82 @@ private Node performCoercionSubstitutions(Node n) {
case NE:
case SHEQ:
case SHNE:
Node left = n.getFirstChild();
Node right = n.getLastChild();
boolean leftIsNull = NodeUtil.isNullOrUndefined(left);
boolean rightIsNull = NodeUtil.isNullOrUndefined(right);
boolean leftIsObject = isObjectType(left);
boolean rightIsObject = isObjectType(right);
if (leftIsObject && rightIsNull || rightIsObject && leftIsNull) {
n.detachChildren();
Node objExpression = leftIsObject ? left : right;
Node replacement = n.getType() == Token.EQ || n.getType() == Token.SHEQ
? IR.not(objExpression)
: objExpression;
parent.replaceChild(n, replacement);
reportCodeChange();
return replacement;
}
break;
return tryReplaceComparisonWithCoercion(n, false /* booleanResult */);
}
return n;
}

/**
* Replaces comparisons (e.g. obj == null, num == 0) with the equivalent type
* coercion (e.g. !obj, !num), if possible.
* @param n a comparison node
* @param booleanResult whether the replacement must evaluate to a boolean
* @return the replacement node or the original node if no replacement was made
*/
private Node tryReplaceComparisonWithCoercion(Node n, boolean booleanResult) {
if (!useTypes) {
return n;
}

Preconditions.checkArgument(
n.getType() == Token.EQ
|| n.getType() == Token.NE
|| n.getType() == Token.SHEQ
|| n.getType() == Token.SHNE);

Node left = n.getFirstChild();
Node right = n.getLastChild();
BooleanCoercability booleanCoercability = canConvertComparisonToBooleanCoercion(left, right);
if (booleanCoercability != BooleanCoercability.NONE) {
n.detachChildren();
Node objExpression = booleanCoercability == BooleanCoercability.LEFT ? left : right;
Node replacement;
if (n.getType() == Token.EQ || n.getType() == Token.SHEQ) {
replacement = IR.not(objExpression);
} else {
replacement = booleanResult ? IR.not(IR.not(objExpression)) : objExpression;
}
n.getParent().replaceChild(n, replacement);
reportCodeChange();
return replacement;
}
return n;
}

/**
* The ability of a comparison node to be converted to a coercion.
*/
private enum BooleanCoercability {
// Comparison cannot be converted to coercion.
NONE,
// Comparison can be converted to coercion of the left child.
LEFT,
// Comparison can be converted to coercion of the right child.
RIGHT
}

private static BooleanCoercability canConvertComparisonToBooleanCoercion(Node left, Node right) {
// Convert null check of an object to coercion.
boolean leftIsNull = NodeUtil.isNullOrUndefined(left);
boolean rightIsNull = NodeUtil.isNullOrUndefined(right);
boolean leftIsObjectType = isObjectType(left);
boolean rightIsObjectType = isObjectType(right);
if (leftIsObjectType && rightIsNull || rightIsObjectType && leftIsNull) {
return leftIsNull ? BooleanCoercability.RIGHT : BooleanCoercability.LEFT;
}

// Convert comparing a number to zero with coercion.
boolean leftIsZero = left.isNumber() && left.getDouble() == 0;
boolean rightIsZero = right.isNumber() && right.getDouble() == 0;
boolean leftIsNumberType = isNumberType(left);
boolean rightIsNumberType = isNumberType(right);
if (leftIsNumberType && rightIsZero || rightIsNumberType && leftIsZero) {
return leftIsZero ? BooleanCoercability.RIGHT : BooleanCoercability.LEFT;
}

return BooleanCoercability.NONE;
}

private static boolean isObjectType(Node n) {
JSType jsType = n.getJSType();
if (jsType == null) {
Expand All @@ -1038,6 +1097,15 @@ private static boolean isObjectType(Node n) {
return !jsType.isUnknownType() && !jsType.isNoType() && jsType.isObject();
}

private static boolean isNumberType(Node n) {
JSType jsType = n.getJSType();
if (jsType == null) {
return false;
}
// Don't restrict by nullable. Nullable numbers are not coercable.
return !jsType.isUnknownType() && !jsType.isNoType() && jsType.isNumberValueType();
}

private Node replaceNode(Node lhs, MinimizedCondition.MeasuredNode rhs) {
Node parent = lhs.getParent();
parent.replaceChild(lhs, rhs.getNode());
Expand Down
63 changes: 57 additions & 6 deletions test/com/google/javascript/jscomp/PeepholeMinimizeConditionsTest.java
View file
Expand Up @@ -742,7 +742,24 @@ public void testIssue925() {
public void testCoercionSubstitution_disabled() {
enableTypeCheck();
useTypes = false;
testSame("var x = {};\nif (x != null) throw 'a';\n");
testSame("var x = {}; if (x != null) throw 'a';");
testSame("var x = {}; var y = x != null;");

testSame("var x = 1; if (x != 0) throw 'a';");
testSame("var x = 1; var y = x != 0;");
}

public void testCoercionSubstitution_booleanResult() {
enableTypeCheck();
test("var x = {}; var y = x != null;", "var x = {}; var y = !!x;");
test("var x = {}; var y = x == null;", "var x = {}; var y = !x;");
test("var x = {}; var y = x !== null;", "var x = {}; var y = !!x;");
test("var x = {}; var y = x === null;", "var x = {}; var y = !x;");

test("var x = 1; var y = x != 0;", "var x = 1; var y = !!x;");
test("var x = 1; var y = x == 0;", "var x = 1; var y = !x;");
test("var x = 1; var y = x !== 0;", "var x = 1; var y = !!x;");
test("var x = 1; var y = x === 0;", "var x = 1; var y = !x;");
}

public void testCoercionSubstitution_if() {
Expand All @@ -755,56 +772,90 @@ public void testCoercionSubstitution_if() {
test("var x = {};\nif (null == x) throw 'a';\n", "var x = {};\nif (!x) throw 'a';\n");
test("var x = {};\nif (null !== x) throw 'a';\n", "var x = {};\nif (x) throw 'a';\n");
test("var x = {};\nif (null === x) throw 'a';\n", "var x = {};\nif (!x) throw 'a';\n");

test("var x = 1;\nif (x != 0) throw 'a';\n", "var x = 1;\nif (x) throw 'a';\n");
test("var x = 1;\nif (x == 0) throw 'a';\n", "var x = 1;\nif (!x) throw 'a';\n");
test("var x = 1;\nif (x !== 0) throw 'a';\n", "var x = 1;\nif (x) throw 'a';\n");
test("var x = 1;\nif (x === 0) throw 'a';\n", "var x = 1;\nif (!x) throw 'a';\n");
test("var x = 1;\nif (0 != x) throw 'a';\n", "var x = 1;\nif (x) throw 'a';\n");
test("var x = 1;\nif (0 == x) throw 'a';\n", "var x = 1;\nif (!x) throw 'a';\n");
test("var x = 1;\nif (0 !== x) throw 'a';\n", "var x = 1;\nif (x) throw 'a';\n");
test("var x = 1;\nif (0 === x) throw 'a';\n", "var x = 1;\nif (!x) throw 'a';\n");
}

public void testCoercionSubstitution_expression() {
enableTypeCheck();
test(
"var x = {}; x != null && alert('b');",
"var x = {}; x && alert('b');");
test(
"var x = 1; x != 0 && alert('b');",
"var x = 1; x && alert('b');");
}

public void testCoercionSubstitution_hook() {
enableTypeCheck();
test("var x = {};\nvar y = x != null ? 1 : 2;\n", "var x = {};\nvar y = x ? 1 : 2;\n");
test("var x = 1;\nvar y = x != 0 ? 1 : 2;\n", "var x = 1;\nvar y = x ? 1 : 2;\n");
}

public void testCoercionSubstitution_not() {
enableTypeCheck();
test("var x = {};\nvar y = !(x != null) ? 1 : 2;\n", "var x = {};\nvar y = x ? 2 : 1;\n");
test("var x = 1;\nvar y = !(x != 0) ? 1 : 2;\n", "var x = 1;\nvar y = x ? 2 : 1;\n");
}

public void testCoercionSubstitution_while() {
enableTypeCheck();
test("var x = {};\nwhile (x != null) throw 'a'\n", "var x = {};\nwhile (x) throw 'a';\n");
test("var x = 1;\nwhile (x != 0) throw 'a'\n", "var x = 1;\nwhile (x) throw 'a';\n");
}

public void testCoercionSubstitution_nullableType() {
enableTypeCheck();
test(
"var x = /** @type {?Object} */ ({}); if (x != null) throw 'a';",
"var x = /** @type {?Object} */ ({}); if (x) throw 'a';");
testSame("var x = /** @type {?number} */ (0); if (x != null) throw 'a';");
testSame("var x = /** @type {?number} */ (1); if (x != 0) throw 'a';");
testSame("var x = /** @type {?string} */ (''); if (x != null) throw 'a';");
testSame("var x = /** @type {?boolean} */ (true); if (x != null) throw 'a';");
}

public void testCoercionSubstitution_unknownType() {
enableTypeCheck();
testSame("var x = /** @type {?} */ ({});\nif (x != null) throw 'a';\n");
testSame("var x = /** @type {?} */ (1);\nif (x != 0) throw 'a';\n");
}

public void testCoercionSubstitution_primitives() {
public void testCoercionSubstitution_primitivesVsNull() {
enableTypeCheck();
testSame("var x = 0;\nif (x != null) throw 'a';\n");
testSame("var x = '';\nif (x != null) throw 'a';\n");
testSame("var x = false;\nif (x != null) throw 'a';\n");
}

public void testCoercionSubstitution_nonNumberVsZero() {
enableTypeCheck();
testSame("var x = {};\nif (x != 0) throw 'a';\n");
testSame("var x = '';\nif (x != 0) throw 'a';\n");
testSame("var x = false;\nif (x != 0) throw 'a';\n");
}

public void testCoercionSubstitution_boxedNumberVsZero() {
enableTypeCheck();
testSame("var x = new Number(0);\nif (x != 0) throw 'a';\n");
}

public void testCoercionSubstitution_boxedPrimitives() {
enableTypeCheck();
testSame("var x = new Number();\nif (x) throw 'a';\n");
testSame("var x = new String();\nif (x) throw 'a';\n");
testSame("var x = new Boolean();\nif (x) throw 'a';\n");
test(
"var x = new Number();\nif (x != null) throw 'a';\n",
"var x = new Number();\nif (x) throw 'a';\n");
test(
"var x = new String();\nif (x != null) throw 'a';\n",
"var x = new String();\nif (x) throw 'a';\n");
test(
"var x = new Boolean();\nif (x != null) throw 'a';\n",
"var x = new Boolean();\nif (x) throw 'a';\n");
}
}

0 comments on commit a24991e

Please sign in to comment.