Skip to content

Commit

Permalink
Ensuring local module names are unique in emit.
Browse files Browse the repository at this point in the history
Fixes #41 and #42.
  • Loading branch information
ahejlsberg committed Jul 19, 2014
1 parent ea46b97 commit b96b010
Show file tree
Hide file tree
Showing 36 changed files with 268 additions and 213 deletions.
13 changes: 11 additions & 2 deletions src/compiler/binder.ts
Expand Up @@ -32,6 +32,7 @@ module ts {
var parent: Node;
var container: Declaration;
var symbolCount = 0;
var lastLocals: Declaration;
var Symbol = objectAllocator.getSymbolConstructor();

if (!file.locals) {
Expand Down Expand Up @@ -162,6 +163,14 @@ module ts {
}
}

// All nodes with locals are kept on a linked list in declaration order. This list is used by the getLocalNameOfContainer function
// in the type checker to validate that the local name used for a container is unique.
function initializeLocals(node: Declaration) {
node.locals = {};
if (lastLocals) lastLocals.nextLocals = node;
lastLocals = node;
}

function bindDeclaration(node: Declaration, symbolKind: SymbolFlags, symbolExcludes: SymbolFlags) {
switch (container.kind) {
case SyntaxKind.ModuleDeclaration:
Expand Down Expand Up @@ -198,7 +207,7 @@ module ts {
declareSymbol(container.symbol.exports, container.symbol, node, symbolKind, symbolExcludes);
break;
}
if (symbolKind & SymbolFlags.HasLocals) node.locals = {};
if (symbolKind & SymbolFlags.HasLocals) initializeLocals(node);
var saveParent = parent;
var saveContainer = container;
parent = node;
Expand Down Expand Up @@ -232,7 +241,7 @@ module ts {
function bindAnonymousDeclaration(node: Node, symbolKind: SymbolFlags, name: string) {
var symbol = createSymbol(symbolKind, name);
addDeclarationToSymbol(symbol, node, symbolKind);
if (symbolKind & SymbolFlags.HasLocals) node.locals = {};
if (symbolKind & SymbolFlags.HasLocals) initializeLocals(node);
var saveParent = parent;
var saveContainer = container;
parent = node;
Expand Down
63 changes: 49 additions & 14 deletions src/compiler/checker.ts
Expand Up @@ -3794,7 +3794,7 @@ module ts {
}
return checkUntypedCall(node);
}
// If FuncExpr’s apparent type(section 3.8.1) is a function type, the call is a typed function call.
// If FuncExpr's apparent type(section 3.8.1) is a function type, the call is a typed function call.
// TypeScript employs overload resolution in typed function calls in order to support functions
// with multiple call signatures.
if (!signatures.length) {
Expand Down Expand Up @@ -3826,7 +3826,7 @@ module ts {
return checkUntypedCall(node);
}

// If ConstructExpr’s apparent type(section 3.8.1) is an object type with one or
// If ConstructExpr's apparent type(section 3.8.1) is an object type with one or
// more construct signatures, the expression is processed in the same manner as a
// function call, but using the construct signatures as the initial set of candidate
// signatures for overload resolution.The result type of the function call becomes
Expand All @@ -3847,7 +3847,7 @@ module ts {
return checkCall(node, constructSignatures);
}

// If ConstructExpr’s apparent type is an object type with no construct signatures but
// If ConstructExpr's apparent type is an object type with no construct signatures but
// one or more call signatures, the expression is processed as a function call. A compile-time
// error occurs if the result of the function call is not Void. The type of the result of the
// operation is Any.
Expand Down Expand Up @@ -5981,14 +5981,53 @@ module ts {

// Emitter support

function getModuleObjectName(node: ModuleDeclaration): string {
return getSourceTextOfNode(node.name);
}

function isExternalModule(symbol: Symbol): boolean {
return symbol.flags & SymbolFlags.ValueModule && symbol.declarations.length === 1 && symbol.declarations[0].kind === SyntaxKind.SourceFile;
}

function isNodeParentedBy(node: Node, parent: Node): boolean {

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Jul 21, 2014

Contributor

Ancestor, not parent

This comment has been minimized.

Copy link
@ahejlsberg

ahejlsberg Jul 21, 2014

Author Member

Agreed.

while (node) {
if (node === parent) return true;
node = node.parent;
}
return false;
}

function isUniqueLocalName(name: string, container: Node): boolean {
name = escapeIdentifier(name);
if (container.locals) {
for (var node = container; isNodeParentedBy(node, container); node = node.nextLocals) {
if (hasProperty(node.locals, name) && node.locals[name].flags & (SymbolFlags.Value | SymbolFlags.ExportValue)) {
return false;
}
}
}
return true;
}

function getLocalNameOfContainer(container: Declaration): string {

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Jul 21, 2014

Contributor

I like getNameOfEmittedModuleFunctionParameter

This comment has been minimized.

Copy link
@ahejlsberg

ahejlsberg Jul 21, 2014

Author Member

It's also used for Enums. I think I'm just going to keep the generic "Container".

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Jul 21, 2014

Contributor

Maybe getLocalNameOfEmittedContainer

var links = getNodeLinks(container);
if (!links.localModuleName) {
var name = container.name.text ? unescapeIdentifier(container.name.text) : "M";

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Jul 21, 2014

Contributor

Why M? can you comment that?

This comment has been minimized.

Copy link
@ahejlsberg

ahejlsberg Jul 21, 2014

Author Member

It's gone again.

while (!isUniqueLocalName(name, container)) {
name = "_" + name;
}
links.localModuleName = name;
}
return links.localModuleName;
}

function getLocalNameForSymbol(symbol: Symbol, location: Node): string {

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Jul 21, 2014

Contributor

What is this for? Can you comment?

This comment has been minimized.

Copy link
@ahejlsberg

ahejlsberg Jul 21, 2014

Author Member

Used in getExpressionNamePrefix to get the local name used for the given module/enum symbol.

var node = location;
while (node) {
if ((node.kind === SyntaxKind.ModuleDeclaration || node.kind === SyntaxKind.EnumDeclaration) && getSymbolOfNode(node) === symbol) {
return getLocalNameOfContainer(node);
}
node = node.parent;
}
Debug.fail("getLocalNameForSymbol failed");
}

function getExpressionNamePrefix(node: Identifier): string {
var symbol = getNodeLinks(node).resolvedSymbol;
if (symbol) {
Expand All @@ -5998,15 +6037,11 @@ module ts {
// kind of entity. SymbolFlags.ExportHasLocal encompasses all the kinds that we
// do NOT prefix.
var exportSymbol = getExportSymbolOfValueSymbolIfExported(symbol);
var isExportedSymbolFoundInLocalScope = exportSymbol !== symbol;
var shouldEmitExportWithoutPrefix = (exportSymbol.flags & SymbolFlags.ExportHasLocal) !== 0;
if (isExportedSymbolFoundInLocalScope && !shouldEmitExportWithoutPrefix) {
if (symbol !== exportSymbol && !(exportSymbol.flags & SymbolFlags.ExportHasLocal)) {
symbol = exportSymbol;
}

// symbol will have a parent if it is an export symbol
if (symbol.parent) {
return isExternalModule(symbol.parent) ? "exports" : symbolToString(symbol.parent);
return isExternalModule(symbol.parent) ? "exports" : getLocalNameForSymbol(getParentOfSymbol(symbol), node.parent);
}
}
}
Expand Down Expand Up @@ -6093,7 +6128,7 @@ module ts {
function invokeEmitter() {
var resolver: EmitResolver = {
getProgram: () => program,
getModuleObjectName: getModuleObjectName,
getLocalNameOfContainer: getLocalNameOfContainer,
getExpressionNamePrefix: getExpressionNamePrefix,
getPropertyAccessSubstitution: getPropertyAccessSubstitution,
getExportAssignmentName: getExportAssignmentName,
Expand Down
14 changes: 9 additions & 5 deletions src/compiler/emitter.ts
Expand Up @@ -1043,7 +1043,7 @@ module ts {
emitStart(node.name);
if (node.flags & NodeFlags.Export) {
var container = getContainingModule(node);
write(container ? resolver.getModuleObjectName(container) : "exports");
write(container ? resolver.getLocalNameOfContainer(container) : "exports");
write(".");
}
emitNode(node.name);
Expand Down Expand Up @@ -1444,16 +1444,18 @@ module ts {
writeLine();
emitStart(node);
write("(function (");
emit(node.name);
emitStart(node.name);
write(resolver.getLocalNameOfContainer(node));
emitEnd(node.name);
write(") {");
increaseIndent();
scopeEmitStart(node);
forEach(node.members, member => {
writeLine();
emitStart(member);
emitNode(node.name);
write(resolver.getLocalNameOfContainer(node));
write("[");
emitNode(node.name);
write(resolver.getLocalNameOfContainer(node));
write("[");
emitQuotedIdentifier(member.name);
write("] = ");
Expand Down Expand Up @@ -1509,7 +1511,9 @@ module ts {
}
emitStart(node);
write("(function (");
emit(node.name);
emitStart(node.name);
write(resolver.getLocalNameOfContainer(node));
emitEnd(node.name);
write(") ");
if (node.body.kind === SyntaxKind.ModuleBlock) {
emit(node.body);
Expand Down
7 changes: 6 additions & 1 deletion src/compiler/parser.ts
Expand Up @@ -62,7 +62,12 @@ module ts {

// Add an extra underscore to identifiers that start with two underscores to avoid issues with magic names like '__proto__'
export function escapeIdentifier(identifier: string): string {
return identifier.charCodeAt(0) === CharacterCodes._ && identifier.charCodeAt(1) === CharacterCodes._ ? "_" + identifier : identifier;
return identifier.length >= 2 && identifier.charCodeAt(0) === CharacterCodes._ && identifier.charCodeAt(1) === CharacterCodes._ ? "_" + identifier : identifier;
}

// Remove extra underscore from escaped identifier
export function unescapeIdentifier(identifier: string): string {
return identifier.length >= 3 && identifier.charCodeAt(0) === CharacterCodes._ && identifier.charCodeAt(1) === CharacterCodes._ && identifier.charCodeAt(2) === CharacterCodes._ ? identifier.substr(1) : identifier;
}

// Return display name of an identifier
Expand Down
40 changes: 20 additions & 20 deletions src/compiler/scanner.ts
Expand Up @@ -142,18 +142,18 @@ module ts {
As per ECMAScript Language Specification 3th Edition, Section 7.6: Identifiers
IdentifierStart ::
Can contain Unicode 3.0.0 categories:
Uppercase letter (Lu),
Lowercase letter (Ll),
Titlecase letter (Lt),
Modifier letter (Lm),
Other letter (Lo), or
Letter number (Nl).
Uppercase letter (Lu),
Lowercase letter (Ll),
Titlecase letter (Lt),
Modifier letter (Lm),
Other letter (Lo), or
Letter number (Nl).
IdentifierPart :: =
Can contain IdentifierStart + Unicode 3.0.0 categories:
Non-spacing mark (Mn),
Combining spacing mark (Mc),
Decimal number (Nd), or
Connector punctuation (Pc).
Non-spacing mark (Mn),
Combining spacing mark (Mc),
Decimal number (Nd), or
Connector punctuation (Pc).
Codepoint ranges for ES3 Identifiers are extracted from the Unicode 3.0.0 specification at:
http://www.unicode.org/Public/3.0-Update/UnicodeData-3.0.0.txt
Expand All @@ -165,18 +165,18 @@ module ts {
As per ECMAScript Language Specification 5th Edition, Section 7.6: ISyntaxToken Names and Identifiers
IdentifierStart ::
Can contain Unicode 6.2 categories:
Uppercase letter (Lu),
Lowercase letter (Ll),
Titlecase letter (Lt),
Modifier letter (Lm),
Other letter (Lo), or
Letter number (Nl).
Uppercase letter (Lu),
Lowercase letter (Ll),
Titlecase letter (Lt),
Modifier letter (Lm),
Other letter (Lo), or
Letter number (Nl).
IdentifierPart ::
Can contain IdentifierStart + Unicode 6.2 categories:
Non-spacing mark (Mn),
Combining spacing mark (Mc),
Decimal number (Nd),
Connector punctuation (Pc),
Non-spacing mark (Mn),
Combining spacing mark (Mc),
Decimal number (Nd),
Connector punctuation (Pc),
<ZWNJ>, or
<ZWJ>.
Expand Down
8 changes: 5 additions & 3 deletions src/compiler/types.ts
Expand Up @@ -241,6 +241,7 @@ module ts {
parent?: Node; // Parent node (initialized by binding)
symbol?: Symbol; // Symbol declared by node (initialized by binding)
locals?: SymbolTable; // Locals associated with node (initialized by binding)
nextLocals?: Node; // Next node in declaration order with locals (initialized by binding)
}

export interface NodeArray<T> extends Array<T>, TextRange { }
Expand Down Expand Up @@ -612,7 +613,7 @@ module ts {

export interface EmitResolver {
getProgram(): Program;
getModuleObjectName(node: ModuleDeclaration): string;
getLocalNameOfContainer(container: Declaration): string;
getExpressionNamePrefix(node: Identifier): string;
getPropertyAccessSubstitution(node: PropertyAccess): string;
getExportAssignmentName(node: SourceFile): string;
Expand Down Expand Up @@ -688,7 +689,7 @@ module ts {

ExportHasLocal = Function | Class | Enum | ValueModule,

HasLocals = Function | Module | Method | Constructor | Accessor | Signature,
HasLocals = Function | Enum | Module | Method | Constructor | Accessor | Signature,

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Jul 21, 2014

Contributor

Add a comment for what this means, now that the meaning has subtly changed because Enums are added

This comment has been minimized.

Copy link
@ahejlsberg

ahejlsberg Jul 21, 2014

Author Member

Enum is gone again.

HasExports = Class | Enum | Module,
HasMembers = Class | Interface | TypeLiteral | ObjectLiteral,

Expand Down Expand Up @@ -740,7 +741,8 @@ module ts {
flags?: NodeCheckFlags; // Set of flags specific to Node
enumMemberValue?: number; // Constant value of enum member
isIllegalTypeReferenceInConstraint?: boolean; // Is type reference in constraint refers to the type parameter from the same list
isVisible?: boolean; // Is this node visible
isVisible?: boolean; // Is this node visible
localModuleName?: string; // Local name for module instance
}

export enum TypeFlags {
Expand Down
Expand Up @@ -35,19 +35,19 @@ var ag2 = new A.A2<string, number>();

//// [ModuleWithExportedAndNonExportedClasses.js]
var A;
(function (A) {
(function (_A) {
var A = (function () {
function A() {
}
return A;
})();
A.A = A;
_A.A = A;
var AG = (function () {
function AG() {
}
return AG;
})();
A.AG = AG;
_A.AG = AG;
var A2 = (function () {
function A2() {
}
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/cloduleWithRecursiveReference.js
Expand Up @@ -16,8 +16,8 @@ var M;
return C;
})();
M.C = C;
(function (C) {
C.C = M.C;
(function (_C) {
_C.C = M.C;
})(M.C || (M.C = {}));
var C = M.C;
})(M || (M = {}));
Expand Up @@ -25,31 +25,31 @@ module M {

//// [collisionCodeGenModuleWithConstructorChildren.js]
var M;
(function (M) {
M.x = 3;
(function (_M) {
_M.x = 3;
var c = (function () {
function c(M, p) {
if (p === void 0) { p = M.x; }
if (p === void 0) { p = _M.x; }
}
return c;
})();
})(M || (M = {}));
var M;
(function (M) {
(function (_M) {
var d = (function () {
function d(M, p) {
if (p === void 0) { p = M.x; }
if (p === void 0) { p = _M.x; }
this.M = M;
}
return d;
})();
})(M || (M = {}));
var M;
(function (M) {
(function (_M) {
var d2 = (function () {
function d2() {
var M = 10;
var p = M.x;
var p = _M.x;
}
return d2;
})();
Expand Down

0 comments on commit b96b010

Please sign in to comment.