Local types #3266
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -338,7 +338,15 @@ module ts { | |
| // Locals of a source file are not in scope (because they get merged into the global symbol table) | ||
| if (location.locals && !isGlobalSourceFile(location)) { | ||
| if (result = getSymbol(location.locals, name, meaning)) { | ||
| // Type parameters of a function are in scope in the entire function declaration, including the parameter | ||
| // list and return type. However, local types are only in scope in the function body. | ||
| if (!(meaning & SymbolFlags.Type) || | ||
| !(result.flags & (SymbolFlags.Type & ~SymbolFlags.TypeParameter)) || | ||
| !isFunctionLike(location) || | ||
| lastLocation === (<FunctionLikeDeclaration>location).body) { | ||
|
There was a problem hiding this comment. Why is this necessary? I thought block scope declarations (like local types) are in the If this condition is necessary, then isn't this broken for let and const as well? There was a problem hiding this comment. Top-level block scoped declarations are always in the There was a problem hiding this comment. So a top level There was a problem hiding this comment. Yes, a top-level There was a problem hiding this comment. My second question has to do with this case function f(x = a) {
let a = 0;
}
There was a problem hiding this comment. There is no logic to skip that symbol, but there is a check that x's initializer only reference identifiers declared before x ( There was a problem hiding this comment. Oh so a top level interface Foo { }
var x: Foo;
function bar(param: Foo = x) { // x reference is an error, but Foo references the outer interface
interface Foo {
prop;
}
var x: Foo;
}There was a problem hiding this comment. Initializers are evaluated inside the function in the generated code. We therefore need identifiers in expressions to bind to locals, even if that is subsequently deemed an error. If the locals weren't in scope at all, the following would produce no errors, but it wouldn't actually work: var a = 1;
function f(x = a) { // We don't want this to bind to the global a
let a = 2;
}We don't have any such problem with local types, so we can just take them out of scope. There was a problem hiding this comment. Oh okay. That makes sense, thanks for explaining. It might be worth adding a comment to that effect. |
||
| break loop; | ||
| } | ||
| result = undefined; | ||
| } | ||
| } | ||
| switch (location.kind) { | ||
|
|
@@ -2465,30 +2473,68 @@ module ts { | |
| } | ||
| } | ||
|
|
||
| // Appends the type parameters given by a list of declarations to a set of type parameters and returns the resulting set. | ||
| // The function allocates a new array if the input type parameter set is undefined, but otherwise it modifies the set | ||
| // in-place and returns the same array. | ||
| function appendTypeParameters(typeParameters: TypeParameter[], declarations: TypeParameterDeclaration[]): TypeParameter[] { | ||
| for (let declaration of declarations) { | ||
| let tp = getDeclaredTypeOfTypeParameter(getSymbolOfNode(declaration)); | ||
| if (!typeParameters) { | ||
|
There was a problem hiding this comment. You could just move this check outside the loop. if (!typeParameters && declarations.length > 0) {
typeParameters = [];
}There was a problem hiding this comment. What's there is more efficient because it creates the single element array in one go vs. first creating an empty array and then pushing. There was a problem hiding this comment. Based on what this function does, it looks like it should really be written as: function appendTypeParameters(declarations: TypeParameterDeclaration[], typeParameters?: TypeParameters[]) {
...The declarations are the actual input. The 'typeParameters' array is the output. We don't generally put output parameters before intput parameters. Also, as the second parameter is optional (based on the There was a problem hiding this comment. No, There was a problem hiding this comment. @ahejlsberg It would be best to document that for consumers so that they can quickly eye the function and know this. |
||
| typeParameters = [tp]; | ||
| } | ||
| else if (!contains(typeParameters, tp)) { | ||
| typeParameters.push(tp); | ||
| } | ||
| } | ||
| return typeParameters; | ||
| } | ||
|
|
||
| // Appends the outer type parameters of a node to a set of type parameters and returns the resulting set. The function | ||
| // allocates a new array if the input type parameter set is undefined, but otherwise it modifies the set in-place and | ||
| // returns the same array. | ||
| function appendOuterTypeParameters(typeParameters: TypeParameter[], node: Node): TypeParameter[]{ | ||
| while (true) { | ||
| node = node.parent; | ||
| if (!node) { | ||
| return typeParameters; | ||
| } | ||
| if (node.kind === SyntaxKind.ClassDeclaration || node.kind === SyntaxKind.FunctionDeclaration || | ||
| node.kind === SyntaxKind.FunctionExpression || node.kind === SyntaxKind.MethodDeclaration || | ||
| node.kind === SyntaxKind.ArrowFunction) { | ||
|
There was a problem hiding this comment. can you use two newlines after { after a wrapped conditional. Right now what happens is that hte next statement is indented exactly as far as the conditional, making it look like it continues the conditional. Providing the extra newline makes it clear that this is a statement inside the body of the code, and is not part of the if-expression. |
||
| let declarations = (<ClassDeclaration | FunctionLikeDeclaration>node).typeParameters; | ||
| if (declarations) { | ||
| return appendTypeParameters(appendOuterTypeParameters(typeParameters, node), declarations); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // The outer type parameters are those defined by enclosing generic classes, methods, or functions. | ||
| function getOuterTypeParametersOfClassOrInterface(symbol: Symbol): TypeParameter[] { | ||
| var kind = symbol.flags & SymbolFlags.Class ? SyntaxKind.ClassDeclaration : SyntaxKind.InterfaceDeclaration; | ||
|
There was a problem hiding this comment. Probably better to check SymbolFlags.Interface as well There was a problem hiding this comment. No need to: You can't declare classes or interfaces inside interfaces. There was a problem hiding this comment. I meant checking There was a problem hiding this comment. Well, the method is only ever called for a class or interface, so if it isn't a class it must be an interface. There was a problem hiding this comment. we'll probably need to come back to this when we do class expressions |
||
| return appendOuterTypeParameters(undefined, getDeclarationOfKind(symbol, kind)); | ||
|
There was a problem hiding this comment. named parameter for the "undefined". |
||
| } | ||
|
|
||
| // The local type parameters are the combined set of type parameters from all declarations of the class or interface. | ||
|
There was a problem hiding this comment. "all declarations of the class" - I'm a little confused, when can we get multiple declarations of a class? There was a problem hiding this comment. Unless you mean all class and interface declarations of a symbol. There was a problem hiding this comment. We currently only permit multiple declarations of interfaces, but we'll likely allow it for ambient classes as part of the work of changing the core type declarations in lib.d.ts to use class declarations. |
||
| function getLocalTypeParametersOfClassOrInterface(symbol: Symbol): TypeParameter[] { | ||
| let result: TypeParameter[]; | ||
| for (let node of symbol.declarations) { | ||
| if (node.kind === SyntaxKind.InterfaceDeclaration || node.kind === SyntaxKind.ClassDeclaration) { | ||
| let declaration = <InterfaceDeclaration>node; | ||
| if (declaration.typeParameters) { | ||
| result = appendTypeParameters(result, declaration.typeParameters); | ||
| } | ||
| } | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| // The full set of type parameters for a generic class or interface type consists of its outer type parameters plus | ||
| // its locally declared type parameters. | ||
| function getTypeParametersOfClassOrInterface(symbol: Symbol): TypeParameter[] { | ||
| return concatenate(getOuterTypeParametersOfClassOrInterface(symbol), getLocalTypeParametersOfClassOrInterface(symbol)); | ||
| } | ||
|
|
||
| function getBaseTypes(type: InterfaceType): ObjectType[] { | ||
| let typeWithBaseTypes = <InterfaceTypeWithBaseTypes>type; | ||
| if (!typeWithBaseTypes.baseTypes) { | ||
|
|
@@ -2558,10 +2604,13 @@ module ts { | |
| if (!links.declaredType) { | ||
| let kind = symbol.flags & SymbolFlags.Class ? TypeFlags.Class : TypeFlags.Interface; | ||
| let type = links.declaredType = <InterfaceType>createObjectType(kind, symbol); | ||
| let outerTypeParameters = getOuterTypeParametersOfClassOrInterface(symbol); | ||
| let localTypeParameters = getLocalTypeParametersOfClassOrInterface(symbol); | ||
| if (outerTypeParameters || localTypeParameters) { | ||
| type.flags |= TypeFlags.Reference; | ||
| type.typeParameters = concatenate(outerTypeParameters, localTypeParameters); | ||
| type.outerTypeParameters = outerTypeParameters; | ||
| type.localTypeParameters = localTypeParameters; | ||
| (<GenericType>type).instantiations = {}; | ||
| (<GenericType>type).instantiations[getTypeListId(type.typeParameters)] = <GenericType>type; | ||
| (<GenericType>type).target = <GenericType>type; | ||
|
|
@@ -2751,12 +2800,12 @@ module ts { | |
| return map(baseSignatures, baseSignature => { | ||
| let signature = baseType.flags & TypeFlags.Reference ? | ||
| getSignatureInstantiation(baseSignature, (<TypeReference>baseType).typeArguments) : cloneSignature(baseSignature); | ||
| signature.typeParameters = classType.localTypeParameters; | ||
| signature.resolvedReturnType = classType; | ||
| return signature; | ||
| }); | ||
| } | ||
| return [createSignature(undefined, classType.localTypeParameters, emptyArray, classType, 0, false, false)]; | ||
| } | ||
|
|
||
| function createTupleTypeMemberSymbols(memberTypes: Type[]): SymbolTable { | ||
|
|
@@ -3105,7 +3154,7 @@ module ts { | |
| let links = getNodeLinks(declaration); | ||
| if (!links.resolvedSignature) { | ||
| let classType = declaration.kind === SyntaxKind.Constructor ? getDeclaredTypeOfClassOrInterface((<ClassDeclaration>declaration.parent).symbol) : undefined; | ||
| let typeParameters = classType ? classType.localTypeParameters : | ||
| declaration.typeParameters ? getTypeParametersFromDeclaration(declaration.typeParameters) : undefined; | ||
| let parameters: Symbol[] = []; | ||
| let hasStringLiterals = false; | ||
|
|
@@ -3422,12 +3471,18 @@ module ts { | |
| else { | ||
| type = getDeclaredTypeOfSymbol(symbol); | ||
| if (type.flags & (TypeFlags.Class | TypeFlags.Interface) && type.flags & TypeFlags.Reference) { | ||
| // In a type reference, the outer type parameters of the referenced class or interface are automatically | ||
| // supplied as type arguments and the type reference only specifies arguments for the local type parameters | ||
|
There was a problem hiding this comment. Why do the outer type parameters have to be supplied as type arguments to the TypeReference? Why can't we just create a type reference that doesn't have any mappings for the outer type parameters? All the mappers for the outer type parameters will just end up mapping things to themselves anyway. There was a problem hiding this comment. That is true only for written type references. Type references created by instantiations will indeed end up having the type parameters mapped to different types. For example: function f<T>() {
class C {
x: T;
}
var c: C; // Really a C<T>
c = new C();
return c;
}
var c = f<string>(); // Type of c is C<string>
var s = c.x; // stringThere was a problem hiding this comment. Yes, but why is it necessary to map them at all for written type references? Why can't we just have them unmapped, and then later in an instantiation, they will get mapped? There was a problem hiding this comment. We would then need There was a problem hiding this comment. I thought the appending of the outer type parameters to the type parameters of the class happens in getDeclaredTypeOfClassOrInterface. I was asking why type arguments need to be supplied for those outer type parameters. Why can't the type reference have these outer type parameters as type parameters, but with no type arguments for them. I thought that if an instantiation is missing type arguments for its type parameters, the mapper will just return the original type parameter. I think what I am asking is: Is not passing the outer type parameters as type arguments a suitable optimization? There was a problem hiding this comment. Here's an example. Enjoy! The infinite expansion is stopped by our isDeeplyNestedGeneric check for type references, but I don't know of a mechanism that could stop what instantiateType would do. function f<T>() {
class C {
x = f<C>();
}
return new C();
}
function g<T>() {
class D {
x = g<D>();
}
return new D();
}
var a = f<string>();
var b = g<string>();
a = b;
b = a;There was a problem hiding this comment. Oh how interesting! I see how the cyclic instantiation happens. Btw, I came up with another example of this, based on a combination of the example you just gave, and the one from #3237. It looks like this: function foo<T>() {
var z = foo();
var y: {
y2: typeof z
};
return y;
}
function bar<T>() {
var z = bar();
var y: {
y2: typeof z;
}
return y;
}
var a = foo<number>();
var b = bar<number>();
a = b;Seems to stack overflow for me. Does your change fix this? What's interesting here is that T is never referenced in foo or bar. Its mere presence causes us to instantiate infinitely. Although I admit I don't really understand what is going on here. Perhaps this is more related to #3237, because we don't reuse the mapper between signature instantiations. There was a problem hiding this comment. No, my change doesn't affect this. Looks like the issue is runaway recursion in There was a problem hiding this comment. Hmm. Even if we put in place a cache for already instantiated signatures, I think you can use this as the same kind of infinitely expanding generics device as type references. For example, There was a problem hiding this comment. I've opened up #3309 for this |
||
| // of the class or interface. | ||
| let localTypeParameters = (<InterfaceType>type).localTypeParameters; | ||
| let expectedTypeArgCount = localTypeParameters ? localTypeParameters.length : 0; | ||
| let typeArgCount = node.typeArguments ? node.typeArguments.length : 0; | ||
| if (typeArgCount === expectedTypeArgCount) { | ||
| type = createTypeReference(<GenericType>type, concatenate((<InterfaceType>type).outerTypeParameters, | ||
|
There was a problem hiding this comment. By the way, I think there is an optimization you can do here. If typeArgCount and expectedTypeArgCount are zero, then |
||
| map(node.typeArguments, getTypeFromTypeNode))); | ||
| } | ||
| else { | ||
| error(node, Diagnostics.Generic_type_0_requires_1_type_argument_s, typeToString(type, /*enclosingDeclaration*/ undefined, TypeFormatFlags.WriteArrayAsGenericType), expectedTypeArgCount); | ||
| type = undefined; | ||
| } | ||
| } | ||
|
|
@@ -3884,7 +3939,7 @@ module ts { | |
| return mapper(<TypeParameter>type); | ||
| } | ||
| if (type.flags & TypeFlags.Anonymous) { | ||
| return type.symbol && type.symbol.flags & (SymbolFlags.Function | SymbolFlags.Method | SymbolFlags.Class | SymbolFlags.TypeLiteral | SymbolFlags.ObjectLiteral) ? | ||
|
There was a problem hiding this comment. Is this for anonymous class expressions? There was a problem hiding this comment. No, it's for the anonymous type created for the static side of a class. There was a problem hiding this comment. Ah yes, because previously the static side of a class could never get instantiated. |
||
| instantiateAnonymousType(<ObjectType>type, mapper) : type; | ||
| } | ||
| if (type.flags & TypeFlags.Reference) { | ||
|
|
@@ -8968,7 +9023,6 @@ module ts { | |
| function checkFunctionDeclaration(node: FunctionDeclaration): void { | ||
| if (produceDiagnostics) { | ||
| checkFunctionLikeDeclaration(node) || | ||
| checkGrammarFunctionName(node.name) || | ||
| checkGrammarForGenerator(node); | ||
|
|
||
|
|
@@ -9330,7 +9384,7 @@ module ts { | |
|
|
||
| function checkVariableStatement(node: VariableStatement) { | ||
| // Grammar checking | ||
| checkGrammarDecorators(node) || checkGrammarModifiers(node) || checkGrammarVariableDeclarationList(node.declarationList) || checkGrammarForDisallowedLetOrConstStatement(node); | ||
|
|
||
| forEach(node.declarationList.declarations, checkSourceElement); | ||
| } | ||
|
|
@@ -9992,10 +10046,6 @@ module ts { | |
| function checkClassDeclaration(node: ClassDeclaration) { | ||
| checkGrammarDeclarationNameInStrictMode(node); | ||
| // Grammar checking | ||
| if (!node.name && !(node.flags & NodeFlags.Default)) { | ||
| grammarErrorOnFirstToken(node, Diagnostics.A_class_declaration_without_the_default_modifier_must_have_a_name); | ||
| } | ||
|
|
@@ -12247,19 +12297,28 @@ module ts { | |
| case SyntaxKind.MethodDeclaration: | ||
| case SyntaxKind.MethodSignature: | ||
| case SyntaxKind.IndexSignature: | ||
| case SyntaxKind.ModuleDeclaration: | ||
| case SyntaxKind.ImportDeclaration: | ||
| case SyntaxKind.ImportEqualsDeclaration: | ||
| case SyntaxKind.ExportDeclaration: | ||
| case SyntaxKind.ExportAssignment: | ||
| case SyntaxKind.Parameter: | ||
| break; | ||
| case SyntaxKind.ClassDeclaration: | ||
| case SyntaxKind.InterfaceDeclaration: | ||
| case SyntaxKind.VariableStatement: | ||
| case SyntaxKind.FunctionDeclaration: | ||
| case SyntaxKind.TypeAliasDeclaration: | ||
| if (node.modifiers && node.parent.kind !== SyntaxKind.ModuleBlock && node.parent.kind !== SyntaxKind.SourceFile) { | ||
| return grammarErrorOnFirstToken(node, Diagnostics.Modifiers_cannot_appear_here); | ||
| } | ||
| break; | ||
| case SyntaxKind.EnumDeclaration: | ||
| if (node.modifiers && (node.modifiers.length > 1 || node.modifiers[0].kind !== SyntaxKind.ConstKeyword) && | ||
|
There was a problem hiding this comment. Break into multiple checks. The mix of |
||
| node.parent.kind !== SyntaxKind.ModuleBlock && node.parent.kind !== SyntaxKind.SourceFile) { | ||
|
There was a problem hiding this comment. Consider: extract common helper for checking that the parent is a module or source file (repeated above as well, as well as in "shouldWriteTypeOfFunctionSymbol", "checkGrammarModifiers", "checkGrammarStatementInAmbientContext", ). |
||
| return grammarErrorOnFirstToken(node, Diagnostics.Modifiers_cannot_appear_here); | ||
| } | ||
| break; | ||
| default: | ||
| return false; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2168,9 +2168,5 @@ | |
| "'class' expressions are not currently supported.": { | ||
| "category": "Error", | ||
| "code": 9003 | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess classes are already block scoped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct.