Skip to content

Commit

Permalink
[NTI] Convert PeepholeMinimizeConditions to TypeI.
Browse files Browse the repository at this point in the history
Unit tests still only run OTI, and will be updated in a future change.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=152748013
  • Loading branch information
shicks authored and brad4d committed Apr 11, 2017
1 parent c898dd7 commit e1d4af0
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 21 deletions.
57 changes: 36 additions & 21 deletions src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java
Expand Up @@ -22,7 +22,7 @@
import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token;
import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.TypeI;
import com.google.javascript.rhino.jstype.TernaryValue;

/**
Expand Down Expand Up @@ -1036,13 +1036,13 @@ private Node tryReplaceComparisonWithCoercion(Node n, boolean booleanResult) {
}

Token op = n.getToken();
Preconditions.checkArgument(
op == Token.EQ || op == Token.NE || op == Token.SHEQ || op == Token.SHNE);
boolean isShallow = op == Token.SHEQ || op == Token.SHNE;
Preconditions.checkArgument(op == Token.EQ || op == Token.NE || isShallow);

Node left = n.getFirstChild();
Node right = n.getLastChild();
BooleanCoercability booleanCoercability =
canConvertComparisonToBooleanCoercion(left, right, op);
canConvertComparisonToBooleanCoercion(left, right, isShallow);
if (booleanCoercability != BooleanCoercability.NONE) {
n.detachChildren();
Node objExpression = booleanCoercability == BooleanCoercability.LEFT ? left : right;
Expand Down Expand Up @@ -1071,8 +1071,14 @@ private enum BooleanCoercability {
RIGHT
}

/**
* Determines whether the types on either side of an (in)equality operation
* are amenable to converting to a simple truthiness check. Specifically,
* this is the case when comparing an object type against null or undefined,
* or when comparing a numeric type against zero.
*/
private static BooleanCoercability canConvertComparisonToBooleanCoercion(
Node left, Node right, Token op) {
Node left, Node right, boolean isShallow) {
// Convert null or undefined check of an object to coercion.
boolean leftIsNull = left.isNull();
boolean rightIsNull = right.isNull();
Expand All @@ -1083,9 +1089,10 @@ private static BooleanCoercability canConvertComparisonToBooleanCoercion(

boolean leftIsObjectType = isObjectType(left);
boolean rightIsObjectType = isObjectType(right);
if (op == Token.SHEQ || op == Token.SHNE) {
if ((leftIsObjectType && !left.getJSType().isNullable() && rightIsUndefined)
|| (rightIsObjectType && !right.getJSType().isNullable() && leftIsUndefined)) {
if (isShallow) {
// Shallow compare: must guarantee object cannot be the other type of null/undefined.
if ((leftIsObjectType && !left.getTypeI().isNullable() && rightIsUndefined)
|| (rightIsObjectType && !right.getTypeI().isNullable() && leftIsUndefined)) {
return leftIsNullOrUndefined ? BooleanCoercability.RIGHT : BooleanCoercability.LEFT;
}
} else {
Expand All @@ -1107,28 +1114,36 @@ private static BooleanCoercability canConvertComparisonToBooleanCoercion(
return BooleanCoercability.NONE;
}

/**
* Returns whether the node's type is an object type, which can be
* reduced to a truthiness check when compared against null or undefined.
*/
private static boolean isObjectType(Node n) {
JSType jsType = n.getJSType();
if (jsType == null) {
TypeI type = n.getTypeI();
if (type == null) {
return false;
}
jsType = jsType.restrictByNotNullOrUndefined();
return !jsType.isUnknownType()
&& !jsType.isNoType()
&& !jsType.isAllType()
&& jsType.isObject();
type = type.restrictByNotNullOrUndefined();
return !type.isUnknownType()
&& !type.isBottom()
&& !type.isTop()
&& type.isObjectType();
}

/**
* Returns whether the node's type is a numeric type, which can be reduced
* to a truthiness check when compared against zero.
*/
private static boolean isNumberType(Node n) {
JSType jsType = n.getJSType();
if (jsType == null) {
TypeI type = n.getTypeI();
if (type == null) {
return false;
}
// Don't restrict by nullable. Nullable numbers are not coercable.
return !jsType.isUnknownType()
&& !jsType.isNoType()
&& !jsType.isAllType()
&& jsType.isNumberValueType();
return !type.isUnknownType()
&& !type.isBottom()
&& !type.isTop()
&& type.isNumberValueType();
}

private Node replaceNode(Node lhs, MinimizedCondition.MeasuredNode rhs) {
Expand Down
5 changes: 5 additions & 0 deletions src/com/google/javascript/jscomp/newtypes/JSType.java
Expand Up @@ -336,6 +336,11 @@ public boolean isNumber() {
return NUMBER_MASK == getMask();
}

@Override
public boolean isNumberValueType() {
return isNumber();
}

public boolean isNullOrUndef() {
int nullUndefMask = NULL_MASK | UNDEFINED_MASK;
return getMask() != 0 && (getMask() | nullUndefMask) == nullUndefMask;
Expand Down
9 changes: 9 additions & 0 deletions src/com/google/javascript/rhino/TypeI.java
Expand Up @@ -109,12 +109,21 @@ public interface TypeI {

boolean isEnumElement();

// TODO(sdh): When OTI is gone, these can be renamed to simply isString and isNumber
// (provided we have sufficiently clear JavaDoc to specify that it does *not* include
// the object wrapper type).
/**
* Whether the type is a scalar string. In OTI, the isString method returns true for String
* objects as well.
*/
boolean isStringValueType();

/**
* Whether the type is a scalar number. In OTI, the isNumber method returns true for Number
* objects as well.
*/
boolean isNumberValueType();

ObjectTypeI autoboxAndGetObject();

JSDocInfo getJSDocInfo();
Expand Down
1 change: 1 addition & 0 deletions src/com/google/javascript/rhino/jstype/JSType.java
Expand Up @@ -186,6 +186,7 @@ public boolean isNumberObjectType() {
return false;
}

@Override
public boolean isNumberValueType() {
return false;
}
Expand Down

0 comments on commit e1d4af0

Please sign in to comment.