Skip to content

Commit

Permalink
Make all LinkedFlowScope fields final.
Browse files Browse the repository at this point in the history
  * inferSlotType and inferQualifiedSlot now return new FlowScopes.
  * createChildFlowScope() and optimize() are now no-ops and can be removed.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=197091856
  • Loading branch information
shicks authored and blickly committed May 18, 2018
1 parent 1695a44 commit d9bce97
Showing 1 changed file with 15 additions and 50 deletions.
65 changes: 15 additions & 50 deletions src/com/google/javascript/jscomp/LinkedFlowScope.java
Expand Up @@ -17,7 +17,6 @@
package com.google.javascript.jscomp;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;

import com.google.javascript.jscomp.type.FlowScope;
import com.google.javascript.rhino.HamtPMap;
Expand All @@ -37,32 +36,23 @@
*/
class LinkedFlowScope implements FlowScope {

// The parent flow scope.
private final LinkedFlowScope parent;

// Map from TypedScope to OverlayScope.
private PMap<TypedScope, OverlayScope> scopes;
private final PMap<TypedScope, OverlayScope> scopes;

private final TypedScope functionScope;

// The TypedScope for the block that this flow scope is defined for.
private final TypedScope syntacticScope;

// Flow scopes assume that all their ancestors are immutable.
// So once a child scope is created, this flow scope may not be modified.
private boolean frozen = false;

/**
* Creates a flow scope without a direct parent. This can happen in three cases: (1) the "bottom"
* scope for a CFG root, (2) a direct child of a parent at the maximum depth, or (3) a joined
* scope with more than one direct parent. The parent is non-null only in the second case.
*/
private LinkedFlowScope(
LinkedFlowScope parent,
PMap<TypedScope, OverlayScope> scopes,
TypedScope syntacticScope,
TypedScope functionScope) {
this.parent = parent;
this.scopes = scopes;
this.syntacticScope = syntacticScope;
this.functionScope = functionScope;
Expand Down Expand Up @@ -106,18 +96,19 @@ private boolean flowsFromBottom() {
* Creates an entry lattice for the flow.
*/
public static LinkedFlowScope createEntryLattice(TypedScope scope) {
return new LinkedFlowScope(null, HamtPMap.<TypedScope, OverlayScope>empty(), scope, scope);
return new LinkedFlowScope(HamtPMap.<TypedScope, OverlayScope>empty(), scope, scope);
}

@Override
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 =
PMap<TypedScope, OverlayScope> newScopes =
!newScope.slots.isEmpty() ? scopes.plus(scope.scope, newScope) : scopes.minus(scope.scope);
return this;
return newScopes != scopes
? new LinkedFlowScope(newScopes, syntacticScope, functionScope)
: this;
}

@Override
Expand Down Expand Up @@ -162,8 +153,7 @@ public LinkedFlowScope inferQualifiedSlot(
v.setType(v.getType().getLeastSupertype(inferredType));
}
}
inferSlotType(symbol, inferredType);
return this;
return inferSlotType(symbol, inferredType);
}

@Override
Expand Down Expand Up @@ -215,45 +205,23 @@ public StaticTypedSlot getOwnSlot(String name) {

@Override
public FlowScope createChildFlowScope() {
return createChildFlowScope(syntacticScope);
return this;
}

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

/**
* Remove flow scopes that add nothing to the flow.
*/
@Override
public LinkedFlowScope optimize() {
LinkedFlowScope current = this;
// NOTE(sdh): This function does not take syntacticScope into account.
// This means that an optimized scope cannot be used to look up names
// by string without first creating a child in the correct block. This
// is not a problem, since this is only used for (a) determining whether
// to join two scopes, (b) determining whether two scopes are equal, or
// (c) optimizing away unnecessary children generated by flowing through
// an expression. In (a) and (b) the result is only inspected locally and
// not escaped. In (c) the result is fed directly into further joins and
// will always have a block scope reassigned before flowing into another
// node. In all cases, it's therefore safe to ignore block scope changes
// when optimizing.
while (current.parent != null && current.scopes == current.parent.scopes) {
current = current.parent;
}
return current;
}

/** Returns whether this.optimize() == that.optimize(), but without walking up the chain. */
private boolean optimizesToSameScope(LinkedFlowScope that) {
// NOTE: The function scope should generally be the same, but it's possible that one is
// bottom, in which case the scopes are definitely not the same, even if both scope maps
// are empty.
return this.scopes == that.scopes && this.functionScope.equals(that.functionScope);
return this;
}

@Override
Expand All @@ -279,10 +247,8 @@ public FlowScope apply(FlowScope a, FlowScope b) {
// To join the two scopes, we have to
LinkedFlowScope linkedA = (LinkedFlowScope) a;
LinkedFlowScope linkedB = (LinkedFlowScope) b;
linkedA.frozen = true;
linkedB.frozen = true;
if (linkedA.optimizesToSameScope(linkedB)) {
return linkedA.createChildFlowScope();
if (linkedA == linkedB) {
return linkedA;
}

// NOTE: it would be nice to put 'null' as the syntactic scope if they're not
Expand All @@ -301,7 +267,6 @@ public FlowScope apply(FlowScope a, FlowScope b) {
// adding irrelevant block-local variables to the joined scope unnecessarily.
TypedScope common = getCommonParentDeclarationScope(linkedA, linkedB);
return new LinkedFlowScope(
null,
join(linkedA, linkedB, common),
common,
linkedA.flowsFromBottom() ? linkedB.functionScope : linkedA.functionScope);
Expand All @@ -325,7 +290,7 @@ public boolean equals(Object other) {
}

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

Expand Down

0 comments on commit d9bce97

Please sign in to comment.