Skip to content

Commit

Permalink
Add @CheckReturnValue to FlowScope#infer* methods and fix the fallout.
Browse files Browse the repository at this point in the history 
This is a step toward making LinkedFlowScope immutable and removing the createChildFlowScope() and optimize() methods.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=197081848
  • Loading branch information
@shicks @blickly
shicks authored and blickly committed May 18, 2018
1 parent fa6801e commit 1695a44
Show file tree
Hide file tree
Showing 9 changed files with 239 additions and 205 deletions.
12 changes: 7 additions & 5 deletions src/com/google/javascript/jscomp/LinkedFlowScope.java
View file
Expand Up @@ -110,24 +110,25 @@ public static LinkedFlowScope createEntryLattice(TypedScope scope) {
}

@Override
public void inferSlotType(String symbol, JSType type) {
public LinkedFlowScope inferSlotType(String symbol, JSType type) {
checkState(!frozen);
OverlayScope scope = getOverlayScopeForVar(symbol, true);
OverlayScope newScope = scope.infer(symbol, type);
// Aggressively remove empty scopes to maintain a reasonable equivalence.
scopes =
!newScope.slots.isEmpty() ? scopes.plus(scope.scope, newScope) : scopes.minus(scope.scope);
return this;
}

@Override
public void inferQualifiedSlot(Node node, String symbol, JSType bottomType,
JSType inferredType, boolean declared) {
public LinkedFlowScope inferQualifiedSlot(
Node node, String symbol, JSType bottomType, JSType inferredType, boolean declared) {
if (functionScope.isGlobal()) {
// Do not infer qualified names on the global scope. Ideally these would be
// added to the scope by TypedScopeCreator, but if they are not, adding them
// here causes scaling problems (large projects can have tens of thousands of
// undeclared qualified names in the global scope) with no real benefit.
return;
return this;
}
TypedVar v = syntacticScope.getVar(symbol);
if (v == null && !functionScope.isBottom()) {
Expand All @@ -153,7 +154,7 @@ public void inferQualifiedSlot(Node node, String symbol, JSType bottomType,
|| !inferredType.isSubtypeOf(declaredType)
|| declaredType.isSubtypeOf(inferredType)
|| inferredType.isEquivalentTo(declaredType)) {
return;
return this;
}
} else if (declaredType != null && !inferredType.isSubtypeOf(declaredType)) {
// If this inferred type is incompatible with another type previously
Expand All @@ -162,6 +163,7 @@ public void inferQualifiedSlot(Node node, String symbol, JSType bottomType,
}
}
inferSlotType(symbol, inferredType);
return this;
}

@Override
Expand Down
87 changes: 47 additions & 40 deletions src/com/google/javascript/jscomp/TypeInference.java
View file
Expand Up @@ -67,6 +67,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import javax.annotation.CheckReturnValue;

/**
* Type inference within a script node or a function body, using the data-flow
Expand Down Expand Up @@ -110,21 +111,23 @@ class TypeInference
this.containerScope = syntacticScope;
inferArguments(syntacticScope);

this.functionScope = LinkedFlowScope.createEntryLattice(syntacticScope);
this.scopeCreator = scopeCreator;
this.assertionFunctionsMap = assertionFunctionsMap;

inferDeclarativelyUnboundVarsWithoutTypes(functionScope);
this.functionScope =
inferDeclarativelyUnboundVarsWithoutTypes(
LinkedFlowScope.createEntryLattice(syntacticScope));

this.bottomScope =
LinkedFlowScope.createEntryLattice(
TypedScope.createLatticeBottom(syntacticScope.getRootNode()));
}

private void inferDeclarativelyUnboundVarsWithoutTypes(FlowScope flow) {
@CheckReturnValue
private FlowScope inferDeclarativelyUnboundVarsWithoutTypes(FlowScope flow) {
TypedScope scope = (TypedScope) flow.getDeclarationScope();
if (!inferredUnboundVars.add(scope)) {
return;
return flow;
}
// For each local variable declared with the VAR keyword, the entry
// type is VOID.
Expand All @@ -133,8 +136,9 @@ private void inferDeclarativelyUnboundVarsWithoutTypes(FlowScope flow) {
continue;
}

flow.inferSlotType(var.getName(), getNativeType(VOID_TYPE));
flow = flow.inferSlotType(var.getName(), getNativeType(VOID_TYPE));
}
return flow;
}

/**
Expand Down Expand Up @@ -202,6 +206,7 @@ FlowScope createEntryLattice() {
}

@Override
@CheckReturnValue
FlowScope flowThrough(Node n, FlowScope input) {
// If we have not walked a path from <entry> to <n>, then we don't
// want to infer anything about this scope.
Expand All @@ -211,7 +216,7 @@ FlowScope flowThrough(Node n, FlowScope input) {

Node root = NodeUtil.getEnclosingScopeRoot(n);
FlowScope output = input.createChildFlowScope(scopeCreator.createScope(root));
inferDeclarativelyUnboundVarsWithoutTypes(output);
output = inferDeclarativelyUnboundVarsWithoutTypes(output);
output = traverse(n, output);
return output;
}
Expand Down Expand Up @@ -261,7 +266,7 @@ List<FlowScope> branchedFlowThrough(Node source, FlowScope input) {
iterKeyType = narrowedKeyType;
}
}
redeclareSimpleVar(informed, item, iterKeyType);
informed = redeclareSimpleVar(informed, item, iterKeyType);
}
} else {
// for/of. The type of `item` is the type parameter of the Iterable type.
Expand All @@ -272,7 +277,7 @@ List<FlowScope> branchedFlowThrough(Node source, FlowScope input) {
// Redeclare the loop var. Note that if we can't determine the type of the "object"
// we declare the var as the unknown type and let TypeCheck warn for using a non-
// Iterable in a for/of loop.
redeclareSimpleVar(informed, item, newType);
informed = redeclareSimpleVar(informed, item, newType);
}
newScope = informed;
break;
Expand Down Expand Up @@ -563,9 +568,8 @@ private void traverseSuper(Node superNode) {
}
}

/**
* Traverse a return value.
*/
/** Traverse a return value. */
@CheckReturnValue
private FlowScope traverseReturn(Node n, FlowScope scope) {
scope = traverseChildren(n, scope);

Expand All @@ -584,9 +588,10 @@ private FlowScope traverseReturn(Node n, FlowScope scope) {
}

/**
* Any value can be thrown, so it's really impossible to determine the type
* of a CATCH param. Treat it as the UNKNOWN type.
* Any value can be thrown, so it's really impossible to determine the type of a CATCH param.
* Treat it as the UNKNOWN type.
*/
@CheckReturnValue
private FlowScope traverseCatch(Node catchNode, FlowScope scope) {
Node name = catchNode.getFirstChild();
JSType type;
Expand All @@ -598,11 +603,11 @@ private FlowScope traverseCatch(Node catchNode, FlowScope scope) {
} else {
type = getNativeType(JSTypeNative.UNKNOWN_TYPE);
}
redeclareSimpleVar(scope, name, type);
name.setJSType(type);
return scope;
return redeclareSimpleVar(scope, name, type);
}

@CheckReturnValue
private FlowScope traverseAssign(Node n, FlowScope scope) {
Node target = n.getFirstChild();
Node value = n.getLastChild();
Expand All @@ -612,10 +617,10 @@ private FlowScope traverseAssign(Node n, FlowScope scope) {
JSType valueType = getJSType(value);
n.setJSType(valueType);

updateScopeForAssignment(scope, target, targetType, valueType);
return scope;
return updateScopeForAssignment(scope, target, targetType, valueType);
}

@CheckReturnValue
private FlowScope traverseAssignOp(Node n, FlowScope scope, JSType resultType) {
Node left = n.getFirstChild();
scope = traverseChildren(n, scope);
Expand All @@ -624,8 +629,7 @@ private FlowScope traverseAssignOp(Node n, FlowScope scope, JSType resultType) {
n.setJSType(resultType);

// The lhs is both an input and an output, so don't update the input type here.
updateScopeForAssignment(scope, left, leftType, resultType, null);
return scope;
return updateScopeForAssignment(scope, left, leftType, resultType, null);
}

private static boolean isInExternFile(Node n) {
Expand Down Expand Up @@ -664,13 +668,15 @@ private static void addMissingInterfaceProperties(JSType constructor) {
}
}

private void updateScopeForAssignment(
@CheckReturnValue
private FlowScope updateScopeForAssignment(
FlowScope scope, Node target, JSType targetType, JSType resultType) {
updateScopeForAssignment(scope, target, targetType, resultType, target);
return updateScopeForAssignment(scope, target, targetType, resultType, target);
}

/** Updates the scope according to the result of an assignment. */
private void updateScopeForAssignment(
@CheckReturnValue
private FlowScope updateScopeForAssignment(
FlowScope scope, Node target, JSType targetType, JSType resultType, Node updateNode) {
checkNotNull(resultType);
checkState(updateNode == null || updateNode == target);
Expand Down Expand Up @@ -731,9 +737,9 @@ private void updateScopeForAssignment(
// || !resultType.isSubtype(varType));

if (isVarTypeBetter) {
redeclareSimpleVar(scope, target, varType);
scope = redeclareSimpleVar(scope, target, varType);
} else {
redeclareSimpleVar(scope, target, resultType);
scope = redeclareSimpleVar(scope, target, resultType);
}

if (updateNode != null) {
Expand Down Expand Up @@ -769,8 +775,9 @@ private void updateScopeForAssignment(
}
}
JSType safeLeftType = targetType == null ? unknownType : targetType;
scope.inferQualifiedSlot(
target, qualifiedName, safeLeftType, resultType, declaredSlotType);
scope =
scope.inferQualifiedSlot(
target, qualifiedName, safeLeftType, resultType, declaredSlotType);
}

if (updateNode != null) {
Expand All @@ -781,6 +788,7 @@ private void updateScopeForAssignment(
default:
break;
}
return scope;
}

/** Defines a property if the property has not been defined yet. */
Expand Down Expand Up @@ -885,8 +893,8 @@ private FlowScope traverseName(Node n, FlowScope scope) {
JSType type = n.getJSType();
if (value != null) {
scope = traverse(value, scope);
updateScopeForAssignment(scope, n, n.getJSType() /* could be null */, getJSType(value));
return scope;
return updateScopeForAssignment(
scope, n, n.getJSType() /* could be null */, getJSType(value));
} else if (n.getParent().isLet()) {
// Whenever we see a LET, we're guaranteed it's not yet in the scope, and we don't need to
// worry about it being from an outer scope. In this case, it has no child, so the actual
Expand All @@ -895,7 +903,7 @@ private FlowScope traverseName(Node n, FlowScope scope) {
// TODO(sdh): I would have thought that #updateScopeForTypeChange would handle using the
// declared type correctly, but for some reason it doesn't so we handle it here.
JSType resultType = type != null ? type : getNativeType(VOID_TYPE);
updateScopeForAssignment(scope, n, type, resultType);
scope = updateScopeForAssignment(scope, n, type, resultType);
type = resultType;
} else {
StaticTypedSlot var = scope.getSlot(varName);
Expand Down Expand Up @@ -992,9 +1000,9 @@ private FlowScope traverseObjectLiteral(Node n, FlowScope scope) {
var.setType(oldType == null ? valueType : oldType.getLeastSupertype(oldType));
}

scope.inferQualifiedSlot(name, qKeyName,
oldType == null ? unknownType : oldType,
valueType, false);
scope =
scope.inferQualifiedSlot(
name, qKeyName, oldType == null ? unknownType : oldType, valueType, false);
}
} else {
n.setJSType(unknownType);
Expand Down Expand Up @@ -1034,7 +1042,7 @@ private FlowScope traverseAdd(Node n, FlowScope scope) {
// TODO(johnlenz): this should not update the type of the lhs as that is use as a
// input and need to be preserved for type checking.
// Instead call this overload `updateScopeForAssignment(scope, left, leftType, type, null);`
updateScopeForAssignment(scope, left, leftType, type);
scope = updateScopeForAssignment(scope, left, leftType, type);
}

return scope;
Expand Down Expand Up @@ -1147,12 +1155,10 @@ private FlowScope narrowScope(FlowScope scope, Node node, JSType narrowed) {

scope = scope.createChildFlowScope();
if (node.isGetProp()) {
scope.inferQualifiedSlot(
return scope.inferQualifiedSlot(
node, node.getQualifiedName(), getJSType(node), narrowed, false);
} else {
redeclareSimpleVar(scope, node, narrowed);
}
return scope;
return redeclareSimpleVar(scope, node, narrowed);
}

/**
Expand Down Expand Up @@ -1946,16 +1952,17 @@ private BooleanOutcomePair newBooleanOutcomePair(
flowScope);
}

private void redeclareSimpleVar(FlowScope scope, Node nameNode, JSType varType) {
@CheckReturnValue
private FlowScope redeclareSimpleVar(FlowScope scope, Node nameNode, JSType varType) {
checkState(nameNode.isName(), nameNode);
String varName = nameNode.getString();
if (varType == null) {
varType = getNativeType(JSTypeNative.UNKNOWN_TYPE);
}
if (isUnflowable(getDeclaredVar(scope, varName))) {
return;
return scope;
}
scope.inferSlotType(varName, varType);
return scope.inferSlotType(varName, varType);
}

private boolean isUnflowable(TypedVar v) {
Expand Down
19 changes: 10 additions & 9 deletions src/com/google/javascript/jscomp/type/ChainableReverseAbstractInterpreter.java
View file
Expand Up @@ -47,6 +47,7 @@
import com.google.javascript.rhino.jstype.TemplatizedType;
import com.google.javascript.rhino.jstype.UnionType;
import com.google.javascript.rhino.jstype.Visitor;
import javax.annotation.CheckReturnValue;

/**
* Chainable reverse abstract interpreter providing basic functionality.
Expand Down Expand Up @@ -152,28 +153,28 @@ protected JSType getTypeIfRefinable(Node node, FlowScope scope) {
}

/**
* Declares a refined type in {@code scope} for the name represented by
* {@code node}. It must be possible to refine the type of the given node in
* the given scope, as determined by {@link #getTypeIfRefinable}.
* Declares a refined type in {@code scope} for the name represented by {@code node}. It must be
* possible to refine the type of the given node in the given scope, as determined by {@link
* #getTypeIfRefinable}. Returns an updated flow scope, which may be different from the passed-in
* scope if any changes occur.
*/
protected void declareNameInScope(FlowScope scope, Node node, JSType type) {
@CheckReturnValue
protected FlowScope declareNameInScope(FlowScope scope, Node node, JSType type) {
switch (node.getToken()) {
case NAME:
scope.inferSlotType(node.getString(), type);
break;
return scope.inferSlotType(node.getString(), type);

case GETPROP:
String qualifiedName = node.getQualifiedName();
checkNotNull(qualifiedName);

JSType origType = node.getJSType();
origType = origType == null ? getNativeType(UNKNOWN_TYPE) : origType;
scope.inferQualifiedSlot(node, qualifiedName, origType, type, false);
break;
return scope.inferQualifiedSlot(node, qualifiedName, origType, type, false);

case THIS:
// "this" references aren't currently modeled in the CFG.
break;
return scope;

default:
throw new IllegalArgumentException("Node cannot be refined. \n" +
Expand Down
12 changes: 8 additions & 4 deletions src/com/google/javascript/jscomp/type/ClosureReverseAbstractInterpreter.java
View file
Expand Up @@ -33,6 +33,7 @@
import com.google.javascript.rhino.jstype.ObjectType;
import com.google.javascript.rhino.jstype.Visitor;
import java.util.Map;
import javax.annotation.CheckReturnValue;

/**
* A reverse abstract interpreter (RAI) for specific closure patterns such as
Expand Down Expand Up @@ -181,17 +182,20 @@ public FlowScope getPreciserScopeKnowingConditionOutcome(Node condition,
condition, blindScope, outcome);
}

private FlowScope restrictParameter(Node parameter, JSType type,
FlowScope blindScope, Function<TypeRestriction, JSType> restriction,
@CheckReturnValue
private FlowScope restrictParameter(
Node parameter,
JSType type,
FlowScope blindScope,
Function<TypeRestriction, JSType> restriction,
boolean outcome) {
// restricting
type = restriction.apply(new TypeRestriction(type, outcome));

// changing the scope
if (type != null) {
FlowScope informed = blindScope.createChildFlowScope();
declareNameInScope(informed, parameter, type);
return informed;
return declareNameInScope(informed, parameter, type);
} else {
return blindScope;
}
Expand Down

0 comments on commit 1695a44

Please sign in to comment.