Skip to content

Commit

Permalink
Make NodeTraversal#getClosestHoistScope lazier.
Browse files Browse the repository at this point in the history
Also moves Scope.isHoistScopeRootNode to NodeTraversal, since that's the only place it's called from.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=180605851
  • Loading branch information
shicks authored and blickly committed Jan 3, 2018
1 parent ae183e1 commit 717340d
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 20 deletions.
42 changes: 34 additions & 8 deletions src/com/google/javascript/jscomp/NodeTraversal.java
Expand Up @@ -16,6 +16,7 @@

package com.google.javascript.jscomp;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Strings.nullToEmpty;
Expand Down Expand Up @@ -53,8 +54,7 @@ public class NodeTraversal {
private final Deque<Scope> scopes = new ArrayDeque<>();

/**
* A stack of scope roots. All scopes that have not been created
* are represented in this Deque.
* A stack of scope roots. See #scopes.
*/
private final ArrayList<Node> scopeRoots = new ArrayList<>();

Expand Down Expand Up @@ -938,15 +938,27 @@ public Scope getScope() {
return scope;
}

private Scope instantiateScopes(int count) {
checkArgument(count <= scopeRoots.size());
Scope scope = scopes.peek();

for (int i = 0; i < count; i++) {
scope = scopeCreator.createScope(scopeRoots.get(i), scope);
scopes.push(scope);
}
scopeRoots.subList(0, count).clear();
return scope;
}

public boolean isHoistScope() {
return Scope.isHoistScopeRootNode(getScopeRoot());
return isHoistScopeRootNode(getScopeRoot());
}

public Node getClosestHoistScopeRoot() {
int roots = scopeRoots.size();
for (int i = roots; i > 0; i--) {
Node rootNode = scopeRoots.get(i - 1);
if (Scope.isHoistScopeRootNode(rootNode)) {
if (isHoistScopeRootNode(rootNode)) {
return rootNode;
}
}
Expand All @@ -955,10 +967,24 @@ public Node getClosestHoistScopeRoot() {
}

public Scope getClosestHoistScope() {
// TODO(moz): This should not call getScope(). We should find the root of the closest hoist
// scope and effectively getScope() from there, which avoids scanning inner scopes that might
// not be needed.
return getScope().getClosestHoistScope();
for (int i = scopeRoots.size(); i > 0; i--) {
if (isHoistScopeRootNode(scopeRoots.get(i - 1))) {
return instantiateScopes(i);
}
}
return scopes.peek().getClosestHoistScope();
}

private static boolean isHoistScopeRootNode(Node n) {
switch (n.getToken()) {
case FUNCTION:
case MODULE_BODY:
case ROOT:
case SCRIPT:
return true;
default:
return NodeUtil.isFunctionBlock(n);
}
}

public TypedScope getTypedScope() {
Expand Down
12 changes: 0 additions & 12 deletions src/com/google/javascript/jscomp/Scope.java
Expand Up @@ -326,18 +326,6 @@ boolean isHoistScope() {
return isFunctionScope() || isFunctionBlockScope() || isGlobal() || isModuleScope();
}

public static boolean isHoistScopeRootNode(Node n) {
switch (n.getToken()) {
case FUNCTION:
case MODULE_BODY:
case ROOT:
case SCRIPT:
return true;
default:
return NodeUtil.isFunctionBlock(n);
}
}

/**
* If a var were declared in this scope, return the scope it would be hoisted to.
*
Expand Down

0 comments on commit 717340d

Please sign in to comment.