Skip to content

Commit

Permalink
[NTI] Small improvements. Store the JSTypes object in all namespaces.…
Browse files Browse the repository at this point in the history 
… Fix a bug with prototype assignments. Fix a bug with function subtyping.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=125818143
  • Loading branch information
@dimvar @blickly
dimvar authored and blickly committed Jun 27, 2016
1 parent eb1913f commit cfb86dc
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 66 deletions.
33 changes: 21 additions & 12 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
View file
Expand Up @@ -456,8 +456,7 @@ private Collection<PropertyDef> getPropDefsFromInterface(
return result.build();
}

private PropertyDef getPropDefFromClass(
NominalType nominalType, String pname) {
private PropertyDef getPropDefFromClass(NominalType nominalType, String pname) {
while (nominalType.getPropDeclaredType(pname) != null) {
Preconditions.checkArgument(nominalType.isFinalized());
Preconditions.checkArgument(nominalType.isClass());
Expand Down Expand Up @@ -934,9 +933,9 @@ private void visitEnum(Node qnameNode) {
}
currentScope.addNamespace(qnameNode,
EnumType.make(
commonTypes,
qnameNode.getQualifiedName(),
jsdoc.getEnumParameterType(),
ImmutableSet.copyOf(propNames)));
jsdoc.getEnumParameterType(), ImmutableSet.copyOf(propNames)));
}

private void visitFunctionEarly(Node fn) {
Expand Down Expand Up @@ -1040,15 +1039,20 @@ private void maybeRecordNominalType(
ImmutableList<String> typeParameters = builder.build();
RawNominalType rawType;
if (fnDoc.usesImplicitMatch()) {
rawType = RawNominalType.makeStructuralInterface(defSite, qname, typeParameters);
rawType = RawNominalType.makeStructuralInterface(
commonTypes, defSite, qname, typeParameters);
} else if (fnDoc.isInterface()) {
rawType = RawNominalType.makeNominalInterface(defSite, qname, typeParameters);
rawType = RawNominalType.makeNominalInterface(
commonTypes, defSite, qname, typeParameters);
} else if (fnDoc.makesStructs()) {
rawType = RawNominalType.makeStructClass(defSite, qname, typeParameters);
rawType = RawNominalType.makeStructClass(
commonTypes, defSite, qname, typeParameters);
} else if (fnDoc.makesDicts()) {
rawType = RawNominalType.makeDictClass(defSite, qname, typeParameters);
rawType = RawNominalType.makeDictClass(
commonTypes, defSite, qname, typeParameters);
} else {
rawType = RawNominalType.makeUnrestrictedClass(defSite, qname, typeParameters);
rawType = RawNominalType.makeUnrestrictedClass(
commonTypes, defSite, qname, typeParameters);
}
nominaltypesByNode.put(defSite, rawType);
if (isRedeclaration) {
Expand Down Expand Up @@ -1141,7 +1145,7 @@ private void maybeAddFunctionToNamespace(Node funQname) {
QualifiedName pname = new QualifiedName(funQname.getLastChild().getString());
if (!ns.isDefined(pname)) {
ns.addNamespace(pname,
new FunctionNamespace(funQname.getQualifiedName(), s));
new FunctionNamespace(commonTypes, funQname.getQualifiedName(), s));
}
}

Expand Down Expand Up @@ -1390,6 +1394,7 @@ else if (isPrototypeProperty(getProp)) {
}
// Direct assignment to the prototype
else if (NodeUtil.isPrototypeAssignment(getProp)) {
getProp.putBooleanProp(Node.ANALYZED_DURING_GTI, true);
visitPrototypeAssignment(getProp);
}
// "Static" property on constructor
Expand Down Expand Up @@ -1506,11 +1511,15 @@ private void mayWarnAboutInterfacePropInit(RawNominalType rawType, Node initiali

private void visitPrototypeAssignment(Node getProp) {
Preconditions.checkArgument(getProp.isGetProp());
Node protoObjNode = getProp.getParent().getLastChild();
if (!protoObjNode.isObjectLit()) {
return;
}
Node ctorNameNode = NodeUtil.getPrototypeClassName(getProp);
QualifiedName ctorQname = QualifiedName.fromNode(ctorNameNode);
RawNominalType rawType = currentScope.getNominalType(ctorQname);
if (rawType == null) {
for (Node objLitChild : getProp.getParent().getLastChild().children()) {
for (Node objLitChild : protoObjNode.children()) {
Node initializer = objLitChild.getLastChild();
if (initializer != null && initializer.isFunction()) {
visitFunctionLate(initializer, null);
Expand All @@ -1519,7 +1528,7 @@ private void visitPrototypeAssignment(Node getProp) {
return;
}
getProp.putBooleanProp(Node.ANALYZED_DURING_GTI, true);
for (Node objLitChild : getProp.getParent().getLastChild().children()) {
for (Node objLitChild : protoObjNode.children()) {
mayAddPropToPrototype(rawType, objLitChild.getString(), objLitChild,
objLitChild.getLastChild());
}
Expand Down
13 changes: 7 additions & 6 deletions src/com/google/javascript/jscomp/NTIScope.java
View file
Expand Up @@ -483,19 +483,21 @@ void addFunNamespace(Node qnameNode) {
Preconditions.checkArgument(isDefinedLocally(varName, false));
Preconditions.checkState(!this.localNamespaces.containsKey(varName));
NTIScope s = Preconditions.checkNotNull(this.localFunDefs.get(varName));
this.localNamespaces.put(varName, new FunctionNamespace(varName, s));
this.localNamespaces.put(varName,
new FunctionNamespace(getCommonTypes(), varName, s));
} else {
Preconditions.checkArgument(!isNamespace(qnameNode));
QualifiedName qname = QualifiedName.fromNode(qnameNode);
Namespace ns = getNamespace(qname.getLeftmostName());
NTIScope s = (NTIScope) ns.getDeclaration(qname).getFunctionScope();
ns.addNamespace(qname.getAllButLeftmost(),
new FunctionNamespace(qname.toString(), s));
new FunctionNamespace(getCommonTypes(), qname.toString(), s));
}
}

void addNamespaceLit(Node qnameNode) {
addNamespace(qnameNode, new NamespaceLit(qnameNode.getQualifiedName()));
addNamespace(qnameNode,
new NamespaceLit(getCommonTypes(), qnameNode.getQualifiedName()));
}

void updateType(String name, JSType newDeclType) {
Expand Down Expand Up @@ -655,7 +657,6 @@ void finalizeScope() {
Preconditions.checkState(isTopLevel() || this.declaredType != null,
"No declared type for function-scope: %s", this.root);
unknownTypeNames = ImmutableSet.of();
JSTypes commonTypes = getCommonTypes();
// For now, we put types of namespaces directly into the locals.
// Alternatively, we could move this into NewTypeInference.initEdgeEnvs
for (Map.Entry<String, Namespace> entry : localNamespaces.entrySet()) {
Expand All @@ -669,9 +670,9 @@ void finalizeScope() {
// window, but we don't check here to avoid hard-coding the name.
// Enforced in GlobalTypeInfo.
nslit.maybeSetWindowInstance(externs.get(name));
t = nslit.toJSType(commonTypes);
t = nslit.toJSType();
} else {
t = ns.toJSType(commonTypes);
t = ns.toJSType();
}
if (externs.containsKey(name)) {
externs.put(name, t);
Expand Down
3 changes: 1 addition & 2 deletions src/com/google/javascript/jscomp/NodeUtil.java
View file
Expand Up @@ -3474,8 +3474,7 @@ static boolean isPrototypeAssignment(Node getProp) {
}
Node parent = getProp.getParent();
return parent.isAssign() && parent.getFirstChild() == getProp
&& parent.getFirstChild().getLastChild().getString().equals("prototype")
&& parent.getLastChild().isObjectLit();
&& parent.getFirstChild().getLastChild().getString().equals("prototype");
}

/**
Expand Down
14 changes: 7 additions & 7 deletions src/com/google/javascript/jscomp/newtypes/EnumType.java
View file
Expand Up @@ -50,19 +50,19 @@ private enum State {
// All properties have the same type, so we only need a set, not a map.
private ImmutableSet<String> props;

private EnumType(
String name, JSTypeExpression typeExpr, Collection<String> props) {
private EnumType(JSTypes commonTypes, String name,
JSTypeExpression typeExpr, Collection<String> props) {
super(commonTypes, name);
Preconditions.checkNotNull(typeExpr);
this.state = State.NOT_RESOLVED;
this.name = name;
// typeExpr is non-null iff the enum is not resolved
this.typeExpr = typeExpr;
this.props = ImmutableSet.copyOf(props);
}

public static EnumType make(
String name, JSTypeExpression typeExpr, Collection<String> props) {
return new EnumType(name, typeExpr, props);
public static EnumType make(JSTypes commonTypes, String name,
JSTypeExpression typeExpr, Collection<String> props) {
return new EnumType(commonTypes, name, typeExpr, props);
}

public boolean isResolved() {
Expand Down Expand Up @@ -114,7 +114,7 @@ void resolveEnum(JSType t) {
* the properties of the object literal are constant.
*/
@Override
protected JSType computeJSType(JSTypes commonTypes) {
protected JSType computeJSType() {
Preconditions.checkNotNull(enumPropType);
Preconditions.checkState(this.namespaceType == null);
PersistentMap<String, Property> propMap = PersistentMap.create();
Expand Down
9 changes: 5 additions & 4 deletions src/com/google/javascript/jscomp/newtypes/FunctionNamespace.java
View file
Expand Up @@ -25,18 +25,19 @@
public final class FunctionNamespace extends Namespace {
private DeclaredTypeRegistry scope;

public FunctionNamespace(String name, DeclaredTypeRegistry scope) {
public FunctionNamespace(
JSTypes commonTypes, String name, DeclaredTypeRegistry scope) {
super(commonTypes, name);
Preconditions.checkNotNull(name);
Preconditions.checkNotNull(scope);
this.name = name;
this.scope = scope;
}

@Override
protected JSType computeJSType(JSTypes commonTypes) {
protected JSType computeJSType() {
Preconditions.checkState(this.namespaceType == null);
return JSType.fromObjectType(ObjectType.makeObjectType(
commonTypes.getFunctionType(),
this.commonTypes.getFunctionType(),
null,
this.scope.getDeclaredFunctionType().toFunctionType(),
this,
Expand Down
5 changes: 3 additions & 2 deletions src/com/google/javascript/jscomp/newtypes/FunctionType.java
View file
Expand Up @@ -505,7 +505,7 @@ private boolean isSubtypeOfHelper(FunctionType other, boolean checkThisType,
if (this.nominalType == null && other.nominalType != null
|| this.nominalType != null && other.nominalType == null
|| this.nominalType != null && other.nominalType != null
&& !this.nominalType.isSubtypeOf(other.nominalType)) {
&& !this.nominalType.isSubtypeOf(other.nominalType, subSuperMap)) {
return false;
}

Expand Down Expand Up @@ -737,7 +737,8 @@ public List<String> getTypeParameters() {

boolean unifyWithSubtype(FunctionType other, List<String> typeParameters,
Multimap<String, JSType> typeMultimap, SubtypeCache subSuperMap) {
Preconditions.checkState(this.typeParameters.isEmpty());
Preconditions.checkState(this.typeParameters.isEmpty(),
"Non-empty type parameters %s", this.typeParameters);
Preconditions.checkState(this.outerVarPreconditions.isEmpty());
Preconditions.checkState(this != TOP_FUNCTION);

Expand Down
18 changes: 12 additions & 6 deletions src/com/google/javascript/jscomp/newtypes/Namespace.java
View file
Expand Up @@ -39,7 +39,8 @@ public abstract class Namespace {
// "Simple type" properties (i.e. represented as JSTypes rather than something
// more specific).
protected PersistentMap<String, Property> otherProps = PersistentMap.create();
protected String name;
protected final String name;
protected final JSTypes commonTypes;
// Represents the namespace as an ObjectType wrapped in a JSType.
// The namespace field of the ObjectType contains the namespace instance.
// In addition,
Expand All @@ -49,7 +50,12 @@ public abstract class Namespace {
// Used to detect recursion when computing the type of circular namespaces.
private boolean duringComputeJSType = false;

protected abstract JSType computeJSType(JSTypes commonTypes);
protected Namespace(JSTypes commonTypes, String name) {
this.name = name;
this.commonTypes = commonTypes;
}

protected abstract JSType computeJSType();

public final String getName() {
return name;
Expand Down Expand Up @@ -216,18 +222,18 @@ final Set<String> getAllPropsOfNamespace() {
return s;
}

public final JSType toJSType(JSTypes commonTypes) {
public final JSType toJSType() {
if (this.namespaceType == null) {
Preconditions.checkNotNull(commonTypes);
for (Namespace ns : this.namespaces.values()) {
if (this.duringComputeJSType) {
return null;
}
this.duringComputeJSType = true;
ns.toJSType(commonTypes);
ns.toJSType();
this.duringComputeJSType = false;
}
this.namespaceType = Preconditions.checkNotNull(computeJSType(commonTypes));
this.namespaceType = Preconditions.checkNotNull(computeJSType());
}
return this.namespaceType;
}
Expand All @@ -251,7 +257,7 @@ public final void copyWindowProperties(JSTypes commonTypes, RawNominalType win)
continue;
}
}
win.addProtoProperty(entry.getKey(), null, ns.toJSType(commonTypes), true);
win.addProtoProperty(entry.getKey(), null, ns.toJSType(), true);
}
for (Map.Entry<String, Property> entry : this.otherProps.entrySet()) {
win.addProtoProperty(
Expand Down
6 changes: 3 additions & 3 deletions src/com/google/javascript/jscomp/newtypes/NamespaceLit.java
View file
Expand Up @@ -27,8 +27,8 @@ public final class NamespaceLit extends Namespace {
// For when window is used as a namespace
private NominalType window = null;

public NamespaceLit(String name) {
this.name = name;
public NamespaceLit(JSTypes commonTypes, String name) {
super(commonTypes, name);
}

NominalType getWindowType() {
Expand All @@ -42,7 +42,7 @@ public void maybeSetWindowInstance(JSType obj) {
}

@Override
protected JSType computeJSType(JSTypes commonTypes) {
protected JSType computeJSType() {
Preconditions.checkState(this.namespaceType == null);
return JSType.fromObjectType(ObjectType.makeObjectType(
this.window, null, null, this, false, ObjectKind.UNRESTRICTED));
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/newtypes/ObjectType.java
View file
Expand Up @@ -1150,7 +1150,7 @@ static ObjectType unifyUnknowns(ObjectType t1, ObjectType t2) {
/**
* Unify {@code this}, which may contain free type variables,
* with {@code other}, a concrete type, modifying the supplied
* {@code typeMultimap} to add any new template varaible type bindings.
* {@code typeMultimap} to add any new template variable type bindings.
* @return Whether unification succeeded
*/
boolean unifyWithSubtype(ObjectType other, List<String> typeParameters,
Expand Down

0 comments on commit cfb86dc

Please sign in to comment.