Skip to content

Commit

Permalink
Various fixes to enable TTL in NTI.
Browse files Browse the repository at this point in the history 
- Infer more THIS types in function binds
- Handle BOTTOM types in TTL type maps
- Improve typing of variables across scopes
- Improve subtyping for uninstantiated nominal types
- Take function exits from throws into account when computing the summary

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164988396
  • Loading branch information
@dimvar @blickly
dimvar authored and blickly committed Aug 11, 2017
1 parent b09c790 commit cb7cffa
Show file tree
Hide file tree
Showing 8 changed files with 227 additions and 73 deletions.
37 changes: 26 additions & 11 deletions src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java
View file
Expand Up @@ -2360,6 +2360,12 @@ private DeclaredFunctionType computeFnDeclaredType(
RawNominalType ctorType = nominaltypesByNode.get(declNode);
FunctionAndSlotType result = getTypeParser().getFunctionType(
fnDoc, functionName, declNode, ctorType, ownerType, parentScope);
// If the function does not have a declared THIS type, see if it's used in a bind,
// and if so, try to infer a THIS type from the object pass to bind.
if (result.functionType.getReceiverType() == null) {
JSType bindRecvType = getReceiverTypeOfBind(declNode);
result.functionType = result.functionType.withReceiverType(bindRecvType);
}
Node qnameNode;
if (declNode.isQualifiedName()) {
qnameNode = declNode;
Expand Down Expand Up @@ -2434,21 +2440,13 @@ private DeclaredFunctionType computeFnDeclaredTypeFromCallee(
private DeclaredFunctionType getDeclaredFunctionTypeFromContext(
String functionName, Node declNode, NTIScope parentScope) {
Node parent = declNode.getParent();
Node maybeBind = parent.isCall() ? parent.getFirstChild() : parent;

// The function literal is used with .bind or goog.bind
if (NodeUtil.isFunctionBind(maybeBind) && !NodeUtil.isGoogPartial(maybeBind)) {
Node call = maybeBind.getParent();
Bind bindComponents = convention.describeFunctionBind(call, true, false);
JSType recvType = bindComponents.thisValue == null
? null : simpleInferExprType(bindComponents.thisValue);
if (recvType == null) {
return null;
}
JSType bindRecvType = getReceiverTypeOfBind(declNode);
if (bindRecvType != null) {
// Use typeParser for the formals, and only add the receiver type here.
DeclaredFunctionType allButRecvType = getTypeParser().getFunctionType(
null, functionName, declNode, null, null, parentScope).functionType;
return allButRecvType.withReceiverType(recvType);
return allButRecvType.withReceiverType(bindRecvType);
}

// The function literal is an argument at a call
Expand All @@ -2475,6 +2473,23 @@ private DeclaredFunctionType getDeclaredFunctionTypeFromContext(
return null;
}

/**
* If the argument function is used in a bind in the AST, infer the receiver type.
* Return null otherwise.
*/
private JSType getReceiverTypeOfBind(Node funDeclNode) {
Node parent = funDeclNode.getParent();
Node maybeBind = parent.isCall() ? parent.getFirstChild() : parent;
// The function literal is used with .bind or goog.bind
if (NodeUtil.isFunctionBind(maybeBind) && !NodeUtil.isGoogPartial(maybeBind)) {
Node call = maybeBind.getParent();
Bind bindComponents = convention.describeFunctionBind(call, true, false);
return bindComponents.thisValue == null
? null : simpleInferExprType(bindComponents.thisValue);
}
return null;
}

private FunctionType getInstantiatedCalleeType(
Node call, FunctionType calleeType, boolean bailForUntypedArguments) {
Node callee = call.getFirstChild();
Expand Down
59 changes: 46 additions & 13 deletions src/com/google/javascript/jscomp/NewTypeInference.java
View file
Expand Up @@ -435,6 +435,9 @@ void add(JSError warning) {
private NTIScope currentScope;
// This TypeEnv should be computed once per scope
private TypeEnv typeEnvFromDeclaredTypes = null;
// Throws are not connected to the implicit return of the CFG. Record type environments here
// to use when computing the summary of a function.
private List<TypeEnv> exitEnvs = null;
private GlobalTypeInfo symbolTable;
private JSTypes commonTypes;
// RETVAL_ID is used when we calculate the summary type of a function
Expand Down Expand Up @@ -777,6 +780,7 @@ private JSType getSummaryOfLocalFunDef(String name) {
private void analyzeFunction(NTIScope scope) {
println("=== Analyzing function: ", scope.getReadableName(), " ===");
currentScope = scope;
exitEnvs = new ArrayList<>();
ControlFlowAnalysis cfa = new ControlFlowAnalysis(compiler, false, false);
cfa.process(null, scope.getRoot());
this.cfg = cfa.getCfg();
Expand Down Expand Up @@ -987,9 +991,13 @@ private void analyzeFunctionFwd(NTIWorkset workset) {
}
break;
case SWITCH:
case THROW:
outEnv = analyzeExprFwd(n.getFirstChild(), inEnv).env;
break;
case THROW: {
outEnv = analyzeExprFwd(n.getFirstChild(), inEnv).env;
exitEnvs.add(outEnv);
break;
}
default:
if (NodeUtil.isStatement(n)) {
throw new RuntimeException("Unhandled statement type: " + n.getToken());
Expand Down Expand Up @@ -1118,7 +1126,7 @@ private void createSummary(NTIScope fn) {
checkArgument(!fnRoot.isFromExterns());
FunctionTypeBuilder builder = new FunctionTypeBuilder(this.commonTypes);
TypeEnv entryEnv = getEntryTypeEnv();
TypeEnv exitEnv = getInEnv(this.cfg.getImplicitReturn());
TypeEnv exitEnv = getExitTypeEnv();
if (exitEnv == null) {
// This function only exits with THROWs
exitEnv = envPutType(new TypeEnv(), RETVAL_ID, BOTTOM);
Expand Down Expand Up @@ -1412,7 +1420,7 @@ private EnvTypePair analyzeExprFwd(
String fnName = symbolTable.getFunInternalName(expr);
JSType fnType = envGetType(inEnv, fnName);
Preconditions.checkState(fnType != null, "Could not find type for %s", fnName);
TypeEnv outEnv = collectTypesForFreeVarsFwd(expr, inEnv);
TypeEnv outEnv = collectTypesForEscapedVarsFwd(expr, inEnv);
resultPair = new EnvTypePair(outEnv, fnType);
break;
}
Expand Down Expand Up @@ -2170,7 +2178,7 @@ private EnvTypePair analyzeInvocationFwd(
// Local function definitions will be type-checked more
// exactly using their summaries, and don't need deferred checks
if (this.currentScope.isLocalFunDef(calleeName)) {
tmpEnv = collectTypesForFreeVarsFwd(callee, tmpEnv);
tmpEnv = collectTypesForEscapedVarsFwd(callee, tmpEnv);
} else if (!origFunType.isGeneric()) {
JSType expectedRetType = requiredType;
println("Updating deferred check with ret: ", expectedRetType,
Expand Down Expand Up @@ -3611,13 +3619,17 @@ private TypeEnv updateLvalueTypeInEnv(
}
}

private TypeEnv collectTypesForFreeVarsFwd(Node n, TypeEnv env) {
/**
* Used when analyzing a scope that defines variables used in inner scopes.
* Returns a type environment that combines the types from all uses of a variable.
*/
private TypeEnv collectTypesForEscapedVarsFwd(Node n, TypeEnv env) {
checkArgument(n.isFunction() || (n.isName() && NodeUtil.isInvocationTarget(n)));
String fnName = n.isFunction() ? symbolTable.getFunInternalName(n) : n.getString();
NTIScope innerScope = this.currentScope.getScope(fnName);
FunctionType summary = summaries.get(innerScope).getFunType();
for (String freeVar : innerScope.getOuterVars()) {
if (innerScope.getDeclaredTypeOf(freeVar) == null) {
FunctionType summary = summaries.get(innerScope).getFunType();
JSType outerType = envGetType(env, freeVar);
if (outerType == null) {
outerType = UNKNOWN;
Expand All @@ -3634,10 +3646,23 @@ private TypeEnv collectTypesForFreeVarsFwd(Node n, TypeEnv env) {
freeVar, outerType.toString(), innerType.toString()));
}
// If n is a callee node, we only want to keep the type in the callee.
// If n is a function expression, we're not sure if it gets called,
// so we join the types.
env = envPutType(env, freeVar,
n.isFunction() ? JSType.join(innerType, outerType) : innerType);
// If n is a function expression, we don't know if it will get called, so we take the
// types from both scopes into account.
JSType freeVarType;
if (n.isFunction()) {
// If the type in our current scope is more precise, then trust it. The variable is
// defined in this scope, and it's more likely that this type is correct.
if (!outerType.isNullOrUndef() // only keep outerType for initialized variables
&& !outerType.isUnknown() && !innerType.isUnknown()
&& outerType.isSubtypeOf(innerType)) {
freeVarType = outerType;
} else {
freeVarType = JSType.join(innerType, outerType);
}
} else {
freeVarType = innerType;
}
env = envPutType(env, freeVar, freeVarType);
}
}
return env;
Expand Down Expand Up @@ -3753,8 +3778,7 @@ private EnvTypePair analyzeExprBwd(
return pair;
}
case INSTANCEOF: {
TypeEnv env = analyzeExprBwd(
expr.getLastChild(), outEnv, commonTypes.topFunction()).env;
TypeEnv env = analyzeExprBwd(expr.getLastChild(), outEnv, commonTypes.topFunction()).env;
EnvTypePair pair = analyzeExprBwd(expr.getFirstChild(), env);
pair.type = BOOLEAN;
return pair;
Expand Down Expand Up @@ -4761,10 +4785,19 @@ private static JSType specializeKeep2ndWhenBottom(
return specializedType.isBottom() ? fallback : specializedType;
}

TypeEnv getEntryTypeEnv() {
private TypeEnv getEntryTypeEnv() {
return getOutEnv(this.cfg.getEntry());
}

private TypeEnv getExitTypeEnv() {
if (!this.cfg.getImplicitReturn().getInEdges().isEmpty()) {
exitEnvs.add(getInEnv(this.cfg.getImplicitReturn()));
}
checkState(!exitEnvs.isEmpty(),
"There must be at least one exit env, either from a normal function exit or a throw.");
return TypeEnv.join(exitEnvs);
}

private static JSType firstNonBottom(JSType t1, JSType t2) {
if (t1.isBottom()) {
Preconditions.checkArgument(!t2.isBottom());
Expand Down
5 changes: 2 additions & 3 deletions src/com/google/javascript/jscomp/TypeTransformation.java
View file
Expand Up @@ -258,9 +258,8 @@ TypeI eval(Node ttlAst, ImmutableMap<String, TypeI> typeVars) {
@VisibleForTesting
TypeI eval(Node ttlAst, ImmutableMap<String, TypeI> typeVars,
ImmutableMap<String, String> nameVars) {
return evalInternal(
ttlAst,
new NameResolver(typeVars, nameVars));
TypeI result = evalInternal(ttlAst, new NameResolver(typeVars, nameVars));
return result.isBottom() ? getUnknownType() : result;
}

private TypeI evalInternal(Node ttlAst, NameResolver nameResolver) {
Expand Down
23 changes: 2 additions & 21 deletions src/com/google/javascript/jscomp/newtypes/NominalType.java
View file
Expand Up @@ -481,11 +481,8 @@ boolean isIObjectSubtypeOf(NominalType other) {

private boolean areTypeMapsCompatible(NominalType other) {
checkState(this.rawType.equals(other.rawType));
if (this.typeMap.isEmpty()) {
return other.instantiationIsUnknownOrIdentity();
}
if (other.typeMap.isEmpty()) {
return instantiationIsUnknownOrIdentity();
if (this.typeMap.isEmpty() || other.typeMap.isEmpty()) {
return true;
}
for (String typeVar : this.rawType.getTypeParameters()) {
Preconditions.checkState(this.typeMap.containsKey(typeVar),
Expand Down Expand Up @@ -547,22 +544,6 @@ static NominalType unifyUnknowns(NominalType nt1, NominalType nt2) {
return new NominalType(builder.build(), nt1.rawType);
}

private boolean instantiationIsUnknownOrIdentity() {
if (this.typeMap.isEmpty()) {
return true;
}
for (String typeVar : this.rawType.getTypeParameters()) {
Preconditions.checkState(this.typeMap.containsKey(typeVar),
"Type variable %s not in the domain: %s",
typeVar, this.typeMap.keySet());
JSType t = this.typeMap.get(typeVar);
if (!(t.isUnknown() || t.equals(JSType.fromTypeVar(getCommonTypes(), typeVar)))) {
return false;
}
}
return true;
}

private static NominalType joinTypeMaps(NominalType nt1, NominalType nt2) {
checkState(nt1.rawType.equals(nt2.rawType));
ImmutableMap.Builder<String, JSType> builder = ImmutableMap.builder();
Expand Down
69 changes: 69 additions & 0 deletions test/com/google/javascript/jscomp/NewTypeInferenceTTLTest.java
View file
Expand Up @@ -705,6 +705,7 @@ public void testMapRecord() {
}

public void testIsTemplatized() {
// 'Array' is raw Array, not Array<?>.
typeCheck(LINE_JOINER.join(
"/**",
" * @template T := cond(isTemplatized('Array'), 'number', 'string') =:",
Expand All @@ -713,6 +714,14 @@ public void testIsTemplatized() {
"function f() { return 'asdf'; }",
"var /** string */ s = f();"));

typeCheck(LINE_JOINER.join(
"/**",
" * @template T := cond(isTemplatized(type('Array', unknown())), 'number', 'string') =:",
" * @return {T}",
" */",
"function f() { return 123; }",
"var /** number */ n = f();"));

typeCheck(LINE_JOINER.join(
"/**",
" * @template T := cond(isTemplatized(type('Array', 'number')), 'number', 'string') =:",
Expand Down Expand Up @@ -1010,4 +1019,64 @@ public void testUsingOrdinaryTypeVariablesInTTL() {
"var /** !Number */ x = f(1);",
"var /** !String */ y = f('');"));
}

public void testCreateDomPattern() {
typeCheck(LINE_JOINER.join(
"/**",
" * @constructor",
" * @template T",
" */",
"function TagName() {}",
"/** @constructor */",
"function Foo() {}",
"/** @type {!TagName<!Foo>} */",
"TagName.FOO;",
"/**",
" * @template T",
" * @param {string|!TagName<T>} x",
" * @template R := cond(isUnknown(T), 'Object', T) =:",
" * @return {R}",
" */",
"function f(x) { return any(); }",
"var /** !Foo */ x = f(TagName.FOO);"));
}

public void testPromiseThen() {
typeCheck(LINE_JOINER.join(
"/**",
" * @interface",
" * @template TYPE",
" */",
"function MyThenable() {};",
"/**",
" * @param {?(function(this:THIS, TYPE): VALUE)=} opt_onFulfilled",
" * @param {?(function(this:THIS, *): *)=} opt_onRejected",
" * @param {THIS=} opt_context *",
" * @return {RESULT}",
" * @template VALUE",
" * @template THIS",
" *",
" * @template RESULT := type('MyPromise',",
" * cond(isUnknown(VALUE), unknown(),",
" * mapunion(VALUE, (V) =>",
" * cond(isTemplatized(V) && sub(rawTypeOf(V), 'IThenable'),",
" * templateTypeOf(V, 0),",
" * cond(sub(V, 'Thenable'),",
" * unknown(),",
" * V)))))",
" * =:",
" *",
" */",
"MyThenable.prototype.then = function(",
" opt_onFulfilled, opt_onRejected, opt_context) {};",
"/**",
" * @constructor",
" * @implements {MyThenable<TYPE>}",
" * @template TYPE",
" */",
"function MyPromise() {}",
"/** @override */",
"MyPromise.prototype.then = function(x, y, z) { return any(); };",
"var x = (new MyPromise).then(null);"));
}
}

0 comments on commit cb7cffa

Please sign in to comment.