Skip to content

Commit

Permalink
Create a separate structure to track forward-declared types.
Browse files Browse the repository at this point in the history
This removes the strange pattern of creating type registries before the
type checking passes are run.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=129137702
  • Loading branch information
blickly authored and Dimitris Vardoulakis committed Aug 5, 2016
1 parent c87776e commit 64b4ed4
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 49 deletions.
2 changes: 0 additions & 2 deletions src/com/google/javascript/jscomp/AbstractCompiler.java
Expand Up @@ -190,8 +190,6 @@ static enum MostRecentTypechecker {
*/
abstract CompilerPass getSymbolTable();

abstract void setSymbolTable(CompilerPass symbolTable);

/**
* Used by three passes that run in sequence (optimize-returns,
* optimize-parameters, remove-unused-variables), to avoid having them
Expand Down
24 changes: 7 additions & 17 deletions src/com/google/javascript/jscomp/Compiler.java
Expand Up @@ -211,6 +211,9 @@ public SourceFile apply(String filename) {
// Used by optimize-returns, optimize-parameters and remove-unused-variables
private DefinitionUseSiteFinder defFinder = null;

// Types that have been forward declared
private final Set<String> forwardDeclaredTypes = new HashSet<>();

// For use by the new type inference
private GlobalTypeInfo symbolTable;

Expand Down Expand Up @@ -1237,20 +1240,14 @@ public TypeIRegistry getTypeIRegistry() {
@Override
public JSTypeRegistry getTypeRegistry() {
if (typeRegistry == null) {
typeRegistry = new JSTypeRegistry(oldErrorReporter);
typeRegistry = new JSTypeRegistry(oldErrorReporter, forwardDeclaredTypes);
}
return typeRegistry;
}

@Override
void forwardDeclareType(String typeName) {
// Always add it to the old type registry, since OTI runs after NTI to
// provide types for the remaining passes.
// TODO(dimvar): change this when we stop running OTI after NTI.
getTypeRegistry().forwardDeclareType(typeName);
if (this.options.getNewTypeInference()) {
getSymbolTable().addUnknownTypeName(typeName);
}
forwardDeclaredTypes.add(typeName);
}

@Override
Expand Down Expand Up @@ -1352,18 +1349,11 @@ Iterable<TypeMismatch> getImplicitInterfaceUses() {
@Override
GlobalTypeInfo getSymbolTable() {
if (this.symbolTable == null) {
this.symbolTable = new GlobalTypeInfo(this);
this.symbolTable = new GlobalTypeInfo(this, forwardDeclaredTypes);
}
return this.symbolTable;
}

@Override
void setSymbolTable(CompilerPass symbolTable) {
Preconditions.checkArgument(
symbolTable == null || symbolTable instanceof GlobalTypeInfo);
this.symbolTable = (GlobalTypeInfo) symbolTable;
}

@Override
DefinitionUseSiteFinder getDefinitionFinder() {
return this.defFinder;
Expand Down Expand Up @@ -1531,7 +1521,7 @@ void orderInputs() {
// Forward-declare all the provided types, so that they
// are not flagged even if they are dropped from the process.
for (String provide : input.getProvides()) {
getTypeRegistry().forwardDeclareType(provide);
forwardDeclareType(provide);
}
}

Expand Down
10 changes: 3 additions & 7 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
Expand Up @@ -275,11 +275,12 @@ class GlobalTypeInfo implements CompilerPass, TypeIRegistry {
private Map<Node, JSType> declaredObjLitProps = new LinkedHashMap<>();

private JSTypes commonTypes;
private Set<String> unknownTypeNames = new LinkedHashSet<>();
private final Set<String> unknownTypeNames;

GlobalTypeInfo(AbstractCompiler compiler) {
GlobalTypeInfo(AbstractCompiler compiler, Set<String> unknownTypeNames) {
this.warnings = new WarningReporter(compiler);
this.compiler = compiler;
this.unknownTypeNames = unknownTypeNames;
this.convention = compiler.getCodingConvention();
this.varNameGen = new UniqueNameGenerator();
this.funNameGen = new DefaultNameGenerator(ImmutableSet.<String>of(), "", null);
Expand Down Expand Up @@ -310,10 +311,6 @@ JSType getPropDeclaredType(Node n) {
return declaredObjLitProps.get(n);
}

void addUnknownTypeName(String name) {
this.unknownTypeNames.add(name);
}

// Differs from the similar method in NTIScope class on how it treats qnames.
String getFunInternalName(Node n) {
Preconditions.checkArgument(n.isFunction());
Expand All @@ -337,7 +334,6 @@ public void process(Node externs, Node root) {

this.globalScope = new NTIScope(root, null, ImmutableList.<String>of(), commonTypes);
this.globalScope.addUnknownTypeNames(this.unknownTypeNames);
this.unknownTypeNames = null; // Don't retain the LinkedHashSet
scopes.add(this.globalScope);

// Processing of a scope is split into many separate phases, and it's not
Expand Down
24 changes: 9 additions & 15 deletions src/com/google/javascript/rhino/jstype/JSTypeRegistry.java
Expand Up @@ -48,6 +48,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Multimap;
import com.google.javascript.rhino.ErrorReporter;
Expand All @@ -62,7 +63,6 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
Expand Down Expand Up @@ -140,7 +140,7 @@ public class JSTypeRegistry implements TypeIRegistry, Serializable {
// Types that have been "forward-declared."
// If these types are not declared anywhere in the binary, we shouldn't
// try to type-check them at all.
private final Set<String> forwardDeclaredTypes = new HashSet<>();
private final Set<String> forwardDeclaredTypes;

// A map of properties to the types on which those properties have been
// declared.
Expand Down Expand Up @@ -178,12 +178,14 @@ public class JSTypeRegistry implements TypeIRegistry, Serializable {
// there are no template types.
private final TemplateTypeMap emptyTemplateTypeMap;

/**
* Constructs a new type registry populated with the built-in types.
*/
public JSTypeRegistry(
ErrorReporter reporter) {
public JSTypeRegistry(ErrorReporter reporter) {
this(reporter, ImmutableSet.<String>of());
}

/** Constructs a new type registry populated with the built-in types. */
public JSTypeRegistry(ErrorReporter reporter, Set<String> forwardDeclaredTypes) {
this.reporter = reporter;
this.forwardDeclaredTypes = forwardDeclaredTypes;
this.emptyTemplateTypeMap = new TemplateTypeMap(
this, ImmutableList.<TemplateType>of(), ImmutableList.<JSType>of());
nativeTypes = new JSType[JSTypeNative.values().length];
Expand Down Expand Up @@ -911,14 +913,6 @@ public void overwriteDeclaredType(String name, JSType t) {
register(t, name);
}

/**
* Records a forward-declared type name. We will not emit errors if this
* type name never resolves to anything.
*/
public void forwardDeclareType(String name) {
forwardDeclaredTypes.add(name);
}

/**
* Whether this is a forward-declared type name.
*/
Expand Down
Expand Up @@ -41,6 +41,7 @@

import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.javascript.rhino.JSTypeExpression;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.jstype.FunctionBuilder;
Expand All @@ -51,7 +52,6 @@
import com.google.javascript.rhino.jstype.ObjectType;
import com.google.javascript.rhino.jstype.RecordTypeBuilder;
import com.google.javascript.rhino.jstype.TemplatizedType;

import junit.framework.TestCase;

public abstract class BaseJSTypeTestCase extends TestCase {
Expand Down Expand Up @@ -119,7 +119,7 @@ public abstract class BaseJSTypeTestCase extends TestCase {
protected void setUp() throws Exception {
super.setUp();
errorReporter = new TestErrorReporter(null, null);
registry = new JSTypeRegistry(errorReporter);
registry = new JSTypeRegistry(errorReporter, ImmutableSet.of("forwardDeclared"));
initTypes();
}

Expand Down
9 changes: 3 additions & 6 deletions test/com/google/javascript/rhino/jstype/JSTypeTest.java
Expand Up @@ -168,7 +168,6 @@ public StaticTypedSlot<JSType> getSlot(String name) {

forwardDeclaredNamedType =
new NamedType(registry, "forwardDeclared", "source", 1, 0);
registry.forwardDeclareType("forwardDeclared");
forwardDeclaredNamedType.resolve(
new SimpleErrorReporter(), EMPTY_SCOPE);

Expand Down Expand Up @@ -4974,9 +4973,8 @@ public void testNamedTypeEquals2() {
*/
public void testForwardDeclaredNamedTypeEquals() {
// test == if references are equal
NamedType a = new NamedType(registry, "typeA", "source", 1, 0);
NamedType b = new NamedType(registry, "typeA", "source", 1, 0);
registry.forwardDeclareType("typeA");
NamedType a = new NamedType(registry, "forwardDeclared", "source", 1, 0);
NamedType b = new NamedType(registry, "forwardDeclared", "source", 1, 0);

assertTypeEquals(a, b);

Expand All @@ -4995,8 +4993,7 @@ public void testForwardDeclaredNamedTypeEquals() {
}

public void testForwardDeclaredNamedType() {
NamedType a = new NamedType(registry, "typeA", "source", 1, 0);
registry.forwardDeclareType("typeA");
NamedType a = new NamedType(registry, "forwardDeclared", "source", 1, 0);

assertTypeEquals(UNKNOWN_TYPE, a.getLeastSupertype(UNKNOWN_TYPE));
assertTypeEquals(CHECKED_UNKNOWN_TYPE,
Expand Down

0 comments on commit 64b4ed4

Please sign in to comment.