diff --git a/src/com/google/javascript/jscomp/LinkedFlowScope.java b/src/com/google/javascript/jscomp/LinkedFlowScope.java index 6e279bd9c4a..6dfbaee8dc8 100644 --- a/src/com/google/javascript/jscomp/LinkedFlowScope.java +++ b/src/com/google/javascript/jscomp/LinkedFlowScope.java @@ -204,26 +204,13 @@ public StaticTypedSlot getOwnSlot(String name) { } @Override - public FlowScope createChildFlowScope() { - return this; - } - - @Override - public FlowScope createChildFlowScope(StaticTypedScope scope) { + public FlowScope withSyntacticScope(StaticTypedScope scope) { TypedScope typedScope = (TypedScope) scope; return scope != syntacticScope ? new LinkedFlowScope(trimScopes(typedScope), typedScope, functionScope) : this; } - /** - * Remove flow scopes that add nothing to the flow. - */ - @Override - public LinkedFlowScope optimize() { - return this; - } - @Override public TypedScope getDeclarationScope() { return syntacticScope; @@ -247,7 +234,7 @@ public FlowScope apply(FlowScope a, FlowScope b) { // To join the two scopes, we have to LinkedFlowScope linkedA = (LinkedFlowScope) a; LinkedFlowScope linkedB = (LinkedFlowScope) b; - if (linkedA == linkedB) { + if (linkedA.scopes == linkedB.scopes && linkedA.functionScope == linkedB.functionScope) { return linkedA; } @@ -288,11 +275,7 @@ public boolean equals(Object other) { if (!(other instanceof LinkedFlowScope)) { return false; } - LinkedFlowScope that = (LinkedFlowScope) other; - if (this == that) { - return true; - } // If two flow scopes are in the same function, then they could have // two possible function scopes: the real one and the BOTTOM scope. @@ -302,11 +285,8 @@ public boolean equals(Object other) { // they're equal--this just means that data flow analysis will have // to propagate the entry lattice a little bit further than it // really needs to. Everything will still come out ok. - if (this.functionScope != that.functionScope) { - return false; - } - - return this.scopes.equivalent(that.scopes, LinkedFlowScope::equalScopes); + return this.functionScope == that.functionScope + && this.scopes.equivalent(that.scopes, LinkedFlowScope::equalScopes); } private static boolean equalScopes(OverlayScope left, OverlayScope right) { @@ -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. */ private static boolean equalSlots(StaticTypedSlot slotA, StaticTypedSlot slotB) { - return !slotA.getType().differsFrom(slotB.getType()); + return slotA == slotB || !slotA.getType().differsFrom(slotB.getType()); } @Override diff --git a/src/com/google/javascript/jscomp/TypeInference.java b/src/com/google/javascript/jscomp/TypeInference.java index 6a78d83ba6a..c508eb73a4e 100644 --- a/src/com/google/javascript/jscomp/TypeInference.java +++ b/src/com/google/javascript/jscomp/TypeInference.java @@ -215,7 +215,7 @@ FlowScope flowThrough(Node n, FlowScope input) { } Node root = NodeUtil.getEnclosingScopeRoot(n); - FlowScope output = input.createChildFlowScope(scopeCreator.createScope(root)); + FlowScope output = input.withSyntacticScope(scopeCreator.createScope(root)); output = inferDeclarativelyUnboundVarsWithoutTypes(output); output = traverse(n, output); return output; @@ -246,7 +246,7 @@ List branchedFlowThrough(Node source, FlowScope input) { Node item = source.getFirstChild(); Node obj = item.getNext(); - FlowScope informed = traverse(obj, output.createChildFlowScope()); + FlowScope informed = traverse(obj, output); if (NodeUtil.isNameDeclaration(item)) { item = item.getFirstChild(); @@ -294,8 +294,7 @@ List branchedFlowThrough(Node source, FlowScope input) { // conditionFlowScope is cached from previous iterations // of the loop. if (conditionFlowScope == null) { - conditionFlowScope = traverse( - condition.getFirstChild(), output.createChildFlowScope()); + conditionFlowScope = traverse(condition.getFirstChild(), output); } } } @@ -319,8 +318,8 @@ List branchedFlowThrough(Node source, FlowScope input) { if (conditionOutcomes == null) { conditionOutcomes = condition.isAnd() - ? traverseAnd(condition, output.createChildFlowScope()) - : traverseOr(condition, output.createChildFlowScope()); + ? traverseAnd(condition, output) + : traverseOr(condition, output); } newScope = reverseInterpreter.getPreciserScopeKnowingConditionOutcome( @@ -332,8 +331,7 @@ List branchedFlowThrough(Node source, FlowScope input) { // conditionFlowScope is cached from previous iterations // of the loop. if (conditionFlowScope == null) { - conditionFlowScope = - traverse(condition, output.createChildFlowScope()); + conditionFlowScope = traverse(condition, output); } newScope = reverseInterpreter.getPreciserScopeKnowingConditionOutcome( @@ -345,7 +343,7 @@ List branchedFlowThrough(Node source, FlowScope input) { break; } - result.add(newScope.optimize()); + result.add(newScope); } return result; } @@ -365,13 +363,11 @@ private FlowScope traverse(Node n, FlowScope scope) { break; case AND: - scope = traverseAnd(n, scope).getJoinedFlowScope() - .createChildFlowScope(); + scope = traverseAnd(n, scope).getJoinedFlowScope(); break; case OR: - scope = traverseOr(n, scope).getJoinedFlowScope() - .createChildFlowScope(); + scope = traverseOr(n, scope).getJoinedFlowScope(); break; case HOOK: @@ -1070,10 +1066,10 @@ private FlowScope traverseHook(Node n, FlowScope scope) { condition, scope, false); // traverse the true node with the trueScope - traverse(trueNode, trueScope.createChildFlowScope()); + traverse(trueNode, trueScope); // traverse the false node with the falseScope - traverse(falseNode, falseScope.createChildFlowScope()); + traverse(falseNode, falseScope); // meet true and false nodes' types and assign JSType trueType = trueNode.getJSType(); @@ -1084,7 +1080,7 @@ private FlowScope traverseHook(Node n, FlowScope scope) { n.setJSType(null); } - return scope.createChildFlowScope(); + return scope; } /** @param n A non-constructor function invocation, i.e. CALL or TAGGED_TEMPLATELIT */ @@ -1153,7 +1149,6 @@ private FlowScope narrowScope(FlowScope scope, Node node, JSType narrowed) { return scope; } - scope = scope.createChildFlowScope(); if (node.isGetProp()) { return scope.inferQualifiedSlot( node, node.getQualifiedName(), getJSType(node), narrowed, false); @@ -1800,8 +1795,7 @@ private BooleanOutcomePair traverseShortCircuitingBinOp( Node right = n.getLastChild(); // type the left node - BooleanOutcomePair leftOutcome = traverseWithinShortCircuitingBinOp( - left, scope.createChildFlowScope()); + BooleanOutcomePair leftOutcome = traverseWithinShortCircuitingBinOp(left, scope); JSType leftType = left.getJSType(); // reverse abstract interpret the left node to produce the correct @@ -1811,8 +1805,7 @@ private BooleanOutcomePair traverseShortCircuitingBinOp( left, leftOutcome.getOutcomeFlowScope(left.getToken(), nIsAnd), nIsAnd); // type the right node - BooleanOutcomePair rightOutcome = traverseWithinShortCircuitingBinOp( - right, rightScope.createChildFlowScope()); + BooleanOutcomePair rightOutcome = traverseWithinShortCircuitingBinOp(right, rightScope); JSType rightType = right.getJSType(); JSType type; diff --git a/src/com/google/javascript/jscomp/type/ClosureReverseAbstractInterpreter.java b/src/com/google/javascript/jscomp/type/ClosureReverseAbstractInterpreter.java index f9783d85188..d40fa462abe 100644 --- a/src/com/google/javascript/jscomp/type/ClosureReverseAbstractInterpreter.java +++ b/src/com/google/javascript/jscomp/type/ClosureReverseAbstractInterpreter.java @@ -194,8 +194,7 @@ private FlowScope restrictParameter( // changing the scope if (type != null) { - FlowScope informed = blindScope.createChildFlowScope(); - return declareNameInScope(informed, parameter, type); + return declareNameInScope(blindScope, parameter, type); } else { return blindScope; } diff --git a/src/com/google/javascript/jscomp/type/FlowScope.java b/src/com/google/javascript/jscomp/type/FlowScope.java index cdffd9c92f3..9a524da6546 100644 --- a/src/com/google/javascript/jscomp/type/FlowScope.java +++ b/src/com/google/javascript/jscomp/type/FlowScope.java @@ -33,16 +33,10 @@ public interface FlowScope extends StaticTypedScope, LatticeElement { /** - * Creates a child of this flow scope, to represent an instruction - * directly following this one. + * Returns a flow scope with the given syntactic scope, which may be required to be a specific + * subclass, such as TypedScope. */ - FlowScope createChildFlowScope(); - - /** - * 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); + FlowScope withSyntacticScope(StaticTypedScope scope); /** * Returns a flow scope with the type of the given {@code symbol} updated to {@code type}. @@ -67,11 +61,6 @@ public interface FlowScope extends StaticTypedScope, LatticeElement { FlowScope inferQualifiedSlot( 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. */ StaticTypedScope getDeclarationScope(); } diff --git a/src/com/google/javascript/jscomp/type/SemanticReverseAbstractInterpreter.java b/src/com/google/javascript/jscomp/type/SemanticReverseAbstractInterpreter.java index cbb3f545408..ddc664f99fd 100644 --- a/src/com/google/javascript/jscomp/type/SemanticReverseAbstractInterpreter.java +++ b/src/com/google/javascript/jscomp/type/SemanticReverseAbstractInterpreter.java @@ -368,7 +368,7 @@ private FlowScope caseAndOrMaybeShortCircuiting( // 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 // must create a child instead. - return unwrap(blindScope == leftScope ? blindScope : blindScope.createChildFlowScope()); + return unwrap(blindScope); } refinements.clear(); // Note: re-wrap the scope, in case it was unwrapped by a nested call to this method. @@ -379,11 +379,10 @@ private FlowScope caseAndOrMaybeShortCircuiting( StaticTypedSlot rightVar = refinements.size() == 1 ? rightScope.getSlot(refinements.iterator().next()) : null; 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()); - FlowScope informed = unwrap(blindScope).createChildFlowScope(); - return informed.inferSlotType(leftVar.getName(), type); + return unwrap(blindScope).inferSlotType(leftVar.getName(), type); } /** @@ -403,8 +402,7 @@ private FlowScope caseAndOrMaybeShortCircuiting( private FlowScope maybeRestrictName( FlowScope blindScope, Node node, JSType originalType, JSType restrictedType) { if (restrictedType != null && restrictedType != originalType) { - FlowScope informed = blindScope.createChildFlowScope(); - return declareNameInScope(informed, node, restrictedType); + return declareNameInScope(blindScope, node, restrictedType); } return blindScope; } @@ -424,7 +422,7 @@ private FlowScope maybeRestrictTwoNames( boolean shouldRefineRight = restrictedRightType != null && restrictedRightType != originalRightType; if (shouldRefineLeft || shouldRefineRight) { - FlowScope informed = blindScope.createChildFlowScope(); + FlowScope informed = blindScope; if (shouldRefineLeft) { informed = declareNameInScope(informed, left, restrictedLeftType); } @@ -501,9 +499,8 @@ private FlowScope caseIn(Node object, String propertyName, FlowScope blindScope) if (qualifiedName != null) { String propertyQualifiedName = qualifiedName + "." + propertyName; if (blindScope.getSlot(propertyQualifiedName) == null) { - FlowScope informed = blindScope.createChildFlowScope(); JSType unknownType = typeRegistry.getNativeType(JSTypeNative.UNKNOWN_TYPE); - return informed.inferQualifiedSlot( + return blindScope.inferQualifiedSlot( object, propertyQualifiedName, unknownType, unknownType, false); } } @@ -630,13 +627,8 @@ private static class RefinementTrackingFlowScope implements FlowScope { } @Override - public FlowScope createChildFlowScope() { - return new RefinementTrackingFlowScope(delegate.createChildFlowScope(), refinements); - } - - @Override - public FlowScope createChildFlowScope(StaticTypedScope scope) { - return new RefinementTrackingFlowScope(delegate.createChildFlowScope(scope), refinements); + public FlowScope withSyntacticScope(StaticTypedScope scope) { + throw new UnsupportedOperationException(); } @Override @@ -656,11 +648,6 @@ private FlowScope wrap(FlowScope scope) { return scope != delegate ? new RefinementTrackingFlowScope(scope, refinements) : this; } - @Override - public FlowScope optimize() { - return wrap(delegate.optimize()); - } - @Override public StaticTypedScope getDeclarationScope() { return delegate.getDeclarationScope(); diff --git a/test/com/google/javascript/jscomp/LinkedFlowScopeTest.java b/test/com/google/javascript/jscomp/LinkedFlowScopeTest.java index c45ee5a1220..08306451fbd 100644 --- a/test/com/google/javascript/jscomp/LinkedFlowScopeTest.java +++ b/test/com/google/javascript/jscomp/LinkedFlowScopeTest.java @@ -55,25 +55,10 @@ public void setUp() throws Exception { localEntry = LinkedFlowScope.createEntryLattice(localScope); } - public void testOptimize() { - assertEquals(localEntry, localEntry.optimize()); - - FlowScope child = localEntry.createChildFlowScope(); - assertEquals(localEntry, child.optimize()); - - child = child.inferSlotType("localB", getNativeNumberType()); - assertEquals(child, child.optimize()); - } - public void testJoin1() { - FlowScope childA = localEntry.createChildFlowScope(); - childA = childA.inferSlotType("localB", getNativeNumberType()); - - FlowScope childAB = childA.createChildFlowScope(); - childAB = childAB.inferSlotType("localB", getNativeStringType()); - - FlowScope childB = localEntry.createChildFlowScope(); - childB = childB.inferSlotType("localB", getNativeBooleanType()); + FlowScope childA = localEntry.inferSlotType("localB", getNativeNumberType()); + FlowScope childAB = childA.inferSlotType("localB", getNativeStringType()); + FlowScope childB = localEntry.inferSlotType("localB", getNativeBooleanType()); assertTypeEquals(getNativeStringType(), childAB.getSlot("localB").getType()); assertTypeEquals(getNativeBooleanType(), childB.getSlot("localB").getType()); @@ -96,11 +81,8 @@ public void testJoin1() { } public void testJoin2() { - FlowScope childA = localEntry.createChildFlowScope(); - childA = childA.inferSlotType("localA", getNativeStringType()); - - FlowScope childB = localEntry.createChildFlowScope(); - childB = childB.inferSlotType("globalB", getNativeBooleanType()); + FlowScope childA = localEntry.inferSlotType("localA", getNativeStringType()); + FlowScope childB = localEntry.inferSlotType("globalB", getNativeBooleanType()); assertTypeEquals(getNativeStringType(), childA.getSlot("localA").getType()); assertTypeEquals(getNativeBooleanType(), childB.getSlot("globalB").getType()); @@ -122,11 +104,8 @@ public void testJoin3() { localScope.declare("localC", null, getNativeStringType(), null); localScope.declare("localD", null, getNativeStringType(), null); - FlowScope childA = localEntry.createChildFlowScope(); - childA = childA.inferSlotType("localC", getNativeNumberType()); - - FlowScope childB = localEntry.createChildFlowScope(); - childA = childA.inferSlotType("localD", getNativeBooleanType()); + FlowScope childA = localEntry.inferSlotType("localC", getNativeNumberType()); + FlowScope childB = localEntry.inferSlotType("localD", getNativeBooleanType()); FlowScope joined = join(childB, childA); assertTypeEquals( @@ -148,13 +127,10 @@ public void testJoin3() { join(childB, childA), join(childA, childB)); } - /** - * Create a long chain of flow scopes where each link in the chain - * contains one slot. - */ - public void testLongChain1() { - FlowScope chainA = localEntry.createChildFlowScope(); - FlowScope chainB = localEntry.createChildFlowScope(); + /** Create a long chain of flow scopes. */ + public void testLongChain() { + FlowScope chainA = localEntry; + FlowScope chainB = localEntry; for (int i = 0; i < LONG_CHAIN_LENGTH; i++) { localScope.declare("local" + i, null, null, null); chainA = @@ -163,67 +139,8 @@ public void testLongChain1() { chainB = chainB.inferSlotType( "local" + i, i % 3 == 0 ? getNativeStringType() : getNativeBooleanType()); - - chainA = chainA.createChildFlowScope(); - chainB = chainB.createChildFlowScope(); } - verifyLongChains(chainA, chainB); - } - - /** - * Create a long chain of flow scopes where each link in the chain - * contains 7 slots. - */ - public void testLongChain2() { - FlowScope chainA = localEntry.createChildFlowScope(); - FlowScope chainB = localEntry.createChildFlowScope(); - for (int i = 0; i < LONG_CHAIN_LENGTH * 7; i++) { - localScope.declare("local" + i, null, null, null); - chainA = - chainA.inferSlotType( - "local" + i, i % 2 == 0 ? getNativeNumberType() : getNativeBooleanType()); - chainB = - chainB.inferSlotType( - "local" + i, i % 3 == 0 ? getNativeStringType() : getNativeBooleanType()); - - if (i % 7 == 0) { - chainA = chainA.createChildFlowScope(); - chainB = chainB.createChildFlowScope(); - } - } - - verifyLongChains(chainA, chainB); - } - - /** - * Create a long chain of flow scopes where every 4 links in the chain - * contain a slot. - */ - public void testLongChain3() { - FlowScope chainA = localEntry.createChildFlowScope(); - FlowScope chainB = localEntry.createChildFlowScope(); - for (int i = 0; i < LONG_CHAIN_LENGTH * 7; i++) { - if (i % 7 == 0) { - int j = i / 7; - localScope.declare("local" + j, null, null, null); - chainA = - chainA.inferSlotType( - "local" + j, j % 2 == 0 ? getNativeNumberType() : getNativeBooleanType()); - chainB = - chainB.inferSlotType( - "local" + j, j % 3 == 0 ? getNativeStringType() : getNativeBooleanType()); - } - - chainA = chainA.createChildFlowScope(); - chainB = chainB.createChildFlowScope(); - } - - verifyLongChains(chainA, chainB); - } - - // Common chain verification for testLongChainN for all N. - private void verifyLongChains(FlowScope chainA, FlowScope chainB) { FlowScope joined = join(chainA, chainB); for (int i = 0; i < LONG_CHAIN_LENGTH; i++) { assertTypeEquals( @@ -254,20 +171,11 @@ private void verifyLongChains(FlowScope chainA, FlowScope chainB) { } public void testDiffer1() { - FlowScope childA = localEntry.createChildFlowScope(); - childA = childA.inferSlotType("localB", getNativeNumberType()); - - FlowScope childAB = childA.createChildFlowScope(); - childAB = childAB.inferSlotType("localB", getNativeStringType()); - - FlowScope childABC = childAB.createChildFlowScope(); - childABC = childABC.inferSlotType("localA", getNativeBooleanType()); - - FlowScope childB = childAB.createChildFlowScope(); - childB = childB.inferSlotType("localB", getNativeStringType()); - - FlowScope childBC = childB.createChildFlowScope(); - childBC = childBC.inferSlotType("localA", getNativeNoType()); + FlowScope childA = localEntry.inferSlotType("localB", getNativeNumberType()); + FlowScope childAB = childA.inferSlotType("localB", getNativeStringType()); + FlowScope childABC = childAB.inferSlotType("localA", getNativeBooleanType()); + FlowScope childB = childAB.inferSlotType("localB", getNativeStringType()); + FlowScope childBC = childB.inferSlotType("localA", getNativeNoType()); assertScopesSame(childAB, childB); assertScopesDiffer(childABC, childBC); @@ -282,11 +190,8 @@ public void testDiffer1() { } public void testDiffer2() { - FlowScope childA = localEntry.createChildFlowScope(); - childA = childA.inferSlotType("localA", getNativeNumberType()); - - FlowScope childB = localEntry.createChildFlowScope(); - childB = childB.inferSlotType("localA", getNativeNoType()); + FlowScope childA = localEntry.inferSlotType("localA", getNativeNumberType()); + FlowScope childB = localEntry.inferSlotType("localA", getNativeNoType()); assertScopesDiffer(childA, childB); } diff --git a/test/com/google/javascript/jscomp/TypeInferenceTest.java b/test/com/google/javascript/jscomp/TypeInferenceTest.java index d6d896746f5..454279da180 100644 --- a/test/com/google/javascript/jscomp/TypeInferenceTest.java +++ b/test/com/google/javascript/jscomp/TypeInferenceTest.java @@ -186,8 +186,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { cfg.getImplicitReturn().getAnnotation(); // Reset the flow scope's syntactic scope to the function block, rather than the function node // itself. This allows pulling out local vars from the function by name to verify their types. - returnScope = - rtnState.getIn().createChildFlowScope(scopeCreator.createScope(n.getLastChild())); + returnScope = rtnState.getIn().withSyntacticScope(scopeCreator.createScope(n.getLastChild())); } private LabeledStatement getLabeledStatement(String label) {