Skip to content

Commit

Permalink
Remove FlowScope#optimize and FlowScope#createChildFlowScope.
Browse files Browse the repository at this point in the history
Also renames createChildFlowScope(TypedScope) to withSyntacticScope, since it no longer necessarily creates a child scope.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=197184414
  • Loading branch information
shicks authored and blickly committed May 18, 2018
1 parent 74d4eaa commit 9242695
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 198 deletions.
30 changes: 5 additions & 25 deletions src/com/google/javascript/jscomp/LinkedFlowScope.java
Expand Up @@ -204,26 +204,13 @@ public StaticTypedSlot getOwnSlot(String name) {
} }


@Override @Override
public FlowScope createChildFlowScope() { public FlowScope withSyntacticScope(StaticTypedScope scope) {
return this;
}

@Override
public FlowScope createChildFlowScope(StaticTypedScope scope) {
TypedScope typedScope = (TypedScope) scope; TypedScope typedScope = (TypedScope) scope;
return scope != syntacticScope return scope != syntacticScope
? new LinkedFlowScope(trimScopes(typedScope), typedScope, functionScope) ? new LinkedFlowScope(trimScopes(typedScope), typedScope, functionScope)
: this; : this;
} }


/**
* Remove flow scopes that add nothing to the flow.
*/
@Override
public LinkedFlowScope optimize() {
return this;
}

@Override @Override
public TypedScope getDeclarationScope() { public TypedScope getDeclarationScope() {
return syntacticScope; return syntacticScope;
Expand All @@ -247,7 +234,7 @@ public FlowScope apply(FlowScope a, FlowScope b) {
// To join the two scopes, we have to // To join the two scopes, we have to
LinkedFlowScope linkedA = (LinkedFlowScope) a; LinkedFlowScope linkedA = (LinkedFlowScope) a;
LinkedFlowScope linkedB = (LinkedFlowScope) b; LinkedFlowScope linkedB = (LinkedFlowScope) b;
if (linkedA == linkedB) { if (linkedA.scopes == linkedB.scopes && linkedA.functionScope == linkedB.functionScope) {
return linkedA; return linkedA;
} }


Expand Down Expand Up @@ -288,11 +275,7 @@ public boolean equals(Object other) {
if (!(other instanceof LinkedFlowScope)) { if (!(other instanceof LinkedFlowScope)) {
return false; return false;
} }

LinkedFlowScope that = (LinkedFlowScope) other; LinkedFlowScope that = (LinkedFlowScope) other;
if (this == that) {
return true;
}


// If two flow scopes are in the same function, then they could have // If two flow scopes are in the same function, then they could have
// two possible function scopes: the real one and the BOTTOM scope. // two possible function scopes: the real one and the BOTTOM scope.
Expand All @@ -302,11 +285,8 @@ public boolean equals(Object other) {
// they're equal--this just means that data flow analysis will have // they're equal--this just means that data flow analysis will have
// to propagate the entry lattice a little bit further than it // to propagate the entry lattice a little bit further than it
// really needs to. Everything will still come out ok. // really needs to. Everything will still come out ok.
if (this.functionScope != that.functionScope) { return this.functionScope == that.functionScope
return false; && this.scopes.equivalent(that.scopes, LinkedFlowScope::equalScopes);
}

return this.scopes.equivalent(that.scopes, LinkedFlowScope::equalScopes);
} }


private static boolean equalScopes(OverlayScope left, OverlayScope right) { private static boolean equalScopes(OverlayScope left, OverlayScope right) {
Expand All @@ -320,7 +300,7 @@ private static boolean equalScopes(OverlayScope left, OverlayScope right) {
* Determines whether two slots are meaningfully different for the purposes of data flow analysis. * Determines whether two slots are meaningfully different for the purposes of data flow analysis.
*/ */
private static boolean equalSlots(StaticTypedSlot slotA, StaticTypedSlot slotB) { private static boolean equalSlots(StaticTypedSlot slotA, StaticTypedSlot slotB) {
return !slotA.getType().differsFrom(slotB.getType()); return slotA == slotB || !slotA.getType().differsFrom(slotB.getType());
} }


@Override @Override
Expand Down
35 changes: 14 additions & 21 deletions src/com/google/javascript/jscomp/TypeInference.java
Expand Up @@ -215,7 +215,7 @@ FlowScope flowThrough(Node n, FlowScope input) {
} }


Node root = NodeUtil.getEnclosingScopeRoot(n); Node root = NodeUtil.getEnclosingScopeRoot(n);
FlowScope output = input.createChildFlowScope(scopeCreator.createScope(root)); FlowScope output = input.withSyntacticScope(scopeCreator.createScope(root));
output = inferDeclarativelyUnboundVarsWithoutTypes(output); output = inferDeclarativelyUnboundVarsWithoutTypes(output);
output = traverse(n, output); output = traverse(n, output);
return output; return output;
Expand Down Expand Up @@ -246,7 +246,7 @@ List<FlowScope> branchedFlowThrough(Node source, FlowScope input) {
Node item = source.getFirstChild(); Node item = source.getFirstChild();
Node obj = item.getNext(); Node obj = item.getNext();


FlowScope informed = traverse(obj, output.createChildFlowScope()); FlowScope informed = traverse(obj, output);


if (NodeUtil.isNameDeclaration(item)) { if (NodeUtil.isNameDeclaration(item)) {
item = item.getFirstChild(); item = item.getFirstChild();
Expand Down Expand Up @@ -294,8 +294,7 @@ List<FlowScope> branchedFlowThrough(Node source, FlowScope input) {
// conditionFlowScope is cached from previous iterations // conditionFlowScope is cached from previous iterations
// of the loop. // of the loop.
if (conditionFlowScope == null) { if (conditionFlowScope == null) {
conditionFlowScope = traverse( conditionFlowScope = traverse(condition.getFirstChild(), output);
condition.getFirstChild(), output.createChildFlowScope());
} }
} }
} }
Expand All @@ -319,8 +318,8 @@ List<FlowScope> branchedFlowThrough(Node source, FlowScope input) {
if (conditionOutcomes == null) { if (conditionOutcomes == null) {
conditionOutcomes = conditionOutcomes =
condition.isAnd() condition.isAnd()
? traverseAnd(condition, output.createChildFlowScope()) ? traverseAnd(condition, output)
: traverseOr(condition, output.createChildFlowScope()); : traverseOr(condition, output);
} }
newScope = newScope =
reverseInterpreter.getPreciserScopeKnowingConditionOutcome( reverseInterpreter.getPreciserScopeKnowingConditionOutcome(
Expand All @@ -332,8 +331,7 @@ List<FlowScope> branchedFlowThrough(Node source, FlowScope input) {
// conditionFlowScope is cached from previous iterations // conditionFlowScope is cached from previous iterations
// of the loop. // of the loop.
if (conditionFlowScope == null) { if (conditionFlowScope == null) {
conditionFlowScope = conditionFlowScope = traverse(condition, output);
traverse(condition, output.createChildFlowScope());
} }
newScope = newScope =
reverseInterpreter.getPreciserScopeKnowingConditionOutcome( reverseInterpreter.getPreciserScopeKnowingConditionOutcome(
Expand All @@ -345,7 +343,7 @@ List<FlowScope> branchedFlowThrough(Node source, FlowScope input) {
break; break;
} }


result.add(newScope.optimize()); result.add(newScope);
} }
return result; return result;
} }
Expand All @@ -365,13 +363,11 @@ private FlowScope traverse(Node n, FlowScope scope) {
break; break;


case AND: case AND:
scope = traverseAnd(n, scope).getJoinedFlowScope() scope = traverseAnd(n, scope).getJoinedFlowScope();
.createChildFlowScope();
break; break;


case OR: case OR:
scope = traverseOr(n, scope).getJoinedFlowScope() scope = traverseOr(n, scope).getJoinedFlowScope();
.createChildFlowScope();
break; break;


case HOOK: case HOOK:
Expand Down Expand Up @@ -1070,10 +1066,10 @@ private FlowScope traverseHook(Node n, FlowScope scope) {
condition, scope, false); condition, scope, false);


// traverse the true node with the trueScope // traverse the true node with the trueScope
traverse(trueNode, trueScope.createChildFlowScope()); traverse(trueNode, trueScope);


// traverse the false node with the falseScope // traverse the false node with the falseScope
traverse(falseNode, falseScope.createChildFlowScope()); traverse(falseNode, falseScope);


// meet true and false nodes' types and assign // meet true and false nodes' types and assign
JSType trueType = trueNode.getJSType(); JSType trueType = trueNode.getJSType();
Expand All @@ -1084,7 +1080,7 @@ private FlowScope traverseHook(Node n, FlowScope scope) {
n.setJSType(null); n.setJSType(null);
} }


return scope.createChildFlowScope(); return scope;
} }


/** @param n A non-constructor function invocation, i.e. CALL or TAGGED_TEMPLATELIT */ /** @param n A non-constructor function invocation, i.e. CALL or TAGGED_TEMPLATELIT */
Expand Down Expand Up @@ -1153,7 +1149,6 @@ private FlowScope narrowScope(FlowScope scope, Node node, JSType narrowed) {
return scope; return scope;
} }


scope = scope.createChildFlowScope();
if (node.isGetProp()) { if (node.isGetProp()) {
return scope.inferQualifiedSlot( return scope.inferQualifiedSlot(
node, node.getQualifiedName(), getJSType(node), narrowed, false); node, node.getQualifiedName(), getJSType(node), narrowed, false);
Expand Down Expand Up @@ -1800,8 +1795,7 @@ private BooleanOutcomePair traverseShortCircuitingBinOp(
Node right = n.getLastChild(); Node right = n.getLastChild();


// type the left node // type the left node
BooleanOutcomePair leftOutcome = traverseWithinShortCircuitingBinOp( BooleanOutcomePair leftOutcome = traverseWithinShortCircuitingBinOp(left, scope);
left, scope.createChildFlowScope());
JSType leftType = left.getJSType(); JSType leftType = left.getJSType();


// reverse abstract interpret the left node to produce the correct // reverse abstract interpret the left node to produce the correct
Expand All @@ -1811,8 +1805,7 @@ private BooleanOutcomePair traverseShortCircuitingBinOp(
left, leftOutcome.getOutcomeFlowScope(left.getToken(), nIsAnd), nIsAnd); left, leftOutcome.getOutcomeFlowScope(left.getToken(), nIsAnd), nIsAnd);


// type the right node // type the right node
BooleanOutcomePair rightOutcome = traverseWithinShortCircuitingBinOp( BooleanOutcomePair rightOutcome = traverseWithinShortCircuitingBinOp(right, rightScope);
right, rightScope.createChildFlowScope());
JSType rightType = right.getJSType(); JSType rightType = right.getJSType();


JSType type; JSType type;
Expand Down
Expand Up @@ -194,8 +194,7 @@ private FlowScope restrictParameter(


// changing the scope // changing the scope
if (type != null) { if (type != null) {
FlowScope informed = blindScope.createChildFlowScope(); return declareNameInScope(blindScope, parameter, type);
return declareNameInScope(informed, parameter, type);
} else { } else {
return blindScope; return blindScope;
} }
Expand Down
17 changes: 3 additions & 14 deletions src/com/google/javascript/jscomp/type/FlowScope.java
Expand Up @@ -33,16 +33,10 @@
public interface FlowScope extends StaticTypedScope, LatticeElement { public interface FlowScope extends StaticTypedScope, LatticeElement {


/** /**
* Creates a child of this flow scope, to represent an instruction * Returns a flow scope with the given syntactic scope, which may be required to be a specific
* directly following this one. * subclass, such as TypedScope.
*/ */
FlowScope createChildFlowScope(); FlowScope withSyntacticScope(StaticTypedScope scope);

/**
* Creates a child flow scope with the given syntactic scope, which may be required to be a
* specific subclass, such as TypedScope.
*/
FlowScope createChildFlowScope(StaticTypedScope scope);


/** /**
* Returns a flow scope with the type of the given {@code symbol} updated to {@code type}. * Returns a flow scope with the type of the given {@code symbol} updated to {@code type}.
Expand All @@ -67,11 +61,6 @@ public interface FlowScope extends StaticTypedScope, LatticeElement {
FlowScope inferQualifiedSlot( FlowScope inferQualifiedSlot(
Node node, String symbol, JSType bottomType, JSType inferredType, boolean declare); Node node, String symbol, JSType bottomType, JSType inferredType, boolean declare);


/**
* Optimize this scope and return a new FlowScope with faster lookup.
*/
FlowScope optimize();

/** Returns the underlying TypedScope. */ /** Returns the underlying TypedScope. */
StaticTypedScope getDeclarationScope(); StaticTypedScope getDeclarationScope();
} }
Expand Up @@ -368,7 +368,7 @@ private FlowScope caseAndOrMaybeShortCircuiting(
// If we did create a more precise scope, blindScope has a child and // If we did create a more precise scope, blindScope has a child and
// it is frozen. We can't just throw it away to return it. So we // it is frozen. We can't just throw it away to return it. So we
// must create a child instead. // must create a child instead.
return unwrap(blindScope == leftScope ? blindScope : blindScope.createChildFlowScope()); return unwrap(blindScope);
} }
refinements.clear(); refinements.clear();
// Note: re-wrap the scope, in case it was unwrapped by a nested call to this method. // Note: re-wrap the scope, in case it was unwrapped by a nested call to this method.
Expand All @@ -379,11 +379,10 @@ private FlowScope caseAndOrMaybeShortCircuiting(
StaticTypedSlot rightVar = StaticTypedSlot rightVar =
refinements.size() == 1 ? rightScope.getSlot(refinements.iterator().next()) : null; refinements.size() == 1 ? rightScope.getSlot(refinements.iterator().next()) : null;
if (rightVar == null || !leftVar.getName().equals(rightVar.getName())) { if (rightVar == null || !leftVar.getName().equals(rightVar.getName())) {
return unwrap(blindScope == rightScope ? blindScope : blindScope.createChildFlowScope()); return unwrap(blindScope);
} }
JSType type = leftVar.getType().getLeastSupertype(rightVar.getType()); JSType type = leftVar.getType().getLeastSupertype(rightVar.getType());
FlowScope informed = unwrap(blindScope).createChildFlowScope(); return unwrap(blindScope).inferSlotType(leftVar.getName(), type);
return informed.inferSlotType(leftVar.getName(), type);
} }


/** /**
Expand All @@ -403,8 +402,7 @@ private FlowScope caseAndOrMaybeShortCircuiting(
private FlowScope maybeRestrictName( private FlowScope maybeRestrictName(
FlowScope blindScope, Node node, JSType originalType, JSType restrictedType) { FlowScope blindScope, Node node, JSType originalType, JSType restrictedType) {
if (restrictedType != null && restrictedType != originalType) { if (restrictedType != null && restrictedType != originalType) {
FlowScope informed = blindScope.createChildFlowScope(); return declareNameInScope(blindScope, node, restrictedType);
return declareNameInScope(informed, node, restrictedType);
} }
return blindScope; return blindScope;
} }
Expand All @@ -424,7 +422,7 @@ private FlowScope maybeRestrictTwoNames(
boolean shouldRefineRight = boolean shouldRefineRight =
restrictedRightType != null && restrictedRightType != originalRightType; restrictedRightType != null && restrictedRightType != originalRightType;
if (shouldRefineLeft || shouldRefineRight) { if (shouldRefineLeft || shouldRefineRight) {
FlowScope informed = blindScope.createChildFlowScope(); FlowScope informed = blindScope;
if (shouldRefineLeft) { if (shouldRefineLeft) {
informed = declareNameInScope(informed, left, restrictedLeftType); informed = declareNameInScope(informed, left, restrictedLeftType);
} }
Expand Down Expand Up @@ -501,9 +499,8 @@ private FlowScope caseIn(Node object, String propertyName, FlowScope blindScope)
if (qualifiedName != null) { if (qualifiedName != null) {
String propertyQualifiedName = qualifiedName + "." + propertyName; String propertyQualifiedName = qualifiedName + "." + propertyName;
if (blindScope.getSlot(propertyQualifiedName) == null) { if (blindScope.getSlot(propertyQualifiedName) == null) {
FlowScope informed = blindScope.createChildFlowScope();
JSType unknownType = typeRegistry.getNativeType(JSTypeNative.UNKNOWN_TYPE); JSType unknownType = typeRegistry.getNativeType(JSTypeNative.UNKNOWN_TYPE);
return informed.inferQualifiedSlot( return blindScope.inferQualifiedSlot(
object, propertyQualifiedName, unknownType, unknownType, false); object, propertyQualifiedName, unknownType, unknownType, false);
} }
} }
Expand Down Expand Up @@ -630,13 +627,8 @@ private static class RefinementTrackingFlowScope implements FlowScope {
} }


@Override @Override
public FlowScope createChildFlowScope() { public FlowScope withSyntacticScope(StaticTypedScope scope) {
return new RefinementTrackingFlowScope(delegate.createChildFlowScope(), refinements); throw new UnsupportedOperationException();
}

@Override
public FlowScope createChildFlowScope(StaticTypedScope scope) {
return new RefinementTrackingFlowScope(delegate.createChildFlowScope(scope), refinements);
} }


@Override @Override
Expand All @@ -656,11 +648,6 @@ private FlowScope wrap(FlowScope scope) {
return scope != delegate ? new RefinementTrackingFlowScope(scope, refinements) : this; return scope != delegate ? new RefinementTrackingFlowScope(scope, refinements) : this;
} }


@Override
public FlowScope optimize() {
return wrap(delegate.optimize());
}

@Override @Override
public StaticTypedScope getDeclarationScope() { public StaticTypedScope getDeclarationScope() {
return delegate.getDeclarationScope(); return delegate.getDeclarationScope();
Expand Down

0 comments on commit 9242695

Please sign in to comment.