Skip to content

Commit

Permalink
Simplify type resolution to no longer be scope-dependent
Browse files Browse the repository at this point in the history 
Removes TypedScope.ScopeResolver, which existed for two purposes: (1) ensure that JSType#resolve is called with the right scope argument, and (2) ensure that inner scopes were resolved before outer scopes.  These are no longer relevant, since NamedType now stores the resolution scope internally, and therefore the resolution order is no longer important.

Now there is only a single resolveTypes() method on TypedScopeCreator and JSTypeRegistry that is called outside of the per-scope loops.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=197846646
  • Loading branch information
@shicks @brad4d
shicks authored and brad4d committed May 24, 2018
1 parent 6e05dcc commit a9f638a
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 81 deletions.
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/DefaultPassConfig.java
View file
Expand Up @@ -2077,7 +2077,7 @@ public void process(Node externs, Node root) {
if (topScope == null) {
regenerateGlobalTypedScope(compiler, root.getParent());
} else {
compiler.getTypeRegistry().resolveTypesInScope(topScope);
compiler.getTypeRegistry().resolveTypes();
}
}
@Override
Expand Down
11 changes: 5 additions & 6 deletions src/com/google/javascript/jscomp/TypeInferencePass.java
View file
Expand Up @@ -101,13 +101,15 @@ void inferAllScopes(Node node) {
compiler, new FirstScopeBuildingCallback(), scopeCreator))
.traverseWithScope(node, topScope);

for (TypedScope s : scopeCreator.getAllMemoizedScopes()) {
s.resolveTypes();
}
scopeCreator.resolveTypes();

(new NodeTraversal(
compiler, new SecondScopeBuildingCallback(), scopeCreator))
.traverseWithScope(node, topScope);

// Resolve any new type names found during the inference.
// This runs for nested block scopes after infer runs on the CFG root.
compiler.getTypeRegistry().resolveTypes();
}

void inferScope(Node n, TypedScope scope) {
Expand Down Expand Up @@ -144,9 +146,6 @@ public void enterScope(NodeTraversal t) {
if (!scope.isBlockScope()) { // ignore scopes that don't have their own CFGs.
inferScope(t.getCurrentNode(), scope);
}
// Resolve any new type names found during the inference.
// This runs for nested block scopes after infer runs on the CFG root.
compiler.getTypeRegistry().resolveTypesInScope(scope);
}

@Override
Expand Down
18 changes: 0 additions & 18 deletions src/com/google/javascript/jscomp/TypedScope.java
View file
Expand Up @@ -180,24 +180,6 @@ private boolean isDeclarativelyUnboundVarWithoutType(TypedVar var) {
&& !var.isExtern();
}

static interface TypeResolver {
void resolveTypes();
}

private TypeResolver typeResolver;

/** Resolve all type references. Only used on typed scopes. */
void resolveTypes() {
if (typeResolver != null) {
typeResolver.resolveTypes();
typeResolver = null;
}
}

void setTypeResolver(TypeResolver resolver) {
this.typeResolver = resolver;
}

public JSType getNamespaceOrTypedefType(String typeName) {
StaticTypedSlot slot = getSlot(typeName);
return slot == null ? null : slot.getType();
Expand Down
59 changes: 28 additions & 31 deletions src/com/google/javascript/jscomp/TypedScopeCreator.java
View file
Expand Up @@ -183,6 +183,8 @@ final class TypedScopeCreator implements ScopeCreator, StaticSymbolTable<TypedVa
// For convenience
private final ObjectType unknownType;

private final List<DeferredSetType> deferredSetTypes = new ArrayList<>();

/**
* Defer attachment of types to nodes until all type names
* have been resolved. Then, we can resolve the type and attach it.
Expand Down Expand Up @@ -322,7 +324,6 @@ private TypedScope createScopeInternal(Node root, TypedScope typedParent) {
typeRegistry, delegateProxies, delegateCallingConventions);
}

newScope.setTypeResolver(scopeBuilder);
return newScope;
}

Expand Down Expand Up @@ -430,6 +431,31 @@ private static void declareNativeType(TypedScope scope, String name, JSType t) {
scope.declare(name, null, t, null, false);
}

/** Set the type for a node now, and enqueue it to be updated with a resolved type later. */
void setDeferredType(Node node, JSType type) {
// Other parts of this pass may read the not-yet-resolved type off the node.
// (like when we set the LHS of an assign with a typed RHS function.)
node.setJSType(type);
deferredSetTypes.add(new DeferredSetType(node, type));
}

void resolveTypes() {
// Resolve types and attach them to nodes.
for (DeferredSetType deferred : deferredSetTypes) {
deferred.resolve();
}

// Resolve types and attach them to scope slots.
for (TypedScope scope : getAllMemoizedScopes()) {
for (TypedVar var : scope.getVarIterable()) {
var.resolveType(typeParsingErrorReporter);
}
}

// Tell the type registry that any remaining types are unknown.
typeRegistry.resolveTypes();
}

/**
* Adds all globally-defined enums and typedefs to the registry's list of non-nullable types.
*
Expand Down Expand Up @@ -491,18 +517,14 @@ private JSType getNativeType(JSTypeNative nativeType) {
return typeRegistry.getNativeType(nativeType);
}

private abstract class AbstractScopeBuilder
implements NodeTraversal.Callback, TypedScope.TypeResolver {
private abstract class AbstractScopeBuilder implements NodeTraversal.Callback {

/** The scope that we're building. */
final TypedScope currentScope;

/** The current hoist scope. */
final TypedScope currentHoistScope;

private final List<DeferredSetType> deferredSetTypes =
new ArrayList<>();

/**
* Object literals with a @lends annotation aren't analyzed until we
* reach the root of the statement they're defined in.
Expand Down Expand Up @@ -544,31 +566,6 @@ void build() {
NodeTraversal.traverseTyped(compiler, currentScope.getRootNode(), this);
}

/** Set the type for a node now, and enqueue it to be updated with a resolved type later. */
void setDeferredType(Node node, JSType type) {
// Other parts of this pass may read the not-yet-resolved type off the node.
// (like when we set the LHS of an assign with a typed RHS function.)
node.setJSType(type);
deferredSetTypes.add(new DeferredSetType(node, type));
}

@Override
public void resolveTypes() {
// Resolve types and attach them to nodes.
for (DeferredSetType deferred : deferredSetTypes) {
deferred.resolve();
}

// Resolve types and attach them to scope slots.
for (TypedVar var : currentScope.getVarIterable()) {
var.resolveType(typeParsingErrorReporter);
}

// Tell the type registry that any remaining types
// are unknown.
typeRegistry.resolveTypesInScope(currentScope);
}

@Override
public final boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
inputId = t.getInputId();
Expand Down
44 changes: 19 additions & 25 deletions src/com/google/javascript/rhino/jstype/JSTypeRegistry.java
View file
Expand Up @@ -50,7 +50,6 @@
import com.google.common.annotations.GwtIncompatible;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -205,8 +204,7 @@ public class JSTypeRegistry implements Serializable {
LinkedHashMultimap.create();

// All the unresolved named types.
private final Multimap<StaticTypedScope, NamedType> unresolvedNamedTypes =
ArrayListMultimap.create();
private final List<NamedType> unresolvedNamedTypes = new ArrayList<>();

// The template type name.
private final Map<String, TemplateType> templateTypes = new HashMap<>();
Expand Down Expand Up @@ -1376,9 +1374,7 @@ private JSType getType(
// interning.
NamedType namedType = createNamedType(scope, jsTypeName, sourceName, lineno, charno);
if (recordUnresolvedTypes) {


unresolvedNamedTypes.put(scope, namedType);
unresolvedNamedTypes.add(namedType);
}
type = namedType;
}
Expand Down Expand Up @@ -1407,28 +1403,26 @@ public void clearNamedTypes() {
}

/** Resolve all the unresolved types in the given scope. */
public void resolveTypesInScope(StaticTypedScope scope) {
for (NamedType type : unresolvedNamedTypes.get(scope)) {
public void resolveTypes() {
for (NamedType type : unresolvedNamedTypes) {
type.resolve(reporter);
}

unresolvedNamedTypes.removeAll(scope);

if (scope != null && scope.getParentScope() == null) {
// By default, the global "this" type is just an anonymous object.
// If the user has defined a Window type, make the Window the
// implicit prototype of "this".
PrototypeObjectType globalThis = (PrototypeObjectType) getNativeType(
JSTypeNative.GLOBAL_THIS);
JSType windowType = getTypeInternal(null, "Window");
if (globalThis.isUnknownType()) {
ObjectType windowObjType = ObjectType.cast(windowType);
if (windowObjType != null) {
globalThis.setImplicitPrototype(windowObjType);
} else {
globalThis.setImplicitPrototype(
getNativeObjectType(JSTypeNative.OBJECT_TYPE));
}
unresolvedNamedTypes.clear();

// By default, the global "this" type is just an anonymous object.
// If the user has defined a Window type, make the Window the
// implicit prototype of "this".
PrototypeObjectType globalThis = (PrototypeObjectType) getNativeType(
JSTypeNative.GLOBAL_THIS);
JSType windowType = getTypeInternal(null, "Window");
if (globalThis.isUnknownType()) {
ObjectType windowObjType = ObjectType.cast(windowType);
if (windowObjType != null) {
globalThis.setImplicitPrototype(windowObjType);
} else {
globalThis.setImplicitPrototype(
getNativeObjectType(JSTypeNative.OBJECT_TYPE));
}
}
}
Expand Down

0 comments on commit a9f638a

Please sign in to comment.