Skip to content

Commit

Permalink
[NTI] Don't specialize types inside a cast.
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=175982203
  • Loading branch information
dimvar authored and Tyler Breisacher committed Nov 17, 2017
1 parent a0e903c commit d2109de
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 31 deletions.
48 changes: 18 additions & 30 deletions src/com/google/javascript/jscomp/NewTypeInference.java
Expand Up @@ -1640,7 +1640,7 @@ private EnvTypePair analyzeExprFwd(
resultPair = analyzeArrayLitFwd(expr, inEnv);
break;
case CAST:
resultPair = analyzeCastFwd(expr, inEnv, specializedType);
resultPair = analyzeCastFwd(expr, inEnv);
break;
case CASE:
// For a statement of the form: switch (exp1) { ... case exp2: ... }
Expand Down Expand Up @@ -1780,18 +1780,19 @@ private JSType pickFallbackTypeAfterBottom(

private EnvTypePair analyzeLogicalOpFwd(
Node expr, TypeEnv inEnv, JSType requiredType, JSType specializedType) {
Token exprKind = expr.getToken();
Token logicalOp = expr.getToken();
Node lhs = expr.getFirstChild();
Node rhs = expr.getLastChild();
if ((specializedType.isTrueOrTruthy() && exprKind == Token.AND)
|| (specializedType.isFalseOrFalsy() && exprKind == Token.OR)) {
if ((specializedType.isTrueOrTruthy() && logicalOp == Token.AND)
|| (specializedType.isFalseOrFalsy() && logicalOp == Token.OR)) {
EnvTypePair lhsPair =
analyzeExprFwd(lhs, inEnv, UNKNOWN, specializedType);
EnvTypePair rhsPair =
analyzeExprFwd(rhs, lhsPair.env, UNKNOWN, specializedType);
return rhsPair;
} else if ((specializedType.isFalseOrFalsy() && exprKind == Token.AND)
|| (specializedType.isTrueOrTruthy() && exprKind == Token.OR)) {
}
if ((specializedType.isFalseOrFalsy() && logicalOp == Token.AND)
|| (specializedType.isTrueOrTruthy() && logicalOp == Token.OR)) {
EnvTypePair shortCircuitPair =
analyzeExprFwd(lhs, inEnv, UNKNOWN, specializedType);
EnvTypePair lhsPair = analyzeExprFwd(
Expand All @@ -1800,21 +1801,16 @@ private EnvTypePair analyzeLogicalOpFwd(
analyzeExprFwd(rhs, lhsPair.env, UNKNOWN, specializedType);
JSType lhsUnspecializedType = JSType.join(shortCircuitPair.type, lhsPair.type);
return combineLhsAndRhsForLogicalOps(
exprKind, lhsUnspecializedType, shortCircuitPair, rhsPair);
} else {
// Independently of the specializedType, && rhs is only analyzed when
// lhs is truthy, and || rhs is only analyzed when lhs is falsy.
JSType stopAfterLhsType = exprKind == Token.AND ? FALSY : TRUTHY;
EnvTypePair shortCircuitPair =
analyzeExprFwd(lhs, inEnv, UNKNOWN, stopAfterLhsType);
EnvTypePair lhsPair = analyzeExprFwd(
lhs, inEnv, UNKNOWN, stopAfterLhsType.negate());
EnvTypePair rhsPair =
analyzeExprFwd(rhs, lhsPair.env, requiredType, specializedType);
JSType lhsUnspecializedType = JSType.join(shortCircuitPair.type, lhsPair.type);
return combineLhsAndRhsForLogicalOps(
exprKind, lhsUnspecializedType, shortCircuitPair, rhsPair);
logicalOp, lhsUnspecializedType, shortCircuitPair, rhsPair);
}
// Independently of the specializedType, && rhs is only analyzed when
// lhs is truthy, and || rhs is only analyzed when lhs is falsy.
JSType stopAfterLhsType = logicalOp == Token.AND ? FALSY : TRUTHY;
EnvTypePair shortCircuitPair = analyzeExprFwd(lhs, inEnv, UNKNOWN, stopAfterLhsType);
EnvTypePair lhsPair = analyzeExprFwd(lhs, inEnv, UNKNOWN, stopAfterLhsType.negate());
EnvTypePair rhsPair = analyzeExprFwd(rhs, lhsPair.env, requiredType, specializedType);
JSType lhsType = JSType.join(shortCircuitPair.type, lhsPair.type);
return combineLhsAndRhsForLogicalOps(logicalOp, lhsType, shortCircuitPair, rhsPair);
}

private EnvTypePair combineLhsAndRhsForLogicalOps(Token logicalOp,
Expand Down Expand Up @@ -2565,17 +2561,9 @@ private EnvTypePair analyzeArrayLitFwd(Node expr, TypeEnv inEnv) {
return new EnvTypePair(env, commonTypes.getArrayInstance(elementType));
}

// Because of the cast, expr doesn't need to have the required type of the context.
// However, we still pass along the specialized type, to specialize types when using
// logical operators.
private EnvTypePair analyzeCastFwd(Node expr, TypeEnv inEnv, JSType specializedType) {
Node parent = expr.getParent();
JSType newSpecType = this.commonTypes.UNKNOWN;
if ((parent.isOr() || parent.isAnd()) && expr == parent.getFirstChild()) {
newSpecType = specializedType;
}
private EnvTypePair analyzeCastFwd(Node expr, TypeEnv inEnv) {
Node insideCast = expr.getFirstChild();
EnvTypePair pair = analyzeExprFwd(insideCast, inEnv, this.commonTypes.UNKNOWN, newSpecType);
EnvTypePair pair = analyzeExprFwd(insideCast, inEnv);
JSType fromType = pair.type;
JSType toType = (JSType) expr.getTypeI();
if (!fromType.isInterfaceInstance()
Expand Down
17 changes: 16 additions & 1 deletion test/com/google/javascript/jscomp/NewTypeInferenceTest.java
Expand Up @@ -19397,7 +19397,7 @@ public void testBivariantArrayGenericsInCompatibilityMode() {
"}"));
}

public void testMaybeSpecializeInCast() {
public void testDontSpecializeInCast() {
// In this case, we're explicitly using the cast to "protect" arr from the
// context, so it's not correct to infer that arr has type !Array<!Sub>
// just because g takes a !Array<!Sub>.
Expand All @@ -19421,10 +19421,25 @@ public void testMaybeSpecializeInCast() {
"}"),
NewTypeInference.RETURN_NONDECLARED_TYPE);

// The cast prevents x from being specialized to !Object
typeCheck(LINE_JOINER.join(
"function foo(/** ?Object */ x) {",
" var /** !Object */ n;",
" return /** @type {*} */ (x) && (n = x);",
"}"),
NewTypeInference.MISTYPED_ASSIGN_RHS);

// Tests that when we analyze the lhs of the OR as falsy, we don't specialize the call to
// googObjectGet as falsy.
typeCheck(LINE_JOINER.join(
"/**",
" * @param {!IObject<?,V>} obj",
" * @return {V|undefined}",
" * @template V",
" */",
"function googObjectGet(obj) {}",
"function f(/** !Object */ x) {",
" return /** @type {!Array} */ (googObjectGet(x)) || null;",
"}"));
}

Expand Down

0 comments on commit d2109de

Please sign in to comment.