From 429074964d35a36142bfe8aa41ac0dfd3749f7d4 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Fri, 25 Jan 2019 07:35:12 -0700 Subject: [PATCH 01/24] initial hoisting implementation (needs tests) --- src/LuaTransformer.ts | 107 +++++++++++++++++++++++-------- src/targets/LuaTransformer.52.ts | 2 +- 2 files changed, 82 insertions(+), 27 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index e4dab185d..13273ba35 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -12,14 +12,19 @@ export type StatementVisitResult = tstl.Statement | tstl.Statement[] | undefined export type ExpressionVisitResult = tstl.Expression | undefined; export enum ScopeType { + File, Function, Switch, Loop, + Conditional, + Block, } interface Scope { type: ScopeType; id: number; + locals: tstl.Identifier[]; + functions: tstl.AssignmentStatement[]; } export class LuaTransformer { @@ -62,7 +67,6 @@ export class LuaTransformer { } public setupState(): void { - this.scopeStack = []; this.genVarCounter = 0; this.currentSourceFile = undefined; this.isModule = false; @@ -74,6 +78,7 @@ export class LuaTransformer { // TODO make all other methods private??? public transformSourceFile(node: ts.SourceFile): [tstl.Block, Set] { this.setupState(); + this.pushScope(ScopeType.File); this.currentSourceFile = node; this.isModule = tsHelper.isFileModule(node); @@ -93,6 +98,7 @@ export class LuaTransformer { [tstl.createIdentifier("exports")] )); } + this.popScope(statements); return [tstl.createBlock(statements, undefined, node), this.luaLibFeatureSet]; } @@ -174,7 +180,10 @@ export class LuaTransformer { } public transformScopeBlock(block: ts.Block): tstl.DoStatement { - return tstl.createDoStatement(this.transformStatements(block.statements), undefined, block); + this.pushScope(ScopeType.Block); + const statements = this.transformStatements(block.statements); + this.popScope(statements); + return tstl.createDoStatement(statements, undefined, block); } public transformImportDeclaration(statement: ts.ImportDeclaration): StatementVisitResult { @@ -747,7 +756,7 @@ export class LuaTransformer { const bodyStatements = this.transformStatements(body.statements); - this.popScope(); + this.popScope(bodyStatements); return headerStatements.concat(bodyStatements); } @@ -977,7 +986,7 @@ export class LuaTransformer { } else { return this.createLocalOrExportedOrGlobalDeclaration( identifierName, - tstl.createNilLiteral(), + undefined, undefined, statement ); @@ -1120,13 +1129,19 @@ export class LuaTransformer { } public transformIfStatement(statement: ts.IfStatement): tstl.IfStatement { + this.pushScope(ScopeType.Conditional); const condition = this.transformExpression(statement.expression); - const ifBlock = tstl.createBlock(this.transformBlockOrStatement(statement.thenStatement)); + const statements = this.transformBlockOrStatement(statement.thenStatement); + this.popScope(statements); + const ifBlock = tstl.createBlock(statements); if (statement.elseStatement) { if (ts.isIfStatement(statement.elseStatement)) { return tstl.createIfStatement(condition, ifBlock, this.transformIfStatement(statement.elseStatement)); } else { - const elseBlock = tstl.createBlock(this.transformBlockOrStatement(statement.elseStatement)); + this.pushScope(ScopeType.Conditional); + const elseStatements = this.transformBlockOrStatement(statement.elseStatement); + this.popScope(elseStatements); + const elseBlock = tstl.createBlock(elseStatements); return tstl.createIfStatement(condition, ifBlock, elseBlock); } } @@ -1181,7 +1196,9 @@ export class LuaTransformer { public transformForOfInitializer(initializer: ts.ForInitializer, expression: tstl.Expression): tstl.Statement { if (ts.isVariableDeclarationList(initializer)) { // Declaration of new variable + this.pushScope(ScopeType.Loop); // Counter hoisting - there's probably a better way to handle this const variableDeclarations = this.transformVariableDeclaration(initializer.declarations[0]); + this.popScope(variableDeclarations); if (ts.isArrayBindingPattern(initializer.declarations[0].name)) { expression = this.createUnpackCall(expression, initializer); } @@ -1207,7 +1224,10 @@ export class LuaTransformer { loop: ts.WhileStatement | ts.DoStatement | ts.ForStatement | ts.ForOfStatement | ts.ForInOrOfStatement ): tstl.Statement[] { - return this.transformBlockOrStatement(loop.statement); + this.pushScope(ScopeType.Loop); + const statements = this.transformBlockOrStatement(loop.statement); + this.popScope(statements); + return statements; } public transformBlockOrStatement(statement: ts.Statement): tstl.Statement[] { @@ -1444,7 +1464,7 @@ export class LuaTransformer { statements.push(tstl.createLabelStatement(`${switchName}_end`)); - this.popScope(); + this.popScope(statements); return statements; } @@ -3127,7 +3147,7 @@ export class LuaTransformer { private createLocalOrExportedOrGlobalDeclaration( lhs: tstl.Identifier | tstl.Identifier[], - rhs: tstl.Expression, + rhs?: tstl.Expression, parent?: tstl.Node, tsOriginal?: ts.Node ): tstl.Statement[] @@ -3146,23 +3166,18 @@ export class LuaTransformer { && (tsOriginal.parent.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) !== 0; if (this.isModule || this.currentNamespace || insideFunction || isLetOrConst) { // local - const isFunction = - tsOriginal - && (ts.isFunctionDeclaration(tsOriginal) - || (ts.isVariableDeclaration(tsOriginal) && ts.isFunctionLike(tsOriginal.initializer))); - if (isFunction) { - // Separate declaration from assignment for functions to allow recursion - return [ - tstl.createVariableDeclarationStatement(lhs, undefined, parent, tsOriginal), - tstl.createAssignmentStatement(lhs, rhs, parent, tsOriginal), - ]; + if (ts.isFunctionDeclaration(tsOriginal)) { + // hoist function declarations + this.addScopeFunction(lhs as tstl.Identifier, rhs as tstl.FunctionExpression, parent, tsOriginal); + return undefined; } else { - return [tstl.createVariableDeclarationStatement(lhs, rhs, parent, tsOriginal)]; + this.addScopeLocals(lhs, isLetOrConst ? undefined : ScopeType.Function); } - - } else { - // global + } + if (rhs) { return [tstl.createAssignmentStatement(lhs, rhs, parent, tsOriginal)]; + } else { + return []; } } @@ -3257,12 +3272,52 @@ export class LuaTransformer { } protected pushScope(scopeType: ScopeType): void { - this.scopeStack.push({type: scopeType, id: this.genVarCounter}); + this.scopeStack.push({type: scopeType, id: this.genVarCounter, locals: [], functions: []}); this.genVarCounter++; } - protected popScope(): Scope { - return this.scopeStack.pop(); + protected popScope(statements: tstl.Statement[]): Scope { + const scope = this.scopeStack.pop(); + + statements.unshift(...scope.functions); + + if (scope.locals.length > 0) { + const declaration = tstl.createVariableDeclarationStatement(scope.locals); + statements.unshift(declaration); + } + + return scope; + } + + protected addScopeLocals(locals: tstl.Identifier | tstl.Identifier[], scopeType?: ScopeType): void { + let scope: Scope; + if (!scopeType) { + scope = this.peekScope(); + } else { + for (let i = this.scopeStack.length - 1; i >= 0; --i) { + if (this.scopeStack[i].type === scopeType) { + scope = this.scopeStack[i]; + break; + } + } + scope = scope || this.scopeStack[0]; // Default to file scope + } + if (Array.isArray(locals)) { + scope.locals.push(...locals); + } else { + scope.locals.push(locals); + } + } + + protected addScopeFunction( + name: tstl.Identifier, + func: tstl.FunctionExpression, + parent?: tstl.Node, + tsOriginal?: ts.Node + ): void { + const scope = this.peekScope(); + scope.locals.push(name); + scope.functions.push(tstl.createAssignmentStatement(name, func, parent, tsOriginal)); } private statementVisitResultToStatementArray(visitResult: StatementVisitResult): tstl.Statement[] { diff --git a/src/targets/LuaTransformer.52.ts b/src/targets/LuaTransformer.52.ts index 5be693c57..9868d1992 100644 --- a/src/targets/LuaTransformer.52.ts +++ b/src/targets/LuaTransformer.52.ts @@ -14,7 +14,7 @@ export class LuaTransformer52 extends LuaTransformer51 { this.pushScope(ScopeType.Loop); const baseResult: tstl.Statement[] = [tstl.createDoStatement(super.transformLoopBody(loop))]; - const scopeId = this.popScope().id; + const scopeId = this.popScope(baseResult).id; const continueLabel = tstl.createLabelStatement(`__continue${scopeId}`); baseResult.push(continueLabel); From 968b12adaefc431ca3d18d2d3d0cfe13ebb25310 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Fri, 25 Jan 2019 08:36:42 -0700 Subject: [PATCH 02/24] fixed issues with switch and loop continue --- src/LuaTransformer.ts | 18 +++++++++++++++--- src/targets/LuaTransformer.52.ts | 4 ++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 13273ba35..d015fa252 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1470,7 +1470,8 @@ export class LuaTransformer { } public transformBreakStatement(breakStatement: ts.BreakStatement): StatementVisitResult { - if (this.peekScope().type === ScopeType.Switch) { + const breakableScopes = [ScopeType.Loop, ScopeType.Switch]; + if (this.findScope(...breakableScopes).type === ScopeType.Switch) { return tstl.createGotoStatement(`____TS_switch${this.scopeStack.length}_end`); } else { return tstl.createBreakStatement(undefined, breakStatement); @@ -3154,14 +3155,16 @@ export class LuaTransformer { { if (this.shouldExportIdentifier(lhs)) { // exported - if (Array.isArray(lhs)) { + if (!rhs) { + return []; + } else if (Array.isArray(lhs)) { return [tstl.createAssignmentStatement(lhs.map(i => this.createExportedIdentifier(i)), rhs, parent)]; } else { return [tstl.createAssignmentStatement(this.createExportedIdentifier(lhs), rhs, parent)]; } } - const insideFunction = this.scopeStack.some(s => s.type === ScopeType.Function); + const insideFunction = this.findScope(ScopeType.Function) !== undefined; const isLetOrConst = tsOriginal && ts.isVariableDeclaration(tsOriginal) && (tsOriginal.parent.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) !== 0; if (this.isModule || this.currentNamespace || insideFunction || isLetOrConst) { @@ -3267,6 +3270,15 @@ export class LuaTransformer { return tstl.createBinaryExpression(expression, tstl.createNumericLiteral(1), tstl.SyntaxKind.AdditionOperator); } + protected findScope(...scopeTypes: ScopeType[]): Scope | undefined { + for (let i = this.scopeStack.length - 1; i >= 0; --i) { + if (scopeTypes.indexOf(this.scopeStack[i].type) >= 0) { + return this.scopeStack[i]; + } + } + return undefined; + } + protected peekScope(): Scope { return this.scopeStack[this.scopeStack.length - 1]; } diff --git a/src/targets/LuaTransformer.52.ts b/src/targets/LuaTransformer.52.ts index 9868d1992..89eef65e5 100644 --- a/src/targets/LuaTransformer.52.ts +++ b/src/targets/LuaTransformer.52.ts @@ -13,7 +13,7 @@ export class LuaTransformer52 extends LuaTransformer51 ): tstl.Statement[] { this.pushScope(ScopeType.Loop); - const baseResult: tstl.Statement[] = [tstl.createDoStatement(super.transformLoopBody(loop))]; + const baseResult: tstl.Statement[] = [tstl.createDoStatement(this.transformBlockOrStatement(loop.statement))]; const scopeId = this.popScope(baseResult).id; const continueLabel = tstl.createLabelStatement(`__continue${scopeId}`); @@ -25,7 +25,7 @@ export class LuaTransformer52 extends LuaTransformer51 /** @override */ public transformContinueStatement(statement: ts.ContinueStatement): StatementVisitResult { return tstl.createGotoStatement( - `__continue${this.peekScope().id}`, + `__continue${this.findScope(ScopeType.Loop).id}`, undefined, statement ); From 86c7bd45ffd462e05b883b7a4231486621baf49c Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sat, 26 Jan 2019 09:55:12 -0700 Subject: [PATCH 03/24] dealing with hoisting of imports and exports --- src/LuaTransformer.ts | 92 ++++++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 45 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index d015fa252..90489c081 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -24,7 +24,7 @@ interface Scope { type: ScopeType; id: number; locals: tstl.Identifier[]; - functions: tstl.AssignmentStatement[]; + functions: tstl.Statement[]; } export class LuaTransformer { @@ -84,6 +84,8 @@ export class LuaTransformer { this.isModule = tsHelper.isFileModule(node); const statements = this.transformStatements(node.statements); + this.popScope(statements); + if (this.isModule) { statements.unshift( tstl.createVariableDeclarationStatement( @@ -98,7 +100,6 @@ export class LuaTransformer { [tstl.createIdentifier("exports")] )); } - this.popScope(statements); return [tstl.createBlock(statements, undefined, node), this.luaLibFeatureSet]; } @@ -218,19 +219,20 @@ export class LuaTransformer { result.push(requireStatement); filteredElements.forEach(importSpecifier => { + const nameIdentifier = this.transformIdentifier(importSpecifier.name); + this.addScopeLocals(nameIdentifier); if (importSpecifier.propertyName) { const propertyIdentifier = this.transformIdentifier(importSpecifier.propertyName); const propertyName = tstl.createStringLiteral(propertyIdentifier.text); - const renamedImport = tstl.createVariableDeclarationStatement( - this.transformIdentifier(importSpecifier.name), + const renamedImport = tstl.createAssignmentStatement( + nameIdentifier, tstl.createTableIndexExpression(importUniqueName, propertyName), undefined, importSpecifier); result.push(renamedImport); } else { - const nameIdentifier = this.transformIdentifier(importSpecifier.name); const name = tstl.createStringLiteral(nameIdentifier.text); - const namedImport = tstl.createVariableDeclarationStatement( + const namedImport = tstl.createAssignmentStatement( nameIdentifier, tstl.createTableIndexExpression(importUniqueName, name), undefined, @@ -241,8 +243,10 @@ export class LuaTransformer { }); return result; } else if (ts.isNamespaceImport(imports)) { - const requireStatement = tstl.createVariableDeclarationStatement( - this.transformIdentifier(imports.name), + const nameIdentifier = this.transformIdentifier(imports.name); + this.addScopeLocals(nameIdentifier); + const requireStatement = tstl.createAssignmentStatement( + nameIdentifier, requireCall, undefined, statement @@ -809,6 +813,7 @@ export class LuaTransformer { tstl.createStringLiteral(this.transformIdentifier(statement.name as ts.Identifier).text))); result.push(localDeclaration); + } else if (this.isModule && (ts.getCombinedModifierFlags(statement) & ts.ModifierFlags.Export)) { // exports.NS = exports.NS or {} const namespaceDeclaration = tstl.createAssignmentStatement( @@ -826,9 +831,9 @@ export class LuaTransformer { this.createExportedIdentifier(this.transformIdentifier(statement.name as ts.Identifier))); result.push(localDeclaration); + } else { // local NS = NS or {} - // TODO this is somewhat redundant since createLocalOrGlobalDeclaration also handles exports const localDeclaration = this.createLocalOrExportedOrGlobalDeclaration( this.transformIdentifier(statement.name as ts.Identifier), tstl.createBinaryExpression( @@ -848,7 +853,10 @@ export class LuaTransformer { // Transform moduleblock to block and visit it if (statement.body && ts.isModuleBlock(statement.body)) { - result.push(tstl.createDoStatement(this.transformStatements(statement.body.statements))); + this.pushScope(ScopeType.Block); + const statements = this.transformStatements(statement.body.statements); + this.popScope(statements); + result.push(tstl.createDoStatement(statements)); } this.currentNamespace = previousNamespace; @@ -1422,7 +1430,7 @@ export class LuaTransformer { this.pushScope(ScopeType.Switch); // Give the switch a unique name to prevent nested switches from acting up. - const switchName = `____TS_switch${this.scopeStack.length}`; + const switchName = `____TS_switch${this.peekScope().id}`; const expression = this.transformExpression(statement.expression); const switchVariable = tstl.createIdentifier(switchName); @@ -1470,9 +1478,9 @@ export class LuaTransformer { } public transformBreakStatement(breakStatement: ts.BreakStatement): StatementVisitResult { - const breakableScopes = [ScopeType.Loop, ScopeType.Switch]; - if (this.findScope(...breakableScopes).type === ScopeType.Switch) { - return tstl.createGotoStatement(`____TS_switch${this.scopeStack.length}_end`); + const breakableScope = this.findScope(ScopeType.Loop, ScopeType.Switch); + if (breakableScope.type === ScopeType.Switch) { + return tstl.createGotoStatement(`____TS_switch${breakableScope.id}_end`); } else { return tstl.createBreakStatement(undefined, breakStatement); } @@ -3153,32 +3161,36 @@ export class LuaTransformer { tsOriginal?: ts.Node ): tstl.Statement[] { + const isFunctionDeclaration = tsOriginal && ts.isFunctionDeclaration(tsOriginal); + + let left: tstl.IdentifierOrTableIndexExpression | tstl.IdentifierOrTableIndexExpression[] = lhs; if (this.shouldExportIdentifier(lhs)) { // exported - if (!rhs) { - return []; - } else if (Array.isArray(lhs)) { - return [tstl.createAssignmentStatement(lhs.map(i => this.createExportedIdentifier(i)), rhs, parent)]; + if (Array.isArray(lhs)) { + left = lhs.map(i => this.createExportedIdentifier(i)); } else { - return [tstl.createAssignmentStatement(this.createExportedIdentifier(lhs), rhs, parent)]; + left = this.createExportedIdentifier(lhs); } - } - const insideFunction = this.findScope(ScopeType.Function) !== undefined; - const isLetOrConst = tsOriginal && ts.isVariableDeclaration(tsOriginal) - && (tsOriginal.parent.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) !== 0; - if (this.isModule || this.currentNamespace || insideFunction || isLetOrConst) { - // local - if (ts.isFunctionDeclaration(tsOriginal)) { - // hoist function declarations - this.addScopeFunction(lhs as tstl.Identifier, rhs as tstl.FunctionExpression, parent, tsOriginal); - return undefined; - } else { - this.addScopeLocals(lhs, isLetOrConst ? undefined : ScopeType.Function); + } else { + const insideFunction = this.findScope(ScopeType.Function) !== undefined; + const isLetOrConst = tsOriginal && ts.isVariableDeclaration(tsOriginal) + && (tsOriginal.parent.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) !== 0; + if (this.isModule || this.currentNamespace || insideFunction || isLetOrConst) { + // hoist locals + const scopeType = !(isLetOrConst || isFunctionDeclaration) ? ScopeType.Function : undefined; + this.addScopeLocals(lhs, scopeType); } } + + if (isFunctionDeclaration) { + // hoist function declarations + this.addScopeFunction(tstl.createAssignmentStatement(left, rhs, parent, tsOriginal)); + return undefined; + } + if (rhs) { - return [tstl.createAssignmentStatement(lhs, rhs, parent, tsOriginal)]; + return [tstl.createAssignmentStatement(left, rhs, parent, tsOriginal)]; } else { return []; } @@ -3290,14 +3302,10 @@ export class LuaTransformer { protected popScope(statements: tstl.Statement[]): Scope { const scope = this.scopeStack.pop(); - statements.unshift(...scope.functions); - if (scope.locals.length > 0) { - const declaration = tstl.createVariableDeclarationStatement(scope.locals); - statements.unshift(declaration); + statements.unshift(tstl.createVariableDeclarationStatement(scope.locals)); } - return scope; } @@ -3321,15 +3329,9 @@ export class LuaTransformer { } } - protected addScopeFunction( - name: tstl.Identifier, - func: tstl.FunctionExpression, - parent?: tstl.Node, - tsOriginal?: ts.Node - ): void { + protected addScopeFunction(func: tstl.AssignmentStatement): void { const scope = this.peekScope(); - scope.locals.push(name); - scope.functions.push(tstl.createAssignmentStatement(name, func, parent, tsOriginal)); + scope.functions.push(func); } private statementVisitResultToStatementArray(visitResult: StatementVisitResult): tstl.Statement[] { From e2374c51cec01e8d8e449b6210cad837fe0794d4 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sun, 27 Jan 2019 07:48:32 -0700 Subject: [PATCH 04/24] hoisting tests --- test/src/util.ts | 18 +++++ test/unit/hoisting.spec.ts | 133 +++++++++++++++++++++++++++++++++++++ test/unit/loops.spec.ts | 8 +-- 3 files changed, 155 insertions(+), 4 deletions(-) create mode 100644 test/unit/hoisting.spec.ts diff --git a/test/src/util.ts b/test/src/util.ts index dca965ade..e5afa7936 100644 --- a/test/src/util.ts +++ b/test/src/util.ts @@ -97,6 +97,24 @@ export function transpileAndExecute( return executeLua(lua); } +export function transpileAndExecuteWithExport( + tsStr: string, + returnExport: string, + compilerOptions?: CompilerOptions, + luaHeader?: string +): any +{ + const wrappedTsString = `declare function JSONStringify(p: any): string; + ${tsStr}`; + + const lua = `return (function() + ${luaHeader ? luaHeader : ""} + ${transpileString(wrappedTsString, compilerOptions, false)} + end)().${returnExport}`; + + return executeLua(lua); +} + export function parseTypeScript(typescript: string, target: LuaTarget = LuaTarget.Lua53) : [ts.SourceFile, ts.TypeChecker] { const compilerHost = { diff --git a/test/unit/hoisting.spec.ts b/test/unit/hoisting.spec.ts new file mode 100644 index 000000000..afe72df8c --- /dev/null +++ b/test/unit/hoisting.spec.ts @@ -0,0 +1,133 @@ +import { Expect, Test, TestCase } from "alsatian"; + +import * as util from "../src/util"; + +export class HoistingTests { + + @Test("Var Hoisting") + public varHoisting(): void + { + const code = + `foo = "foo"; + var foo; + return foo;`; + const result = util.transpileAndExecute(code); + Expect(result).toBe("foo"); + } + + @Test("Exported Var Hoisting") + public exportedVarHoisting(): void + { + const code = + `foo = "foo"; + export var foo;`; + const result = util.transpileAndExecuteWithExport(code, "foo"); + Expect(result).toBe("foo"); + } + + @TestCase("let") + @TestCase("const") + @Test("Let/Const Hoisting") + public letConstHoisting(varType: string): void + { + const code = + `let bar: string; + function setBar() { bar = foo; } + ${varType} foo = "foo"; + setBar(); + return foo;`; + const result = util.transpileAndExecute(code); + Expect(result).toBe("foo"); + } + + @TestCase("let") + @TestCase("const") + @Test("Exported Let/Const Hoisting") + public exportedLetConstHoisting(varType: string): void + { + const code = + `let bar: string; + function setBar() { bar = foo; } + export ${varType} foo = "foo"; + setBar();`; + const result = util.transpileAndExecuteWithExport(code, "foo"); + Expect(result).toBe("foo"); + } + + @Test("Global Function Hoisting") + public globalFunctionHoisting(): void + { + const code = + `const foo = bar(); + function bar() { return "bar"; } + return foo;`; + const result = util.transpileAndExecute(code); + Expect(result).toBe("bar"); + } + + @Test("Local Function Hoisting") + public localFunctionHoisting(): void + { + const code = + `export const foo = bar(); + function bar() { return "bar"; }`; + const result = util.transpileAndExecuteWithExport(code, "foo"); + Expect(result).toBe("bar"); + } + + @Test("Exported Function Hoisting") + public exportedFunctionHoisting(): void + { + const code = + `const foo = bar(); + export function bar() { return "bar"; } + export const baz = foo;`; + const result = util.transpileAndExecuteWithExport(code, "baz"); + Expect(result).toBe("bar"); + } + + @Test("Namespace Function Hoisting") + public namespaceFunctionHoisting(): void + { + const code = + `let foo: string; + namespace NS { + foo = bar(); + function bar() { return "bar"; } + }`; + const result = util.transpileAndExecute("return foo;", undefined, undefined, code); + Expect(result).toBe("bar"); + } + + @Test("Exported Namespace Function Hoisting") + public exportedNamespaceFunctionHoisting(): void + { + const code = + `let foo: string; + namespace NS { + foo = bar(); + export function bar() { return "bar"; } + }`; + const result = util.transpileAndExecute("return foo;", undefined, undefined, code); + Expect(result).toBe("bar"); + } + + @TestCase("var", "foo") + @TestCase("let", "bar") + @TestCase("const", "bar") + @Test("Hoisting in Non-Function Scope") + public hoistingInNonFunctionScope(varType: string, expectResult: string): void + { + const code = + `function foo() { + ${varType} bar = "bar"; + for (let i = 0; i < 1; ++i) { + ${varType} bar = "foo"; + } + return bar; + } + return foo();`; + const result = util.transpileAndExecute(code); + Expect(result).toBe(expectResult); + } +} diff --git a/test/unit/loops.spec.ts b/test/unit/loops.spec.ts index 48c9d3fcf..63ace357f 100644 --- a/test/unit/loops.spec.ts +++ b/test/unit/loops.spec.ts @@ -678,10 +678,10 @@ export class LuaLoopTests const luajit = util.transpileString(loop, { luaTarget: LuaTarget.LuaJIT }); // Assert - Expect(lua51.indexOf("::__continue0::") !== -1).toBe(false); // No labels in 5.1 - Expect(lua52.indexOf("::__continue0::") !== -1).toBe(true); // Labels from 5.2 onwards - Expect(lua53.indexOf("::__continue0::") !== -1).toBe(true); - Expect(luajit.indexOf("::__continue0::") !== -1).toBe(true); + Expect(lua51.indexOf("::__continue1::") !== -1).toBe(false); // No labels in 5.1 + Expect(lua52.indexOf("::__continue1::") !== -1).toBe(true); // Labels from 5.2 onwards + Expect(lua53.indexOf("::__continue1::") !== -1).toBe(true); + Expect(luajit.indexOf("::__continue1::") !== -1).toBe(true); } @Test("for dead code after return") From 7833a65449459618817098090952012d6cdfe37e Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sun, 27 Jan 2019 15:19:28 -0700 Subject: [PATCH 05/24] fixed namespace hoisting and added more tests --- src/LuaTransformer.ts | 6 ++- test/unit/hoisting.spec.ts | 102 +++++++++++++++++++++++++++++-------- 2 files changed, 86 insertions(+), 22 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 3fa2eec0a..0e7aa72f0 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -809,7 +809,8 @@ export class LuaTransformer { result.push(namespaceDeclaration); // local innerNS = outerNS.innerNS - const localDeclaration = tstl.createVariableDeclarationStatement( + this.addScopeLocals(this.transformIdentifier(statement.name as ts.Identifier)); + const localDeclaration = tstl.createAssignmentStatement( this.transformIdentifier(statement.name as ts.Identifier), tstl.createTableIndexExpression( this.transformIdentifier(this.currentNamespace.name as ts.Identifier), @@ -829,7 +830,8 @@ export class LuaTransformer { result.push(namespaceDeclaration); // local NS = exports.NS - const localDeclaration = tstl.createVariableDeclarationStatement( + this.addScopeLocals(this.transformIdentifier(statement.name as ts.Identifier)); + const localDeclaration = tstl.createAssignmentStatement( this.transformIdentifier(statement.name as ts.Identifier), this.createExportedIdentifier(this.transformIdentifier(statement.name as ts.Identifier))); diff --git a/test/unit/hoisting.spec.ts b/test/unit/hoisting.spec.ts index afe72df8c..07bbf495d 100644 --- a/test/unit/hoisting.spec.ts +++ b/test/unit/hoisting.spec.ts @@ -5,8 +5,7 @@ import * as util from "../src/util"; export class HoistingTests { @Test("Var Hoisting") - public varHoisting(): void - { + public varHoisting(): void { const code = `foo = "foo"; var foo; @@ -16,8 +15,7 @@ export class HoistingTests { } @Test("Exported Var Hoisting") - public exportedVarHoisting(): void - { + public exportedVarHoisting(): void { const code = `foo = "foo"; export var foo;`; @@ -28,8 +26,7 @@ export class HoistingTests { @TestCase("let") @TestCase("const") @Test("Let/Const Hoisting") - public letConstHoisting(varType: string): void - { + public letConstHoisting(varType: string): void { const code = `let bar: string; function setBar() { bar = foo; } @@ -43,8 +40,7 @@ export class HoistingTests { @TestCase("let") @TestCase("const") @Test("Exported Let/Const Hoisting") - public exportedLetConstHoisting(varType: string): void - { + public exportedLetConstHoisting(varType: string): void { const code = `let bar: string; function setBar() { bar = foo; } @@ -55,8 +51,7 @@ export class HoistingTests { } @Test("Global Function Hoisting") - public globalFunctionHoisting(): void - { + public globalFunctionHoisting(): void { const code = `const foo = bar(); function bar() { return "bar"; } @@ -66,8 +61,7 @@ export class HoistingTests { } @Test("Local Function Hoisting") - public localFunctionHoisting(): void - { + public localFunctionHoisting(): void { const code = `export const foo = bar(); function bar() { return "bar"; }`; @@ -76,8 +70,7 @@ export class HoistingTests { } @Test("Exported Function Hoisting") - public exportedFunctionHoisting(): void - { + public exportedFunctionHoisting(): void { const code = `const foo = bar(); export function bar() { return "bar"; } @@ -87,8 +80,7 @@ export class HoistingTests { } @Test("Namespace Function Hoisting") - public namespaceFunctionHoisting(): void - { + public namespaceFunctionHoisting(): void { const code = `let foo: string; namespace NS { @@ -100,8 +92,7 @@ export class HoistingTests { } @Test("Exported Namespace Function Hoisting") - public exportedNamespaceFunctionHoisting(): void - { + public exportedNamespaceFunctionHoisting(): void { const code = `let foo: string; namespace NS { @@ -116,8 +107,7 @@ export class HoistingTests { @TestCase("let", "bar") @TestCase("const", "bar") @Test("Hoisting in Non-Function Scope") - public hoistingInNonFunctionScope(varType: string, expectResult: string): void - { + public hoistingInNonFunctionScope(varType: string, expectResult: string): void { const code = `function foo() { ${varType} bar = "bar"; @@ -130,4 +120,76 @@ export class HoistingTests { const result = util.transpileAndExecute(code); Expect(result).toBe(expectResult); } + + @Test("Namespace Hoisting") + public namespaceHoisting(): void { + const code = + `function bar() { + return NS.foo; + } + namespace NS { + export let foo = "foo"; + } + export const foo = bar();`; + const result = util.transpileAndExecuteWithExport(code, "foo"); + Expect(result).toBe("foo"); + } + + @Test("Exported Namespace Hoisting") + public exportedNamespaceHoisting(): void { + const code = + `function bar() { + return NS.foo; + } + export namespace NS { + export let foo = "foo"; + } + export const foo = bar();`; + const result = util.transpileAndExecuteWithExport(code, "foo"); + Expect(result).toBe("foo"); + } + + @Test("Nested Namespace Hoisting") + public nestedNamespaceHoisting(): void { + const code = + `export namespace Outer { + export function bar() { + return Inner.foo; + } + namespace Inner { + export let foo = "foo"; + } + } + export const foo = Outer.bar();`; + const result = util.transpileAndExecuteWithExport(code, "foo"); + Expect(result).toBe("foo"); + } + + @Test("Class Hoisting") + public classHoisting(): void { + const code = + `function makeFoo() { + return new Foo(); + } + class Foo { + public bar = "foo"; + } + export const foo = makeFoo().bar;`; + const result = util.transpileAndExecuteWithExport(code, "foo"); + Expect(result).toBe("foo"); + } + + @Test("Enum Hoisting") + public enumHoisting(): void { + const code = + `function bar() { + return E.A; + } + enum E { + A = "foo" + } + export const foo = bar();`; + const result = util.transpileAndExecuteWithExport(code, "foo"); + Expect(result).toBe("foo"); + } } From 23cde6a5fd2f267a82fa931048e7011669e4d9a9 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sun, 27 Jan 2019 17:18:27 -0700 Subject: [PATCH 06/24] rebuilt translation tests with hoisting --- .../lua/characterEscapeSequence.lua | 19 +++--- test/translation/lua/continue.lua | 7 ++- test/translation/lua/continueConcurrent.lua | 9 +-- test/translation/lua/continueNested.lua | 14 +++-- .../lua/continueNestedConcurrent.lua | 16 ++--- test/translation/lua/do.lua | 5 +- test/translation/lua/enumMembersOnly.lua | 3 +- test/translation/lua/for.lua | 5 +- test/translation/lua/forIn.lua | 2 +- test/translation/lua/forOf.lua | 2 +- test/translation/lua/getSetAccessors.lua | 7 ++- test/translation/lua/interfaceIndex.lua | 3 +- .../lua/modulesChangedVariableExport.lua | 3 +- .../lua/modulesNamespaceExport.lua | 3 +- .../lua/modulesNamespaceExportEnum.lua | 3 +- ...modulesNamespaceNestedWithMemberExport.lua | 6 +- .../lua/modulesNamespaceWithMemberExport.lua | 3 +- .../modulesNamespaceWithMemberNoExport.lua | 3 +- .../lua/modulesVariableNoExport.lua | 3 +- test/translation/lua/namespaceMerge.lua | 3 +- test/translation/lua/namespaceNested.lua | 3 +- test/translation/lua/tryCatch.lua | 5 +- test/translation/lua/tryCatchFinally.lua | 7 ++- test/translation/lua/tryFinally.lua | 5 +- test/translation/lua/tupleReturn.lua | 58 ++++++++++--------- test/translation/lua/typeAssert.lua | 5 +- test/translation/lua/while.lua | 5 +- 27 files changed, 117 insertions(+), 90 deletions(-) diff --git a/test/translation/lua/characterEscapeSequence.lua b/test/translation/lua/characterEscapeSequence.lua index 36abfe9d0..1179699bc 100644 --- a/test/translation/lua/characterEscapeSequence.lua +++ b/test/translation/lua/characterEscapeSequence.lua @@ -1,9 +1,10 @@ -local quoteInDoubleQuotes = "\' \' \'"; -local quoteInTemplateString = "\' \' \'"; -local doubleQuoteInQuotes = "\" \" \""; -local doubleQuoteInDoubleQuotes = "\" \" \""; -local doubleQuoteInTemplateString = "\" \" \""; -local escapedCharsInQuotes = "\\ \0 \b \t \n \v \f \" \' \`"; -local escapedCharsInDoubleQUotes = "\\ \0 \b \t \n \v \f \" \' \`"; -local escapedCharsInTemplateString = "\\ \0 \b \t \n \v \f \" \' \`"; -local nonEmptyTemplateString = "Level 0: \n\t " .. (tostring("Level 1: \n\t\t " .. (tostring("Level 3: \n\t\t\t " .. (tostring("Last level \n --") .. " \n --")) .. " \n --")) .. " \n --"); +local quoteInDoubleQuotes, quoteInTemplateString, doubleQuoteInQuotes, doubleQuoteInDoubleQuotes, doubleQuoteInTemplateString, escapedCharsInQuotes, escapedCharsInDoubleQUotes, escapedCharsInTemplateString, nonEmptyTemplateString; +quoteInDoubleQuotes = "\' \' \'"; +quoteInTemplateString = "\' \' \'"; +doubleQuoteInQuotes = "\" \" \""; +doubleQuoteInDoubleQuotes = "\" \" \""; +doubleQuoteInTemplateString = "\" \" \""; +escapedCharsInQuotes = "\\ \0 \b \t \n \v \f \" \' \`"; +escapedCharsInDoubleQUotes = "\\ \0 \b \t \n \v \f \" \' \`"; +escapedCharsInTemplateString = "\\ \0 \b \t \n \v \f \" \' \`"; +nonEmptyTemplateString = "Level 0: \n\t " .. (tostring("Level 1: \n\t\t " .. (tostring("Level 3: \n\t\t\t " .. (tostring("Last level \n --") .. " \n --")) .. " \n --")) .. " \n --"); diff --git a/test/translation/lua/continue.lua b/test/translation/lua/continue.lua index ffb13df0c..a4a615636 100644 --- a/test/translation/lua/continue.lua +++ b/test/translation/lua/continue.lua @@ -1,10 +1,11 @@ -local i = 0; +local i; +i = 0; while i < 10 do do if i < 5 then - goto __continue0; + goto __continue1; end end - ::__continue0:: + ::__continue1:: i = i + 1; end diff --git a/test/translation/lua/continueConcurrent.lua b/test/translation/lua/continueConcurrent.lua index 60d60047e..0deb036e0 100644 --- a/test/translation/lua/continueConcurrent.lua +++ b/test/translation/lua/continueConcurrent.lua @@ -1,13 +1,14 @@ -local i = 0; +local i; +i = 0; while i < 10 do do if i < 5 then - goto __continue0; + goto __continue1; end if i == 7 then - goto __continue0; + goto __continue1; end end - ::__continue0:: + ::__continue1:: i = i + 1; end diff --git a/test/translation/lua/continueNested.lua b/test/translation/lua/continueNested.lua index 88be74803..137211d85 100644 --- a/test/translation/lua/continueNested.lua +++ b/test/translation/lua/continueNested.lua @@ -1,20 +1,22 @@ -local i = 0; +local i; +i = 0; while i < 5 do do + local j; if (i % 2) == 0 then - goto __continue0; + goto __continue1; end - local j = 0; + j = 0; while j < 2 do do if j == 1 then - goto __continue1; + goto __continue3; end end - ::__continue1:: + ::__continue3:: j = j + 1; end end - ::__continue0:: + ::__continue1:: i = i + 1; end diff --git a/test/translation/lua/continueNestedConcurrent.lua b/test/translation/lua/continueNestedConcurrent.lua index 1e1ed1ca1..7083b5a6e 100644 --- a/test/translation/lua/continueNestedConcurrent.lua +++ b/test/translation/lua/continueNestedConcurrent.lua @@ -1,23 +1,25 @@ -local i = 0; +local i; +i = 0; while i < 5 do do + local j; if (i % 2) == 0 then - goto __continue0; + goto __continue1; end - local j = 0; + j = 0; while j < 2 do do if j == 1 then - goto __continue1; + goto __continue3; end end - ::__continue1:: + ::__continue3:: j = j + 1; end if i == 4 then - goto __continue0; + goto __continue1; end end - ::__continue0:: + ::__continue1:: i = i + 1; end diff --git a/test/translation/lua/do.lua b/test/translation/lua/do.lua index dcedbadcc..6c2be3b3b 100644 --- a/test/translation/lua/do.lua +++ b/test/translation/lua/do.lua @@ -1,7 +1,8 @@ -local e = 10; +local e; +e = 10; repeat do e = e - 1; end - ::__continue0:: + ::__continue1:: until not (e > 0); diff --git a/test/translation/lua/enumMembersOnly.lua b/test/translation/lua/enumMembersOnly.lua index 06bdb5f70..126867ab9 100644 --- a/test/translation/lua/enumMembersOnly.lua +++ b/test/translation/lua/enumMembersOnly.lua @@ -1,5 +1,6 @@ +local a; val1 = 0; val2 = 2; val3 = 3; val4 = "bye"; -local a = val1; +a = val1; diff --git a/test/translation/lua/for.lua b/test/translation/lua/for.lua index 23293c334..e5b8dcee8 100644 --- a/test/translation/lua/for.lua +++ b/test/translation/lua/for.lua @@ -1,7 +1,8 @@ -local i = 1; +local i; +i = 1; while i <= 100 do do end - ::__continue0:: + ::__continue1:: i = i + 1; end diff --git a/test/translation/lua/forIn.lua b/test/translation/lua/forIn.lua index 685c401c4..5882e705d 100644 --- a/test/translation/lua/forIn.lua +++ b/test/translation/lua/forIn.lua @@ -1,5 +1,5 @@ for i in pairs({a = 1, b = 2, c = 3, d = 4}) do do end - ::__continue0:: + ::__continue1:: end diff --git a/test/translation/lua/forOf.lua b/test/translation/lua/forOf.lua index 308f64684..ce8676017 100644 --- a/test/translation/lua/forOf.lua +++ b/test/translation/lua/forOf.lua @@ -3,5 +3,5 @@ for ____TS_index = 1, #____TS_array do local i = ____TS_array[____TS_index]; do end - ::__continue0:: + ::__continue1:: end diff --git a/test/translation/lua/getSetAccessors.lua b/test/translation/lua/getSetAccessors.lua index 9fe62bbb6..a7b08df80 100644 --- a/test/translation/lua/getSetAccessors.lua +++ b/test/translation/lua/getSetAccessors.lua @@ -1,3 +1,4 @@ +local instance, b, c; MyClass = MyClass or {}; MyClass.__index = MyClass; MyClass.new = function(construct, ...) @@ -15,7 +16,7 @@ end; MyClass.set__field = function(self, v) self._field = v * 2; end; -local instance = MyClass.new(true); +instance = MyClass.new(true); instance:set__field(4); -local b = instance:get__field(); -local c = (4 + instance:get__field()) * 3; +b = instance:get__field(); +c = (4 + instance:get__field()) * 3; diff --git a/test/translation/lua/interfaceIndex.lua b/test/translation/lua/interfaceIndex.lua index a29d6d09d..fd4bf2dd5 100644 --- a/test/translation/lua/interfaceIndex.lua +++ b/test/translation/lua/interfaceIndex.lua @@ -1,2 +1,3 @@ -local a = {}; +local a; +a = {}; a.abc = "def"; diff --git a/test/translation/lua/modulesChangedVariableExport.lua b/test/translation/lua/modulesChangedVariableExport.lua index 2776db6f6..aa02323b6 100644 --- a/test/translation/lua/modulesChangedVariableExport.lua +++ b/test/translation/lua/modulesChangedVariableExport.lua @@ -1,4 +1,3 @@ local exports = exports or {}; -exports.test = nil; exports.test = 1; -return exports; \ No newline at end of file +return exports; diff --git a/test/translation/lua/modulesNamespaceExport.lua b/test/translation/lua/modulesNamespaceExport.lua index 122f64cf0..2fcdd8d53 100644 --- a/test/translation/lua/modulesNamespaceExport.lua +++ b/test/translation/lua/modulesNamespaceExport.lua @@ -1,6 +1,7 @@ local exports = exports or {}; +local TestSpace; exports.TestSpace = exports.TestSpace or {}; -local TestSpace = exports.TestSpace; +TestSpace = exports.TestSpace; do end return exports; diff --git a/test/translation/lua/modulesNamespaceExportEnum.lua b/test/translation/lua/modulesNamespaceExportEnum.lua index 938fdd56b..a0056e377 100644 --- a/test/translation/lua/modulesNamespaceExportEnum.lua +++ b/test/translation/lua/modulesNamespaceExportEnum.lua @@ -1,6 +1,7 @@ local exports = exports or {}; +local test; exports.test = exports.test or {}; -local test = exports.test; +test = exports.test; do test.TestEnum = {}; test.TestEnum.foo = "foo"; diff --git a/test/translation/lua/modulesNamespaceNestedWithMemberExport.lua b/test/translation/lua/modulesNamespaceNestedWithMemberExport.lua index 44509f50d..902d719bd 100644 --- a/test/translation/lua/modulesNamespaceNestedWithMemberExport.lua +++ b/test/translation/lua/modulesNamespaceNestedWithMemberExport.lua @@ -1,9 +1,11 @@ local exports = exports or {}; +local TestSpace; exports.TestSpace = exports.TestSpace or {}; -local TestSpace = exports.TestSpace; +TestSpace = exports.TestSpace; do + local TestNestedSpace; TestSpace.TestNestedSpace = TestSpace.TestNestedSpace or {}; - local TestNestedSpace = TestSpace.TestNestedSpace; + TestNestedSpace = TestSpace.TestNestedSpace; do TestNestedSpace.innerFunc = function() end; diff --git a/test/translation/lua/modulesNamespaceWithMemberExport.lua b/test/translation/lua/modulesNamespaceWithMemberExport.lua index 020bcf4d5..d6192ed57 100644 --- a/test/translation/lua/modulesNamespaceWithMemberExport.lua +++ b/test/translation/lua/modulesNamespaceWithMemberExport.lua @@ -1,6 +1,7 @@ local exports = exports or {}; +local TestSpace; exports.TestSpace = exports.TestSpace or {}; -local TestSpace = exports.TestSpace; +TestSpace = exports.TestSpace; do TestSpace.innerFunc = function() end; diff --git a/test/translation/lua/modulesNamespaceWithMemberNoExport.lua b/test/translation/lua/modulesNamespaceWithMemberNoExport.lua index b0abeb433..1ee06a28b 100644 --- a/test/translation/lua/modulesNamespaceWithMemberNoExport.lua +++ b/test/translation/lua/modulesNamespaceWithMemberNoExport.lua @@ -1,6 +1,7 @@ local exports = exports or {}; +local TestSpace; exports.TestSpace = exports.TestSpace or {}; -local TestSpace = exports.TestSpace; +TestSpace = exports.TestSpace; do local innerFunc; innerFunc = function() diff --git a/test/translation/lua/modulesVariableNoExport.lua b/test/translation/lua/modulesVariableNoExport.lua index f20c4efeb..537c5219e 100644 --- a/test/translation/lua/modulesVariableNoExport.lua +++ b/test/translation/lua/modulesVariableNoExport.lua @@ -1 +1,2 @@ -local test = "test"; +local test; +test = "test"; diff --git a/test/translation/lua/namespaceMerge.lua b/test/translation/lua/namespaceMerge.lua index e585c3c83..0bdf40c4f 100644 --- a/test/translation/lua/namespaceMerge.lua +++ b/test/translation/lua/namespaceMerge.lua @@ -1,3 +1,4 @@ +local mergedClass; MergedClass = MergedClass or {}; MergedClass.__index = MergedClass; MergedClass.new = function(construct, ...) @@ -27,7 +28,7 @@ do MergedClass.namespaceFunc = function() end; end -local mergedClass = MergedClass.new(true); +mergedClass = MergedClass.new(true); mergedClass:methodB(); mergedClass:propertyFunc(); MergedClass:staticMethodB(); diff --git a/test/translation/lua/namespaceNested.lua b/test/translation/lua/namespaceNested.lua index dd1006ee9..f61e0373f 100644 --- a/test/translation/lua/namespaceNested.lua +++ b/test/translation/lua/namespaceNested.lua @@ -1,7 +1,8 @@ myNamespace = myNamespace or {}; do + local myNestedNamespace; myNamespace.myNestedNamespace = myNamespace.myNestedNamespace or {}; - local myNestedNamespace = myNamespace.myNestedNamespace; + myNestedNamespace = myNamespace.myNestedNamespace; do local nsMember; nsMember = function() diff --git a/test/translation/lua/tryCatch.lua b/test/translation/lua/tryCatch.lua index f3194765e..62b256af5 100644 --- a/test/translation/lua/tryCatch.lua +++ b/test/translation/lua/tryCatch.lua @@ -1,8 +1,9 @@ +local a, b; do local ____TS_try, er = pcall(function() - local a = 42; + a = 42; end); if not ____TS_try then - local b = "fail"; + b = "fail"; end end diff --git a/test/translation/lua/tryCatchFinally.lua b/test/translation/lua/tryCatchFinally.lua index 4b5caf9a6..53059e25a 100644 --- a/test/translation/lua/tryCatchFinally.lua +++ b/test/translation/lua/tryCatchFinally.lua @@ -1,11 +1,12 @@ +local a, b, c; do local ____TS_try, er = pcall(function() - local a = 42; + a = 42; end); if not ____TS_try then - local b = "fail"; + b = "fail"; end do - local c = "finally"; + c = "finally"; end end diff --git a/test/translation/lua/tryFinally.lua b/test/translation/lua/tryFinally.lua index a117b8bb2..a234b2db7 100644 --- a/test/translation/lua/tryFinally.lua +++ b/test/translation/lua/tryFinally.lua @@ -1,8 +1,9 @@ +local a, b; do pcall(function() - local a = 42; + a = 42; end); do - local b = "finally"; + b = "finally"; end end diff --git a/test/translation/lua/tupleReturn.lua b/test/translation/lua/tupleReturn.lua index a75c85272..077d7edb2 100644 --- a/test/translation/lua/tupleReturn.lua +++ b/test/translation/lua/tupleReturn.lua @@ -1,28 +1,30 @@ -tupleReturn = function() - return 0, "foobar"; -end; -tupleReturn(); -noTupleReturn(); -local a, b = tupleReturn(); -local c, d = table.unpack(noTupleReturn()); -a, b = tupleReturn(); -c, d = table.unpack(noTupleReturn()); -local e = ({tupleReturn()}); -local f = noTupleReturn(); -e = ({tupleReturn()}); -f = noTupleReturn(); -foo(({tupleReturn()})); -foo(noTupleReturn()); -tupleReturnFromVar = function() - local r = {1, "baz"}; - return table.unpack(r); -end; -tupleReturnForward = function() - return tupleReturn(); -end; -tupleNoForward = function() - return ({tupleReturn()}); -end; -tupleReturnUnpack = function() - return table.unpack(tupleNoForward()); -end; +local a, b, c, d, e, f; +tupleReturn = function() + return 0, "foobar"; +end; +tupleReturnFromVar = function() + local r; + r = {1, "baz"}; + return table.unpack(r); +end; +tupleReturnForward = function() + return tupleReturn(); +end; +tupleNoForward = function() + return ({tupleReturn()}); +end; +tupleReturnUnpack = function() + return table.unpack(tupleNoForward()); +end; +tupleReturn(); +noTupleReturn(); +a, b = tupleReturn(); +c, d = table.unpack(noTupleReturn()); +a, b = tupleReturn(); +c, d = table.unpack(noTupleReturn()); +e = ({tupleReturn()}); +f = noTupleReturn(); +e = ({tupleReturn()}); +f = noTupleReturn(); +foo(({tupleReturn()})); +foo(noTupleReturn()); diff --git a/test/translation/lua/typeAssert.lua b/test/translation/lua/typeAssert.lua index 1c7069d36..8c9aa72e6 100644 --- a/test/translation/lua/typeAssert.lua +++ b/test/translation/lua/typeAssert.lua @@ -1,2 +1,3 @@ -local test1 = 10; -local test2 = 10; +local test1, test2; +test1 = 10; +test2 = 10; diff --git a/test/translation/lua/while.lua b/test/translation/lua/while.lua index 5571f8880..763fcf570 100644 --- a/test/translation/lua/while.lua +++ b/test/translation/lua/while.lua @@ -1,7 +1,8 @@ -local d = 10; +local d; +d = 10; while d > 0 do do d = d - 1; end - ::__continue0:: + ::__continue1:: end From 5fff23cb1386b558359d6c8aed89c52d801d97ec Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Mon, 28 Jan 2019 06:15:14 -0700 Subject: [PATCH 07/24] fixed tests --- test/translation/lua/modulesImportAll.lua | 3 ++- test/translation/lua/modulesImportNamed.lua | 3 ++- test/translation/lua/modulesImportRenamed.lua | 3 ++- test/unit/assignmentDestructuring.spec.ts | 6 +++--- test/unit/assignments.spec.ts | 14 +++++++------- test/unit/enum.spec.ts | 8 ++++---- test/unit/expressions.spec.ts | 4 ++-- test/unit/objectLiteral.spec.ts | 2 +- 8 files changed, 23 insertions(+), 20 deletions(-) diff --git a/test/translation/lua/modulesImportAll.lua b/test/translation/lua/modulesImportAll.lua index 334fc07a7..35e6b9e8d 100644 --- a/test/translation/lua/modulesImportAll.lua +++ b/test/translation/lua/modulesImportAll.lua @@ -1 +1,2 @@ -local Test = require("test"); +local Test; +Test = require("test"); diff --git a/test/translation/lua/modulesImportNamed.lua b/test/translation/lua/modulesImportNamed.lua index 27cbae8c9..732452bf7 100644 --- a/test/translation/lua/modulesImportNamed.lua +++ b/test/translation/lua/modulesImportNamed.lua @@ -1,2 +1,3 @@ +local TestClass; local __TSTL_test = require("test"); -local TestClass = __TSTL_test.TestClass; +TestClass = __TSTL_test.TestClass; diff --git a/test/translation/lua/modulesImportRenamed.lua b/test/translation/lua/modulesImportRenamed.lua index 84c484b5d..40fec9c5c 100644 --- a/test/translation/lua/modulesImportRenamed.lua +++ b/test/translation/lua/modulesImportRenamed.lua @@ -1,2 +1,3 @@ +local RenamedClass; local __TSTL_test = require("test"); -local RenamedClass = __TSTL_test.TestClass; +RenamedClass = __TSTL_test.TestClass; diff --git a/test/unit/assignmentDestructuring.spec.ts b/test/unit/assignmentDestructuring.spec.ts index c503d871a..a0357204a 100644 --- a/test/unit/assignmentDestructuring.spec.ts +++ b/test/unit/assignmentDestructuring.spec.ts @@ -15,7 +15,7 @@ export class AssignmentDestructuringTests { this.assignmentDestruturingTs, {luaTarget: LuaTarget.Lua51, luaLibImport: "none"} ); // Assert - Expect(lua).toBe(`local a, b = unpack(myFunc());`); + Expect(lua).toBe(`local a, b;\na, b = unpack(myFunc());`); } @Test("Assignment destructuring [5.2]") @@ -25,7 +25,7 @@ export class AssignmentDestructuringTests { this.assignmentDestruturingTs, {luaTarget: LuaTarget.Lua52, luaLibImport: "none"} ); // Assert - Expect(lua).toBe(`local a, b = table.unpack(myFunc());`); + Expect(lua).toBe(`local a, b;\na, b = table.unpack(myFunc());`); } @Test("Assignment destructuring [JIT]") @@ -35,6 +35,6 @@ export class AssignmentDestructuringTests { this.assignmentDestruturingTs, {luaTarget: LuaTarget.LuaJIT, luaLibImport: "none"} ); // Assert - Expect(lua).toBe(`local a, b = unpack(myFunc());`); + Expect(lua).toBe(`local a, b;\na, b = unpack(myFunc());`); } } diff --git a/test/unit/assignments.spec.ts b/test/unit/assignments.spec.ts index 94034ab25..a5b13d7ba 100644 --- a/test/unit/assignments.spec.ts +++ b/test/unit/assignments.spec.ts @@ -32,7 +32,7 @@ export class AssignmentTests { @Test("Const assignment") public constAssignment(inp: string, out: string): void { const lua = util.transpileString(`const myvar = ${inp};`); - Expect(lua).toBe(`local myvar = ${out};`); + Expect(lua).toBe(`local myvar;\nmyvar = ${out};`); } @TestCase(`"abc"`, `"abc"`) @@ -44,7 +44,7 @@ export class AssignmentTests { @Test("Let assignment") public letAssignment(inp: string, out: string): void { const lua = util.transpileString(`let myvar = ${inp};`); - Expect(lua).toBe(`local myvar = ${out};`); + Expect(lua).toBe(`local myvar;\nmyvar = ${out};`); } @TestCase(`"abc"`, `"abc"`) @@ -105,7 +105,7 @@ export class AssignmentTests { + `let [a,b] = abc();`; const lua = util.transpileString(code); - Expect(lua).toBe("local a, b = abc();"); + Expect(lua).toBe("local a, b;\na, b = abc();"); } @Test("TupleReturn Single assignment") @@ -116,7 +116,7 @@ export class AssignmentTests { + `a = abc();`; const lua = util.transpileString(code); - Expect(lua).toBe("local a = ({abc()});\na = ({abc()});"); + Expect(lua).toBe("local a;\na = ({abc()});\na = ({abc()});"); } @Test("TupleReturn interface assignment") @@ -128,7 +128,7 @@ export class AssignmentTests { + `let [a,b] = jkl.abc();`; const lua = util.transpileString(code); - Expect(lua).toBe("local a, b = jkl:abc();"); + Expect(lua).toBe("local a, b;\na, b = jkl:abc();"); } @Test("TupleReturn namespace assignment") @@ -140,7 +140,7 @@ export class AssignmentTests { + `let [a,b] = def.abc();`; const lua = util.transpileString(code); - Expect(lua).toBe("local a, b = def.abc();"); + Expect(lua).toBe("local a, b;\na, b = def.abc();"); } @Test("TupleReturn method assignment") @@ -152,7 +152,7 @@ export class AssignmentTests { + `let [a,b] = jkl.abc();`; const lua = util.transpileString(code); - Expect(lua).toBe("local jkl = def.new(true);\nlocal a, b = jkl:abc();"); + Expect(lua).toBe("local jkl, a, b;\njkl = def.new(true);\na, b = jkl:abc();"); } @Test("TupleReturn functional") diff --git a/test/unit/enum.spec.ts b/test/unit/enum.spec.ts index b2a2a09ce..5e771adcb 100644 --- a/test/unit/enum.spec.ts +++ b/test/unit/enum.spec.ts @@ -15,7 +15,7 @@ export class EnumTests { const valueOne = TestEnum.MEMBER_ONE; `; - Expect(util.transpileString(testCode)).toBe(`local valueOne = "test";`); + Expect(util.transpileString(testCode)).toBe(`local valueOne;\nvalueOne = "test";`); } @Test("Const enum") @@ -29,7 +29,7 @@ export class EnumTests { const valueOne = TestEnum.MEMBER_ONE; `; - Expect(util.transpileString(testCode)).toBe(`local valueOne = "test";`); + Expect(util.transpileString(testCode)).toBe(`local valueOne;\nvalueOne = "test";`); } @Test("Const enum without initializer") @@ -43,7 +43,7 @@ export class EnumTests { const valueOne = TestEnum.MEMBER_ONE; `; - Expect(util.transpileString(testCode)).toBe(`local valueOne = 0;`); + Expect(util.transpileString(testCode)).toBe(`local valueOne;\nvalueOne = 0;`); } @Test("Const enum without initializer in some values") @@ -58,7 +58,7 @@ export class EnumTests { const valueOne = TestEnum.MEMBER_TWO; `; - Expect(util.transpileString(testCode)).toBe(`local valueOne = 4;`); + Expect(util.transpileString(testCode)).toBe(`local valueOne;\nvalueOne = 4;`); } @Test("Invalid heterogeneous enum") diff --git a/test/unit/expressions.spec.ts b/test/unit/expressions.spec.ts index bb2cfb0bc..70242b92c 100644 --- a/test/unit/expressions.spec.ts +++ b/test/unit/expressions.spec.ts @@ -13,9 +13,9 @@ export class ExpressionTests { @TestCase("--i", "i = i - 1;") @TestCase("!a", "not a;") @TestCase("-a", "-a;") - @TestCase("let a = delete tbl['test']", "local a = (function()\n tbl.test = nil;\n return true;\nend)();") + @TestCase("let a = delete tbl['test']", "local a;\na = (function()\n tbl.test = nil;\n return true;\nend)();") @TestCase("delete tbl['test']", "tbl.test = nil;") - @TestCase("let a = delete tbl.test", "local a = (function()\n tbl.test = nil;\n return true;\nend)();") + @TestCase("let a = delete tbl.test", "local a;\na = (function()\n tbl.test = nil;\n return true;\nend)();") @TestCase("delete tbl.test", "tbl.test = nil;") @Test("Unary expressions basic") public unaryBasic(input: string, lua: string): void { diff --git a/test/unit/objectLiteral.spec.ts b/test/unit/objectLiteral.spec.ts index dd94ff419..bc4930d39 100644 --- a/test/unit/objectLiteral.spec.ts +++ b/test/unit/objectLiteral.spec.ts @@ -14,7 +14,7 @@ export class ObjectLiteralTests { @Test("Object Literal") public objectLiteral(inp: string, out: string): void { const lua = util.transpileString(`const myvar = ${inp};`); - Expect(lua).toBe(`local myvar = ${out}`); + Expect(lua).toBe(`local myvar;\nmyvar = ${out}`); } @TestCase("3", 3) From 4927c88053caf139e5889b9d6eda68702ba08d2a Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Mon, 28 Jan 2019 07:06:23 -0700 Subject: [PATCH 08/24] More Hoisting Adjustments - pushing scope for try/catch/finally - some refactoring and commenting - fixed more tests --- src/LuaTransformer.ts | 71 +++++++++++-------- test/compiler/projects/watchmode/watch.ts | 2 +- test/compiler/testfiles/watch.ts | 2 +- test/translation/lua/tryCatch.lua | 3 +- test/translation/lua/tryCatchFinally.lua | 4 +- test/translation/lua/tryFinally.lua | 3 +- .../compiler/configuration/options.spec.ts | 4 +- 7 files changed, 52 insertions(+), 37 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 139310412..ad3e1d958 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -174,8 +174,15 @@ export class LuaTransformer { return tstlStatements; } - public transformBlock(block: ts.Block): tstl.Block { - return tstl.createBlock(this.transformStatements(block.statements), block); + public transformBlock(block: ts.Block, pushScope?: boolean): tstl.Block { + if (pushScope) { + this.pushScope(ScopeType.Block); + } + const statements = this.transformStatements(block.statements); + if (pushScope) { + this.popScope(statements); + } + return tstl.createBlock(statements, block); } public transformScopeBlock(block: ts.Block): tstl.DoStatement { @@ -222,18 +229,17 @@ export class LuaTransformer { filteredElements.forEach(importSpecifier => { const nameIdentifier = this.transformIdentifier(importSpecifier.name); - this.addScopeLocals(nameIdentifier); if (importSpecifier.propertyName) { const propertyIdentifier = this.transformIdentifier(importSpecifier.propertyName); const propertyName = tstl.createStringLiteral(propertyIdentifier.text); - const renamedImport = tstl.createAssignmentStatement( + const renamedImport = this.createHoistedVariableDeclaration( nameIdentifier, tstl.createTableIndexExpression(importUniqueName, propertyName), importSpecifier); result.push(renamedImport); } else { const name = tstl.createStringLiteral(nameIdentifier.text); - const namedImport = tstl.createAssignmentStatement( + const namedImport = this.createHoistedVariableDeclaration( nameIdentifier, tstl.createTableIndexExpression(importUniqueName, name), importSpecifier @@ -243,10 +249,8 @@ export class LuaTransformer { }); return result; } else if (ts.isNamespaceImport(imports)) { - const nameIdentifier = this.transformIdentifier(imports.name); - this.addScopeLocals(nameIdentifier); - const requireStatement = tstl.createAssignmentStatement( - nameIdentifier, + const requireStatement = this.createHoistedVariableDeclaration( + this.transformIdentifier(imports.name), requireCall, statement ); @@ -804,8 +808,7 @@ export class LuaTransformer { result.push(namespaceDeclaration); // local innerNS = outerNS.innerNS - this.addScopeLocals(this.transformIdentifier(statement.name as ts.Identifier)); - const localDeclaration = tstl.createAssignmentStatement( + const localDeclaration = this.createHoistedVariableDeclaration( this.transformIdentifier(statement.name as ts.Identifier), tstl.createTableIndexExpression( this.transformIdentifier(this.currentNamespace.name as ts.Identifier), @@ -825,8 +828,7 @@ export class LuaTransformer { result.push(namespaceDeclaration); // local NS = exports.NS - this.addScopeLocals(this.transformIdentifier(statement.name as ts.Identifier)); - const localDeclaration = tstl.createAssignmentStatement( + const localDeclaration = this.createHoistedVariableDeclaration( this.transformIdentifier(statement.name as ts.Identifier), this.createExportedIdentifier(this.transformIdentifier(statement.name as ts.Identifier))); @@ -1488,7 +1490,7 @@ export class LuaTransformer { public transformTryStatement(statement: ts.TryStatement): StatementVisitResult { const pCall = tstl.createIdentifier("pcall"); - const tryBlock = this.transformBlock(statement.tryBlock); + const tryBlock = this.transformBlock(statement.tryBlock, true); const tryCall = tstl.createCallExpression(pCall, [tstl.createFunctionExpression(tryBlock)]); const result: tstl.Statement[] = []; @@ -1505,14 +1507,14 @@ export class LuaTransformer { result.push(catchAssignment); const notTryResult = tstl.createUnaryExpression(tryResult, tstl.SyntaxKind.NotOperator); - result.push(tstl.createIfStatement(notTryResult, this.transformBlock(statement.catchClause.block))); + result.push(tstl.createIfStatement(notTryResult, this.transformBlock(statement.catchClause.block, true))); } else { result.push(tstl.createExpressionStatement(tryCall)); } if (statement.finallyBlock) { - result.push(tstl.createDoStatement(this.transformBlock(statement.finallyBlock).statements)); + result.push(tstl.createDoStatement(this.transformBlock(statement.finallyBlock, true).statements)); } return tstl.createDoStatement( @@ -3280,12 +3282,7 @@ export class LuaTransformer { } protected findScope(...scopeTypes: ScopeType[]): Scope | undefined { - for (let i = this.scopeStack.length - 1; i >= 0; --i) { - if (scopeTypes.indexOf(this.scopeStack[i].type) >= 0) { - return this.scopeStack[i]; - } - } - return undefined; + return this.scopeStack.slice().reverse().find(s => scopeTypes.find(t => s.type === t) !== undefined); } protected peekScope(): Scope { @@ -3297,12 +3294,18 @@ export class LuaTransformer { this.genVarCounter++; } + // prepends hoisted declarations to statements protected popScope(statements: tstl.Statement[]): Scope { const scope = this.scopeStack.pop(); + + // hoisted function declarations statements.unshift(...scope.functions); + + // hoisted locals if (scope.locals.length > 0) { statements.unshift(tstl.createVariableDeclarationStatement(scope.locals)); } + return scope; } @@ -3310,15 +3313,12 @@ export class LuaTransformer { let scope: Scope; if (!scopeType) { scope = this.peekScope(); + } else { - for (let i = this.scopeStack.length - 1; i >= 0; --i) { - if (this.scopeStack[i].type === scopeType) { - scope = this.scopeStack[i]; - break; - } - } - scope = scope || this.scopeStack[0]; // Default to file scope + // find first scope with desired type (default to file scope) + scope = this.scopeStack.slice().reverse().find(s => s.type === scopeType) || this.scopeStack[0]; } + if (Array.isArray(locals)) { scope.locals.push(...locals); } else { @@ -3326,11 +3326,22 @@ export class LuaTransformer { } } - protected addScopeFunction(func: tstl.AssignmentStatement): void { + protected addScopeFunction(func: tstl.Statement): void { const scope = this.peekScope(); scope.functions.push(func); } + protected createHoistedVariableDeclaration( + variable: tstl.Identifier, + initializer?: tstl.Expression, + tsOriginal?: ts.Node, + parent?: tstl.Node + ): tstl.AssignmentStatement + { + this.addScopeLocals(variable); + return tstl.createAssignmentStatement(variable, initializer, tsOriginal, parent); + } + private statementVisitResultToStatementArray(visitResult: StatementVisitResult): tstl.Statement[] { if (!Array.isArray(visitResult)) { if (visitResult) { diff --git a/test/compiler/projects/watchmode/watch.ts b/test/compiler/projects/watchmode/watch.ts index 4e2a15260..eb6402268 100644 --- a/test/compiler/projects/watchmode/watch.ts +++ b/test/compiler/projects/watchmode/watch.ts @@ -1 +1 @@ -class MyTest {} \ No newline at end of file +class MyTest2 {} \ No newline at end of file diff --git a/test/compiler/testfiles/watch.ts b/test/compiler/testfiles/watch.ts index 4e2a15260..eb6402268 100644 --- a/test/compiler/testfiles/watch.ts +++ b/test/compiler/testfiles/watch.ts @@ -1 +1 @@ -class MyTest {} \ No newline at end of file +class MyTest2 {} \ No newline at end of file diff --git a/test/translation/lua/tryCatch.lua b/test/translation/lua/tryCatch.lua index 62b256af5..92250d99f 100644 --- a/test/translation/lua/tryCatch.lua +++ b/test/translation/lua/tryCatch.lua @@ -1,9 +1,10 @@ -local a, b; do local ____TS_try, er = pcall(function() + local a; a = 42; end); if not ____TS_try then + local b; b = "fail"; end end diff --git a/test/translation/lua/tryCatchFinally.lua b/test/translation/lua/tryCatchFinally.lua index 53059e25a..65578d6ee 100644 --- a/test/translation/lua/tryCatchFinally.lua +++ b/test/translation/lua/tryCatchFinally.lua @@ -1,12 +1,14 @@ -local a, b, c; do local ____TS_try, er = pcall(function() + local a; a = 42; end); if not ____TS_try then + local b; b = "fail"; end do + local c; c = "finally"; end end diff --git a/test/translation/lua/tryFinally.lua b/test/translation/lua/tryFinally.lua index a234b2db7..e23df9fee 100644 --- a/test/translation/lua/tryFinally.lua +++ b/test/translation/lua/tryFinally.lua @@ -1,9 +1,10 @@ -local a, b; do pcall(function() + local a; a = 42; end); do + local b; b = "finally"; end end diff --git a/test/unit/compiler/configuration/options.spec.ts b/test/unit/compiler/configuration/options.spec.ts index f1338b6b1..64b6476ab 100644 --- a/test/unit/compiler/configuration/options.spec.ts +++ b/test/unit/compiler/configuration/options.spec.ts @@ -24,6 +24,6 @@ export class ObjectLiteralTests const options = {LuaLibImportKind: importKind}; const result = util.transpileString("const a = new Map();", options); - Expect(result).toBe("local a = Map.new(true);"); + Expect(result).toBe("local a;\na = Map.new(true);"); } -} \ No newline at end of file +} From 77303dc296e89157f440d6946e4c4d614d629a01 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Mon, 28 Jan 2019 14:05:48 -0700 Subject: [PATCH 09/24] reverting accidentally changed test files --- test/compiler/projects/watchmode/watch.ts | 2 +- test/compiler/testfiles/watch.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/compiler/projects/watchmode/watch.ts b/test/compiler/projects/watchmode/watch.ts index eb6402268..4e2a15260 100644 --- a/test/compiler/projects/watchmode/watch.ts +++ b/test/compiler/projects/watchmode/watch.ts @@ -1 +1 @@ -class MyTest2 {} \ No newline at end of file +class MyTest {} \ No newline at end of file diff --git a/test/compiler/testfiles/watch.ts b/test/compiler/testfiles/watch.ts index eb6402268..4e2a15260 100644 --- a/test/compiler/testfiles/watch.ts +++ b/test/compiler/testfiles/watch.ts @@ -1 +1 @@ -class MyTest2 {} \ No newline at end of file +class MyTest {} \ No newline at end of file From e1678124a5f9f9e5043cb50f76eae497ccf9dc78 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Mon, 28 Jan 2019 16:41:39 -0700 Subject: [PATCH 10/24] renamed some things for clarity --- src/LuaTransformer.ts | 63 +++++++++++++++++++------------------- test/src/util.ts | 2 +- test/unit/hoisting.spec.ts | 18 +++++------ 3 files changed, 41 insertions(+), 42 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index ad3e1d958..d4fe87478 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -23,8 +23,8 @@ export enum ScopeType { interface Scope { type: ScopeType; id: number; - locals: tstl.Identifier[]; - functions: tstl.Statement[]; + hoistedLocals: tstl.Identifier[]; + hoistedFunctions: tstl.Statement[]; } export class LuaTransformer { @@ -82,7 +82,7 @@ export class LuaTransformer { this.isModule = tsHelper.isFileModule(node); const statements = this.transformStatements(node.statements); - this.popScope(statements); + this.popScopeAndPrependHoistedStatements(statements); if (this.isModule) { statements.unshift( @@ -180,7 +180,7 @@ export class LuaTransformer { } const statements = this.transformStatements(block.statements); if (pushScope) { - this.popScope(statements); + this.popScopeAndPrependHoistedStatements(statements); } return tstl.createBlock(statements, block); } @@ -188,7 +188,7 @@ export class LuaTransformer { public transformScopeBlock(block: ts.Block): tstl.DoStatement { this.pushScope(ScopeType.Block); const statements = this.transformStatements(block.statements); - this.popScope(statements); + this.popScopeAndPrependHoistedStatements(statements); return tstl.createDoStatement(statements, block); } @@ -232,14 +232,14 @@ export class LuaTransformer { if (importSpecifier.propertyName) { const propertyIdentifier = this.transformIdentifier(importSpecifier.propertyName); const propertyName = tstl.createStringLiteral(propertyIdentifier.text); - const renamedImport = this.createHoistedVariableDeclaration( + const renamedImport = this.createHoistedVariableDeclarationStatement( nameIdentifier, tstl.createTableIndexExpression(importUniqueName, propertyName), importSpecifier); result.push(renamedImport); } else { const name = tstl.createStringLiteral(nameIdentifier.text); - const namedImport = this.createHoistedVariableDeclaration( + const namedImport = this.createHoistedVariableDeclarationStatement( nameIdentifier, tstl.createTableIndexExpression(importUniqueName, name), importSpecifier @@ -249,7 +249,7 @@ export class LuaTransformer { }); return result; } else if (ts.isNamespaceImport(imports)) { - const requireStatement = this.createHoistedVariableDeclaration( + const requireStatement = this.createHoistedVariableDeclarationStatement( this.transformIdentifier(imports.name), requireCall, statement @@ -762,7 +762,7 @@ export class LuaTransformer { const bodyStatements = this.transformStatements(body.statements); - this.popScope(bodyStatements); + this.popScopeAndPrependHoistedStatements(bodyStatements); return headerStatements.concat(bodyStatements); } @@ -808,7 +808,7 @@ export class LuaTransformer { result.push(namespaceDeclaration); // local innerNS = outerNS.innerNS - const localDeclaration = this.createHoistedVariableDeclaration( + const localDeclaration = this.createHoistedVariableDeclarationStatement( this.transformIdentifier(statement.name as ts.Identifier), tstl.createTableIndexExpression( this.transformIdentifier(this.currentNamespace.name as ts.Identifier), @@ -828,7 +828,7 @@ export class LuaTransformer { result.push(namespaceDeclaration); // local NS = exports.NS - const localDeclaration = this.createHoistedVariableDeclaration( + const localDeclaration = this.createHoistedVariableDeclarationStatement( this.transformIdentifier(statement.name as ts.Identifier), this.createExportedIdentifier(this.transformIdentifier(statement.name as ts.Identifier))); @@ -857,7 +857,7 @@ export class LuaTransformer { if (statement.body && ts.isModuleBlock(statement.body)) { this.pushScope(ScopeType.Block); const statements = this.transformStatements(statement.body.statements); - this.popScope(statements); + this.popScopeAndPrependHoistedStatements(statements); result.push(tstl.createDoStatement(statements)); } @@ -1136,7 +1136,7 @@ export class LuaTransformer { this.pushScope(ScopeType.Conditional); const condition = this.transformExpression(statement.expression); const statements = this.transformBlockOrStatement(statement.thenStatement); - this.popScope(statements); + this.popScopeAndPrependHoistedStatements(statements); const ifBlock = tstl.createBlock(statements); if (statement.elseStatement) { if (ts.isIfStatement(statement.elseStatement)) { @@ -1144,7 +1144,7 @@ export class LuaTransformer { } else { this.pushScope(ScopeType.Conditional); const elseStatements = this.transformBlockOrStatement(statement.elseStatement); - this.popScope(elseStatements); + this.popScopeAndPrependHoistedStatements(elseStatements); const elseBlock = tstl.createBlock(elseStatements); return tstl.createIfStatement(condition, ifBlock, elseBlock); } @@ -1200,7 +1200,7 @@ export class LuaTransformer { // Declaration of new variable this.pushScope(ScopeType.Loop); // Counter hoisting - there's probably a better way to handle this const variableDeclarations = this.transformVariableDeclaration(initializer.declarations[0]); - this.popScope(variableDeclarations); + this.popScopeAndPrependHoistedStatements(variableDeclarations); if (ts.isArrayBindingPattern(initializer.declarations[0].name)) { expression = this.createUnpackCall(expression, initializer); } @@ -1228,7 +1228,7 @@ export class LuaTransformer { { this.pushScope(ScopeType.Loop); const body = this.transformBlockOrStatement(loop.statement); - const scopeId = this.popScope(body).id; + const scopeId = this.popScopeAndPrependHoistedStatements(body).id; if (this.options.luaTarget === LuaTarget.Lua51) { return body; @@ -1474,7 +1474,7 @@ export class LuaTransformer { statements.push(tstl.createLabelStatement(`${switchName}_end`)); - this.popScope(statements); + this.popScopeAndPrependHoistedStatements(statements); return statements; } @@ -3178,13 +3178,13 @@ export class LuaTransformer { if (this.isModule || this.currentNamespace || insideFunction || isLetOrConst) { // hoist locals const scopeType = !(isLetOrConst || isFunctionDeclaration) ? ScopeType.Function : undefined; - this.addScopeLocals(lhs, scopeType); + this.hoistLocals(lhs, scopeType); } } if (isFunctionDeclaration) { // hoist function declarations - this.addScopeFunction(tstl.createAssignmentStatement(left, rhs, tsOriginal, parent)); + this.hoistFunctionDeclaration(tstl.createAssignmentStatement(left, rhs, tsOriginal, parent)); return undefined; } @@ -3290,26 +3290,25 @@ export class LuaTransformer { } protected pushScope(scopeType: ScopeType): void { - this.scopeStack.push({type: scopeType, id: this.genVarCounter, locals: [], functions: []}); + this.scopeStack.push({type: scopeType, id: this.genVarCounter, hoistedLocals: [], hoistedFunctions: []}); this.genVarCounter++; } - // prepends hoisted declarations to statements - protected popScope(statements: tstl.Statement[]): Scope { + protected popScopeAndPrependHoistedStatements(statements: tstl.Statement[]): Scope { const scope = this.scopeStack.pop(); // hoisted function declarations - statements.unshift(...scope.functions); + statements.unshift(...scope.hoistedFunctions); // hoisted locals - if (scope.locals.length > 0) { - statements.unshift(tstl.createVariableDeclarationStatement(scope.locals)); + if (scope.hoistedLocals.length > 0) { + statements.unshift(tstl.createVariableDeclarationStatement(scope.hoistedLocals)); } return scope; } - protected addScopeLocals(locals: tstl.Identifier | tstl.Identifier[], scopeType?: ScopeType): void { + protected hoistLocals(locals: tstl.Identifier | tstl.Identifier[], scopeType?: ScopeType): void { let scope: Scope; if (!scopeType) { scope = this.peekScope(); @@ -3320,25 +3319,25 @@ export class LuaTransformer { } if (Array.isArray(locals)) { - scope.locals.push(...locals); + scope.hoistedLocals.push(...locals); } else { - scope.locals.push(locals); + scope.hoistedLocals.push(locals); } } - protected addScopeFunction(func: tstl.Statement): void { + protected hoistFunctionDeclaration(func: tstl.Statement): void { const scope = this.peekScope(); - scope.functions.push(func); + scope.hoistedFunctions.push(func); } - protected createHoistedVariableDeclaration( + protected createHoistedVariableDeclarationStatement( variable: tstl.Identifier, initializer?: tstl.Expression, tsOriginal?: ts.Node, parent?: tstl.Node ): tstl.AssignmentStatement { - this.addScopeLocals(variable); + this.hoistLocals(variable); return tstl.createAssignmentStatement(variable, initializer, tsOriginal, parent); } diff --git a/test/src/util.ts b/test/src/util.ts index 867c5a95e..fe63b218f 100644 --- a/test/src/util.ts +++ b/test/src/util.ts @@ -98,7 +98,7 @@ export function transpileAndExecute( return executeLua(lua); } -export function transpileAndExecuteWithExport( +export function transpileExecuteAndReturnExport( tsStr: string, returnExport: string, compilerOptions?: CompilerOptions, diff --git a/test/unit/hoisting.spec.ts b/test/unit/hoisting.spec.ts index 07bbf495d..2927ac8cf 100644 --- a/test/unit/hoisting.spec.ts +++ b/test/unit/hoisting.spec.ts @@ -19,7 +19,7 @@ export class HoistingTests { const code = `foo = "foo"; export var foo;`; - const result = util.transpileAndExecuteWithExport(code, "foo"); + const result = util.transpileExecuteAndReturnExport(code, "foo"); Expect(result).toBe("foo"); } @@ -46,7 +46,7 @@ export class HoistingTests { function setBar() { bar = foo; } export ${varType} foo = "foo"; setBar();`; - const result = util.transpileAndExecuteWithExport(code, "foo"); + const result = util.transpileExecuteAndReturnExport(code, "foo"); Expect(result).toBe("foo"); } @@ -65,7 +65,7 @@ export class HoistingTests { const code = `export const foo = bar(); function bar() { return "bar"; }`; - const result = util.transpileAndExecuteWithExport(code, "foo"); + const result = util.transpileExecuteAndReturnExport(code, "foo"); Expect(result).toBe("bar"); } @@ -75,7 +75,7 @@ export class HoistingTests { `const foo = bar(); export function bar() { return "bar"; } export const baz = foo;`; - const result = util.transpileAndExecuteWithExport(code, "baz"); + const result = util.transpileExecuteAndReturnExport(code, "baz"); Expect(result).toBe("bar"); } @@ -131,7 +131,7 @@ export class HoistingTests { export let foo = "foo"; } export const foo = bar();`; - const result = util.transpileAndExecuteWithExport(code, "foo"); + const result = util.transpileExecuteAndReturnExport(code, "foo"); Expect(result).toBe("foo"); } @@ -145,7 +145,7 @@ export class HoistingTests { export let foo = "foo"; } export const foo = bar();`; - const result = util.transpileAndExecuteWithExport(code, "foo"); + const result = util.transpileExecuteAndReturnExport(code, "foo"); Expect(result).toBe("foo"); } @@ -161,7 +161,7 @@ export class HoistingTests { } } export const foo = Outer.bar();`; - const result = util.transpileAndExecuteWithExport(code, "foo"); + const result = util.transpileExecuteAndReturnExport(code, "foo"); Expect(result).toBe("foo"); } @@ -175,7 +175,7 @@ export class HoistingTests { public bar = "foo"; } export const foo = makeFoo().bar;`; - const result = util.transpileAndExecuteWithExport(code, "foo"); + const result = util.transpileExecuteAndReturnExport(code, "foo"); Expect(result).toBe("foo"); } @@ -189,7 +189,7 @@ export class HoistingTests { A = "foo" } export const foo = bar();`; - const result = util.transpileAndExecuteWithExport(code, "foo"); + const result = util.transpileExecuteAndReturnExport(code, "foo"); Expect(result).toBe("foo"); } } From 3000e3b3c240648fcbb9c6cefe096901cb906d62 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Wed, 30 Jan 2019 17:01:17 -0700 Subject: [PATCH 11/24] working on possible smart-hoisting detection --- src/LuaAST.ts | 10 ++++++- src/LuaTransformer.ts | 61 ++++++++++++++++++++++++++++++------------- src/TSHelper.ts | 27 +++++++++++++++++++ 3 files changed, 79 insertions(+), 19 deletions(-) diff --git a/src/LuaAST.ts b/src/LuaAST.ts index 31ab9975d..cba7fb475 100644 --- a/src/LuaAST.ts +++ b/src/LuaAST.ts @@ -106,6 +106,7 @@ export interface TextRange { export interface Node extends TextRange { kind: SyntaxKind; parent?: Node; + tsOriginal?: ts.Node; } export function createNode(kind: SyntaxKind, tsOriginal?: ts.Node, parent?: Node): Node { @@ -115,7 +116,7 @@ export function createNode(kind: SyntaxKind, tsOriginal?: ts.Node, parent?: Node pos = tsOriginal.pos; end = tsOriginal.end; } - return {kind, parent, pos, end}; + return {kind, parent, pos, end, tsOriginal}; } export function cloneNode(node: T): T { @@ -125,6 +126,7 @@ export function cloneNode(node: T): T { export function setNodeOriginal(node: T, tsOriginal: ts.Node): T { node.pos = tsOriginal.pos; node.end = tsOriginal.end; + node.tsOriginal = tsOriginal; return node; } @@ -139,6 +141,9 @@ export function setParent(node: Node | Node[] | undefined, parent: Node): void n.pos = parent.pos; n.end = parent.end; } + if (!n.tsOriginal) { + n.tsOriginal = parent.tsOriginal; + } }); } else { node.parent = parent; @@ -146,6 +151,9 @@ export function setParent(node: Node | Node[] | undefined, parent: Node): void node.pos = parent.pos; node.end = parent.end; } + if (!node.tsOriginal) { + node.tsOriginal = parent.tsOriginal; + } } } diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index d4fe87478..465563b39 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -23,6 +23,7 @@ export enum ScopeType { interface Scope { type: ScopeType; id: number; + node: ts.Node; hoistedLocals: tstl.Identifier[]; hoistedFunctions: tstl.Statement[]; } @@ -76,7 +77,7 @@ export class LuaTransformer { // TODO make all other methods private??? public transformSourceFile(node: ts.SourceFile): [tstl.Block, Set] { this.setupState(); - this.pushScope(ScopeType.File); + this.pushScope(ScopeType.File, node); this.currentSourceFile = node; this.isModule = tsHelper.isFileModule(node); @@ -176,7 +177,7 @@ export class LuaTransformer { public transformBlock(block: ts.Block, pushScope?: boolean): tstl.Block { if (pushScope) { - this.pushScope(ScopeType.Block); + this.pushScope(ScopeType.Block, block); } const statements = this.transformStatements(block.statements); if (pushScope) { @@ -186,7 +187,7 @@ export class LuaTransformer { } public transformScopeBlock(block: ts.Block): tstl.DoStatement { - this.pushScope(ScopeType.Block); + this.pushScope(ScopeType.Block, block); const statements = this.transformStatements(block.statements); this.popScopeAndPrependHoistedStatements(statements); return tstl.createDoStatement(statements, block); @@ -743,7 +744,7 @@ export class LuaTransformer { spreadIdentifier?: tstl.Identifier ): tstl.Statement[] { - this.pushScope(ScopeType.Function); + this.pushScope(ScopeType.Function, body); const headerStatements = []; @@ -855,7 +856,7 @@ export class LuaTransformer { // Transform moduleblock to block and visit it if (statement.body && ts.isModuleBlock(statement.body)) { - this.pushScope(ScopeType.Block); + this.pushScope(ScopeType.Block, statement); const statements = this.transformStatements(statement.body.statements); this.popScopeAndPrependHoistedStatements(statements); result.push(tstl.createDoStatement(statements)); @@ -1133,7 +1134,7 @@ export class LuaTransformer { } public transformIfStatement(statement: ts.IfStatement): tstl.IfStatement { - this.pushScope(ScopeType.Conditional); + this.pushScope(ScopeType.Conditional, statement.thenStatement); const condition = this.transformExpression(statement.expression); const statements = this.transformBlockOrStatement(statement.thenStatement); this.popScopeAndPrependHoistedStatements(statements); @@ -1142,7 +1143,7 @@ export class LuaTransformer { if (ts.isIfStatement(statement.elseStatement)) { return tstl.createIfStatement(condition, ifBlock, this.transformIfStatement(statement.elseStatement)); } else { - this.pushScope(ScopeType.Conditional); + this.pushScope(ScopeType.Conditional, statement.elseStatement); const elseStatements = this.transformBlockOrStatement(statement.elseStatement); this.popScopeAndPrependHoistedStatements(elseStatements); const elseBlock = tstl.createBlock(elseStatements); @@ -1198,7 +1199,7 @@ export class LuaTransformer { public transformForOfInitializer(initializer: ts.ForInitializer, expression: tstl.Expression): tstl.Statement { if (ts.isVariableDeclarationList(initializer)) { // Declaration of new variable - this.pushScope(ScopeType.Loop); // Counter hoisting - there's probably a better way to handle this + this.pushScope(ScopeType.Loop, initializer); // Counter hoisting - there's probably a better way to handle this const variableDeclarations = this.transformVariableDeclaration(initializer.declarations[0]); this.popScopeAndPrependHoistedStatements(variableDeclarations); if (ts.isArrayBindingPattern(initializer.declarations[0].name)) { @@ -1226,7 +1227,7 @@ export class LuaTransformer { loop: ts.WhileStatement | ts.DoStatement | ts.ForStatement | ts.ForOfStatement | ts.ForInOrOfStatement ): tstl.Statement[] { - this.pushScope(ScopeType.Loop); + this.pushScope(ScopeType.Loop, loop.statement); const body = this.transformBlockOrStatement(loop.statement); const scopeId = this.popScopeAndPrependHoistedStatements(body).id; @@ -1429,7 +1430,7 @@ export class LuaTransformer { throw TSTLErrors.UnsupportedForTarget("Switch statements", this.options.luaTarget, statement); } - this.pushScope(ScopeType.Switch); + this.pushScope(ScopeType.Switch, statement); // Give the switch a unique name to prevent nested switches from acting up. const switchName = `____TS_switch${this.peekScope().id}`; @@ -3153,6 +3154,20 @@ export class LuaTransformer { return tstl.createIdentifier("self", tsOriginal); } + private shouldHoistIdentifier(identifier: tstl.Identifier, isVar: boolean): boolean { + if (!identifier.tsOriginal || identifier.tsOriginal.pos < 0) { + return false; + } + + const scope = isVar ? this.findScope(ScopeType.Function) : this.peekScope(); + const firstReference = tsHelper.findFirstReferenceInScope( + identifier.tsOriginal as ts.Identifier, + scope.node, + this.checker + ); + return firstReference !== undefined && firstReference.pos < identifier.tsOriginal.pos; + } + private createLocalOrExportedOrGlobalDeclaration( lhs: tstl.Identifier | tstl.Identifier[], rhs?: tstl.Expression, @@ -3176,9 +3191,15 @@ export class LuaTransformer { const isLetOrConst = tsOriginal && ts.isVariableDeclaration(tsOriginal) && tsOriginal.parent && (tsOriginal.parent.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) !== 0; if (this.isModule || this.currentNamespace || insideFunction || isLetOrConst) { - // hoist locals - const scopeType = !(isLetOrConst || isFunctionDeclaration) ? ScopeType.Function : undefined; - this.hoistLocals(lhs, scopeType); + const isVar = !(isLetOrConst || isFunctionDeclaration); + const hoist = isFunctionDeclaration || (Array.isArray(lhs) + ? lhs.some(i => this.shouldHoistIdentifier(i, !isLetOrConst)) + : this.shouldHoistIdentifier(lhs, isVar)); + if (hoist) { + this.hoistLocals(lhs, isLetOrConst || isFunctionDeclaration ? undefined : ScopeType.Function); + } else { + return [tstl.createVariableDeclarationStatement(lhs, rhs, tsOriginal, parent)]; + } } } @@ -3289,8 +3310,8 @@ export class LuaTransformer { return this.scopeStack[this.scopeStack.length - 1]; } - protected pushScope(scopeType: ScopeType): void { - this.scopeStack.push({type: scopeType, id: this.genVarCounter, hoistedLocals: [], hoistedFunctions: []}); + protected pushScope(scopeType: ScopeType, node: ts.Node): void { + this.scopeStack.push({type: scopeType, id: this.genVarCounter, node, hoistedLocals: [], hoistedFunctions: []}); this.genVarCounter++; } @@ -3335,10 +3356,14 @@ export class LuaTransformer { initializer?: tstl.Expression, tsOriginal?: ts.Node, parent?: tstl.Node - ): tstl.AssignmentStatement + ): tstl.AssignmentStatement | tstl.VariableDeclarationStatement { - this.hoistLocals(variable); - return tstl.createAssignmentStatement(variable, initializer, tsOriginal, parent); + if (this.shouldHoistIdentifier(variable, false)) { + this.hoistLocals(variable); + return tstl.createAssignmentStatement(variable, initializer, tsOriginal, parent); + } else { + return tstl.createVariableDeclarationStatement(variable, initializer, tsOriginal, parent); + } } private statementVisitResultToStatementArray(visitResult: StatementVisitResult): tstl.Statement[] { diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 89e57f575..12e1dba3f 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -541,4 +541,31 @@ export class TSHelper { return false; } + + public static findFirstReferenceInScope( + identifier: ts.Identifier, + scope: ts.Node, + checker: ts.TypeChecker + ): ts.Identifier | undefined + { + const symbol = checker.getSymbolAtLocation(identifier); + let result: ts.Identifier | undefined; + let ctx: ts.TransformationContext; + const visitor: ts.Visitor = node => { + if (ts.isIdentifier(node) + && node.text === identifier.text + && checker.getSymbolAtLocation(node) === symbol) + { + result = result || node; + return node; + } + return ts.visitEachChild(node, visitor, ctx); + }; + const factory: ts.TransformerFactory = c => { + ctx = c; + return n => ts.visitNode(n, visitor); + }; + ts.transform(scope, [factory]); + return result; + } } From b4157b318ac86d9e6d3ce479a327429a8e888e0f Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Thu, 31 Jan 2019 06:30:24 -0700 Subject: [PATCH 12/24] addressed comments --- src/LuaTransformer.ts | 61 +++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 50c94df44..5c084cb93 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -82,7 +82,8 @@ export class LuaTransformer { this.isModule = tsHelper.isFileModule(node); const statements = this.transformStatements(node.statements); - this.popScopeAndPrependHoistedStatements(statements); + const [, hoistedStatements] = this.popScope(); + statements.unshift(...hoistedStatements); if (this.isModule) { statements.unshift( @@ -111,7 +112,7 @@ export class LuaTransformer { switch (node.kind) { // Block case ts.SyntaxKind.Block: - return this.transformScopeBlock(node as ts.Block); + return this.transformBlockAsDoStatement(node as ts.Block); // Declaration Statements case ts.SyntaxKind.ImportDeclaration: return this.transformImportDeclaration(node as ts.ImportDeclaration); @@ -174,21 +175,19 @@ export class LuaTransformer { return tstlStatements; } - public transformBlock(block: ts.Block, pushScope?: boolean): tstl.Block { - if (pushScope) { - this.pushScope(ScopeType.Block); - } + public transformBlock(block: ts.Block): tstl.Block { + this.pushScope(ScopeType.Block); const statements = this.transformStatements(block.statements); - if (pushScope) { - this.popScopeAndPrependHoistedStatements(statements); - } + const [, hoistedStatements] = this.popScope(); + statements.unshift(...hoistedStatements); return tstl.createBlock(statements, block); } - public transformScopeBlock(block: ts.Block): tstl.DoStatement { + public transformBlockAsDoStatement(block: ts.Block): tstl.DoStatement { this.pushScope(ScopeType.Block); const statements = this.transformStatements(block.statements); - this.popScopeAndPrependHoistedStatements(statements); + const [, hoistedStatements] = this.popScope(); + statements.unshift(...hoistedStatements); return tstl.createDoStatement(statements, block); } @@ -762,7 +761,8 @@ export class LuaTransformer { const bodyStatements = this.transformStatements(body.statements); - this.popScopeAndPrependHoistedStatements(bodyStatements); + const [, hoistedStatements] = this.popScope(); + bodyStatements.unshift(...hoistedStatements); return headerStatements.concat(bodyStatements); } @@ -857,7 +857,8 @@ export class LuaTransformer { if (statement.body && ts.isModuleBlock(statement.body)) { this.pushScope(ScopeType.Block); const statements = this.transformStatements(statement.body.statements); - this.popScopeAndPrependHoistedStatements(statements); + const [, hoistedStatements] = this.popScope(); + statements.unshift(...hoistedStatements); result.push(tstl.createDoStatement(statements)); } @@ -1141,7 +1142,8 @@ export class LuaTransformer { this.pushScope(ScopeType.Conditional); const condition = this.transformExpression(statement.expression); const statements = this.transformBlockOrStatement(statement.thenStatement); - this.popScopeAndPrependHoistedStatements(statements); + const [, hoistedStatements] = this.popScope(); + statements.unshift(...hoistedStatements); const ifBlock = tstl.createBlock(statements); if (statement.elseStatement) { if (ts.isIfStatement(statement.elseStatement)) { @@ -1149,7 +1151,8 @@ export class LuaTransformer { } else { this.pushScope(ScopeType.Conditional); const elseStatements = this.transformBlockOrStatement(statement.elseStatement); - this.popScopeAndPrependHoistedStatements(elseStatements); + const [, hoistedStatements] = this.popScope(); + elseStatements.unshift(...hoistedStatements); const elseBlock = tstl.createBlock(elseStatements); return tstl.createIfStatement(condition, ifBlock, elseBlock); } @@ -1209,7 +1212,8 @@ export class LuaTransformer { // Declaration of new variable this.pushScope(ScopeType.Loop); // Counter hoisting - there's probably a better way to handle this const variableDeclarations = this.transformVariableDeclaration(initializer.declarations[0]); - this.popScopeAndPrependHoistedStatements(variableDeclarations); + const [, hoistedStatements] = this.popScope(); + variableDeclarations.unshift(...hoistedStatements); if (ts.isArrayBindingPattern(initializer.declarations[0].name)) { expression = this.createUnpackCall(expression, initializer); } @@ -1237,7 +1241,9 @@ export class LuaTransformer { { this.pushScope(ScopeType.Loop); const body = this.transformBlockOrStatement(loop.statement); - const scopeId = this.popScopeAndPrependHoistedStatements(body).id; + const [scope, hoistedStatements] = this.popScope(); + body.unshift(...hoistedStatements); + const scopeId = scope.id; if (this.options.luaTarget === LuaTarget.Lua51) { return body; @@ -1483,7 +1489,8 @@ export class LuaTransformer { statements.push(tstl.createLabelStatement(`${switchName}_end`)); - this.popScopeAndPrependHoistedStatements(statements); + const [, hoistedStatements] = this.popScope(); + statements.unshift(...hoistedStatements); return statements; } @@ -1499,7 +1506,7 @@ export class LuaTransformer { public transformTryStatement(statement: ts.TryStatement): StatementVisitResult { const pCall = tstl.createIdentifier("pcall"); - const tryBlock = this.transformBlock(statement.tryBlock, true); + const tryBlock = this.transformBlock(statement.tryBlock); const tryCall = tstl.createCallExpression(pCall, [tstl.createFunctionExpression(tryBlock)]); const result: tstl.Statement[] = []; @@ -1516,14 +1523,14 @@ export class LuaTransformer { result.push(catchAssignment); const notTryResult = tstl.createUnaryExpression(tryResult, tstl.SyntaxKind.NotOperator); - result.push(tstl.createIfStatement(notTryResult, this.transformBlock(statement.catchClause.block, true))); + result.push(tstl.createIfStatement(notTryResult, this.transformBlock(statement.catchClause.block))); } else { result.push(tstl.createExpressionStatement(tryCall)); } if (statement.finallyBlock) { - result.push(tstl.createDoStatement(this.transformBlock(statement.finallyBlock, true).statements)); + result.push(tstl.createDoStatement(this.transformBlock(statement.finallyBlock).statements)); } return tstl.createDoStatement( @@ -3310,18 +3317,20 @@ export class LuaTransformer { this.genVarCounter++; } - protected popScopeAndPrependHoistedStatements(statements: tstl.Statement[]): Scope { + protected popScope(): [Scope, tstl.Statement[]] { const scope = this.scopeStack.pop(); - // hoisted function declarations - statements.unshift(...scope.hoistedFunctions); + const hoistedStatements: tstl.Statement[] = []; // hoisted locals if (scope.hoistedLocals.length > 0) { - statements.unshift(tstl.createVariableDeclarationStatement(scope.hoistedLocals)); + hoistedStatements.push(tstl.createVariableDeclarationStatement(scope.hoistedLocals)); } - return scope; + // hoisted function declarations + hoistedStatements.push(...scope.hoistedFunctions); + + return [scope, hoistedStatements]; } protected hoistLocals(locals: tstl.Identifier | tstl.Identifier[], scopeType?: ScopeType): void { From 27bb3b5d671fed3daab2555b3585767c1ae01a50 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Thu, 31 Jan 2019 09:06:03 -0700 Subject: [PATCH 13/24] smart-hoisting working --- src/LuaTransformer.ts | 27 ++++++++++++++------------- src/TSHelper.ts | 38 +++++++++++++++++++++----------------- 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 2efcf4961..6e6fe1971 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1211,7 +1211,7 @@ export class LuaTransformer { public transformForOfInitializer(initializer: ts.ForInitializer, expression: tstl.Expression): tstl.Statement { if (ts.isVariableDeclarationList(initializer)) { // Declaration of new variable - this.pushScope(ScopeType.Loop, initializer); // Counter hoisting - there's probably a better way to handle this + this.pushScope(ScopeType.Loop, initializer); // Counter hoisting - probably a better way to handle this const variableDeclarations = this.transformVariableDeclaration(initializer.declarations[0]); const [, hoistedStatements] = this.popScope(); variableDeclarations.unshift(...hoistedStatements); @@ -3178,17 +3178,11 @@ export class LuaTransformer { } private shouldHoistIdentifier(identifier: tstl.Identifier, isVar: boolean): boolean { - if (!identifier.tsOriginal || identifier.tsOriginal.pos < 0) { + if (!identifier.tsOriginal || !ts.isIdentifier(identifier.tsOriginal) || identifier.tsOriginal.pos < 0) { return false; } - - const scope = isVar ? this.findScope(ScopeType.Function) : this.peekScope(); - const firstReference = tsHelper.findFirstReferenceInScope( - identifier.tsOriginal as ts.Identifier, - scope.node, - this.checker - ); - return firstReference !== undefined && firstReference.pos < identifier.tsOriginal.pos; + const scope = isVar ? (this.findScope(ScopeType.Function) || this.scopeStack[0]) : this.peekScope(); + return tsHelper.needsHoisting(identifier.tsOriginal as ts.Identifier, scope.node, this.checker); } private createLocalOrExportedOrGlobalDeclaration( @@ -3214,19 +3208,26 @@ export class LuaTransformer { const isLetOrConst = tsOriginal && ts.isVariableDeclaration(tsOriginal) && tsOriginal.parent && (tsOriginal.parent.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) !== 0; if (this.isModule || this.currentNamespace || insideFunction || isLetOrConst) { + // local const isVar = !(isLetOrConst || isFunctionDeclaration); - const hoist = isFunctionDeclaration || (Array.isArray(lhs) + const hoist = Array.isArray(lhs) ? lhs.some(i => this.shouldHoistIdentifier(i, !isLetOrConst)) - : this.shouldHoistIdentifier(lhs, isVar)); + : this.shouldHoistIdentifier(lhs, isVar); if (hoist) { this.hoistLocals(lhs, isLetOrConst || isFunctionDeclaration ? undefined : ScopeType.Function); + } else if (rhs && tstl.isFunctionExpression(rhs)) { + // separate declaration and assignment to support recursive functions + return [ + tstl.createVariableDeclarationStatement(lhs, undefined, tsOriginal, parent), + tstl.createAssignmentStatement(left, rhs, tsOriginal, parent), + ]; } else { return [tstl.createVariableDeclarationStatement(lhs, rhs, tsOriginal, parent)]; } } } - if (isFunctionDeclaration) { + if (isFunctionDeclaration && this.shouldHoistIdentifier(lhs as tstl.Identifier, false)) { // hoist function declarations this.hoistFunctionDeclaration(tstl.createAssignmentStatement(left, rhs, tsOriginal, parent)); return undefined; diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 12e1dba3f..05bfa7379 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -542,30 +542,34 @@ export class TSHelper { return false; } - public static findFirstReferenceInScope( - identifier: ts.Identifier, - scope: ts.Node, - checker: ts.TypeChecker - ): ts.Identifier | undefined + public static needsHoisting(identifier: ts.Identifier, scope: ts.Node, checker: ts.TypeChecker) : boolean { const symbol = checker.getSymbolAtLocation(identifier); - let result: ts.Identifier | undefined; - let ctx: ts.TransformationContext; - const visitor: ts.Visitor = node => { - if (ts.isIdentifier(node) + let result = false; + const functionStack: ts.FunctionDeclaration[] = []; + const visitor = (node: ts.Node) => { + if (ts.isFunctionDeclaration(node)) { + if (node.name !== identifier) { // don't recurse into self + functionStack.push(node); + ts.forEachChild(node, visitor); + functionStack.pop(); + } + return; + + } else if (ts.isIdentifier(node) && node.text === identifier.text && checker.getSymbolAtLocation(node) === symbol) { - result = result || node; - return node; + // hoist if referenced before declaration, or inside a local function which itself will be hoisted + result = (node.pos < identifier.pos) + || (functionStack.length > 0 && this.needsHoisting(functionStack[0].name, scope, checker)); + } + + if (!result) { + ts.forEachChild(node, visitor); } - return ts.visitEachChild(node, visitor, ctx); - }; - const factory: ts.TransformerFactory = c => { - ctx = c; - return n => ts.visitNode(n, visitor); }; - ts.transform(scope, [factory]); + ts.forEachChild(scope, visitor); return result; } } From 58b5237426fb2e0bfd127904f79459e46fc833af Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Thu, 31 Jan 2019 12:00:32 -0700 Subject: [PATCH 14/24] fixed tests --- .../lua/characterEscapeSequence.lua | 19 ++++++------- test/translation/lua/continue.lua | 3 +- test/translation/lua/continueConcurrent.lua | 3 +- test/translation/lua/continueNested.lua | 6 ++-- .../lua/continueNestedConcurrent.lua | 6 ++-- test/translation/lua/do.lua | 3 +- test/translation/lua/enumMembersOnly.lua | 3 +- test/translation/lua/for.lua | 3 +- test/translation/lua/getSetAccessors.lua | 7 ++--- test/translation/lua/interfaceIndex.lua | 3 +- test/translation/lua/modulesImportAll.lua | 3 +- test/translation/lua/modulesImportNamed.lua | 3 +- test/translation/lua/modulesImportRenamed.lua | 3 +- .../lua/modulesNamespaceExport.lua | 3 +- .../lua/modulesNamespaceExportEnum.lua | 3 +- ...modulesNamespaceNestedWithMemberExport.lua | 6 ++-- .../lua/modulesNamespaceWithMemberExport.lua | 3 +- .../modulesNamespaceWithMemberNoExport.lua | 3 +- .../lua/modulesVariableNoExport.lua | 3 +- test/translation/lua/namespaceMerge.lua | 3 +- test/translation/lua/namespaceNested.lua | 3 +- test/translation/lua/tryCatch.lua | 6 ++-- test/translation/lua/tryCatchFinally.lua | 9 ++---- test/translation/lua/tryFinally.lua | 6 ++-- test/translation/lua/tupleReturn.lua | 28 +++++++++---------- test/translation/lua/typeAssert.lua | 5 ++-- test/translation/lua/while.lua | 3 +- test/unit/assignmentDestructuring.spec.ts | 6 ++-- test/unit/assignments.spec.ts | 14 +++++----- .../compiler/configuration/options.spec.ts | 2 +- test/unit/enum.spec.ts | 8 +++--- test/unit/expressions.spec.ts | 4 +-- test/unit/objectLiteral.spec.ts | 2 +- 33 files changed, 75 insertions(+), 110 deletions(-) diff --git a/test/translation/lua/characterEscapeSequence.lua b/test/translation/lua/characterEscapeSequence.lua index 1179699bc..36abfe9d0 100644 --- a/test/translation/lua/characterEscapeSequence.lua +++ b/test/translation/lua/characterEscapeSequence.lua @@ -1,10 +1,9 @@ -local quoteInDoubleQuotes, quoteInTemplateString, doubleQuoteInQuotes, doubleQuoteInDoubleQuotes, doubleQuoteInTemplateString, escapedCharsInQuotes, escapedCharsInDoubleQUotes, escapedCharsInTemplateString, nonEmptyTemplateString; -quoteInDoubleQuotes = "\' \' \'"; -quoteInTemplateString = "\' \' \'"; -doubleQuoteInQuotes = "\" \" \""; -doubleQuoteInDoubleQuotes = "\" \" \""; -doubleQuoteInTemplateString = "\" \" \""; -escapedCharsInQuotes = "\\ \0 \b \t \n \v \f \" \' \`"; -escapedCharsInDoubleQUotes = "\\ \0 \b \t \n \v \f \" \' \`"; -escapedCharsInTemplateString = "\\ \0 \b \t \n \v \f \" \' \`"; -nonEmptyTemplateString = "Level 0: \n\t " .. (tostring("Level 1: \n\t\t " .. (tostring("Level 3: \n\t\t\t " .. (tostring("Last level \n --") .. " \n --")) .. " \n --")) .. " \n --"); +local quoteInDoubleQuotes = "\' \' \'"; +local quoteInTemplateString = "\' \' \'"; +local doubleQuoteInQuotes = "\" \" \""; +local doubleQuoteInDoubleQuotes = "\" \" \""; +local doubleQuoteInTemplateString = "\" \" \""; +local escapedCharsInQuotes = "\\ \0 \b \t \n \v \f \" \' \`"; +local escapedCharsInDoubleQUotes = "\\ \0 \b \t \n \v \f \" \' \`"; +local escapedCharsInTemplateString = "\\ \0 \b \t \n \v \f \" \' \`"; +local nonEmptyTemplateString = "Level 0: \n\t " .. (tostring("Level 1: \n\t\t " .. (tostring("Level 3: \n\t\t\t " .. (tostring("Last level \n --") .. " \n --")) .. " \n --")) .. " \n --"); diff --git a/test/translation/lua/continue.lua b/test/translation/lua/continue.lua index a4a615636..58127ab1b 100644 --- a/test/translation/lua/continue.lua +++ b/test/translation/lua/continue.lua @@ -1,5 +1,4 @@ -local i; -i = 0; +local i = 0; while i < 10 do do if i < 5 then diff --git a/test/translation/lua/continueConcurrent.lua b/test/translation/lua/continueConcurrent.lua index 0deb036e0..f689b22bc 100644 --- a/test/translation/lua/continueConcurrent.lua +++ b/test/translation/lua/continueConcurrent.lua @@ -1,5 +1,4 @@ -local i; -i = 0; +local i = 0; while i < 10 do do if i < 5 then diff --git a/test/translation/lua/continueNested.lua b/test/translation/lua/continueNested.lua index 137211d85..39e5f7307 100644 --- a/test/translation/lua/continueNested.lua +++ b/test/translation/lua/continueNested.lua @@ -1,12 +1,10 @@ -local i; -i = 0; +local i = 0; while i < 5 do do - local j; if (i % 2) == 0 then goto __continue1; end - j = 0; + local j = 0; while j < 2 do do if j == 1 then diff --git a/test/translation/lua/continueNestedConcurrent.lua b/test/translation/lua/continueNestedConcurrent.lua index 7083b5a6e..614cb2e0b 100644 --- a/test/translation/lua/continueNestedConcurrent.lua +++ b/test/translation/lua/continueNestedConcurrent.lua @@ -1,12 +1,10 @@ -local i; -i = 0; +local i = 0; while i < 5 do do - local j; if (i % 2) == 0 then goto __continue1; end - j = 0; + local j = 0; while j < 2 do do if j == 1 then diff --git a/test/translation/lua/do.lua b/test/translation/lua/do.lua index 6c2be3b3b..14639278d 100644 --- a/test/translation/lua/do.lua +++ b/test/translation/lua/do.lua @@ -1,5 +1,4 @@ -local e; -e = 10; +local e = 10; repeat do e = e - 1; diff --git a/test/translation/lua/enumMembersOnly.lua b/test/translation/lua/enumMembersOnly.lua index 126867ab9..06bdb5f70 100644 --- a/test/translation/lua/enumMembersOnly.lua +++ b/test/translation/lua/enumMembersOnly.lua @@ -1,6 +1,5 @@ -local a; val1 = 0; val2 = 2; val3 = 3; val4 = "bye"; -a = val1; +local a = val1; diff --git a/test/translation/lua/for.lua b/test/translation/lua/for.lua index e5b8dcee8..b81e83929 100644 --- a/test/translation/lua/for.lua +++ b/test/translation/lua/for.lua @@ -1,5 +1,4 @@ -local i; -i = 1; +local i = 1; while i <= 100 do do end diff --git a/test/translation/lua/getSetAccessors.lua b/test/translation/lua/getSetAccessors.lua index a7b08df80..9fe62bbb6 100644 --- a/test/translation/lua/getSetAccessors.lua +++ b/test/translation/lua/getSetAccessors.lua @@ -1,4 +1,3 @@ -local instance, b, c; MyClass = MyClass or {}; MyClass.__index = MyClass; MyClass.new = function(construct, ...) @@ -16,7 +15,7 @@ end; MyClass.set__field = function(self, v) self._field = v * 2; end; -instance = MyClass.new(true); +local instance = MyClass.new(true); instance:set__field(4); -b = instance:get__field(); -c = (4 + instance:get__field()) * 3; +local b = instance:get__field(); +local c = (4 + instance:get__field()) * 3; diff --git a/test/translation/lua/interfaceIndex.lua b/test/translation/lua/interfaceIndex.lua index fd4bf2dd5..a29d6d09d 100644 --- a/test/translation/lua/interfaceIndex.lua +++ b/test/translation/lua/interfaceIndex.lua @@ -1,3 +1,2 @@ -local a; -a = {}; +local a = {}; a.abc = "def"; diff --git a/test/translation/lua/modulesImportAll.lua b/test/translation/lua/modulesImportAll.lua index 35e6b9e8d..334fc07a7 100644 --- a/test/translation/lua/modulesImportAll.lua +++ b/test/translation/lua/modulesImportAll.lua @@ -1,2 +1 @@ -local Test; -Test = require("test"); +local Test = require("test"); diff --git a/test/translation/lua/modulesImportNamed.lua b/test/translation/lua/modulesImportNamed.lua index 732452bf7..27cbae8c9 100644 --- a/test/translation/lua/modulesImportNamed.lua +++ b/test/translation/lua/modulesImportNamed.lua @@ -1,3 +1,2 @@ -local TestClass; local __TSTL_test = require("test"); -TestClass = __TSTL_test.TestClass; +local TestClass = __TSTL_test.TestClass; diff --git a/test/translation/lua/modulesImportRenamed.lua b/test/translation/lua/modulesImportRenamed.lua index 40fec9c5c..84c484b5d 100644 --- a/test/translation/lua/modulesImportRenamed.lua +++ b/test/translation/lua/modulesImportRenamed.lua @@ -1,3 +1,2 @@ -local RenamedClass; local __TSTL_test = require("test"); -RenamedClass = __TSTL_test.TestClass; +local RenamedClass = __TSTL_test.TestClass; diff --git a/test/translation/lua/modulesNamespaceExport.lua b/test/translation/lua/modulesNamespaceExport.lua index 2fcdd8d53..122f64cf0 100644 --- a/test/translation/lua/modulesNamespaceExport.lua +++ b/test/translation/lua/modulesNamespaceExport.lua @@ -1,7 +1,6 @@ local exports = exports or {}; -local TestSpace; exports.TestSpace = exports.TestSpace or {}; -TestSpace = exports.TestSpace; +local TestSpace = exports.TestSpace; do end return exports; diff --git a/test/translation/lua/modulesNamespaceExportEnum.lua b/test/translation/lua/modulesNamespaceExportEnum.lua index a0056e377..938fdd56b 100644 --- a/test/translation/lua/modulesNamespaceExportEnum.lua +++ b/test/translation/lua/modulesNamespaceExportEnum.lua @@ -1,7 +1,6 @@ local exports = exports or {}; -local test; exports.test = exports.test or {}; -test = exports.test; +local test = exports.test; do test.TestEnum = {}; test.TestEnum.foo = "foo"; diff --git a/test/translation/lua/modulesNamespaceNestedWithMemberExport.lua b/test/translation/lua/modulesNamespaceNestedWithMemberExport.lua index 902d719bd..44509f50d 100644 --- a/test/translation/lua/modulesNamespaceNestedWithMemberExport.lua +++ b/test/translation/lua/modulesNamespaceNestedWithMemberExport.lua @@ -1,11 +1,9 @@ local exports = exports or {}; -local TestSpace; exports.TestSpace = exports.TestSpace or {}; -TestSpace = exports.TestSpace; +local TestSpace = exports.TestSpace; do - local TestNestedSpace; TestSpace.TestNestedSpace = TestSpace.TestNestedSpace or {}; - TestNestedSpace = TestSpace.TestNestedSpace; + local TestNestedSpace = TestSpace.TestNestedSpace; do TestNestedSpace.innerFunc = function() end; diff --git a/test/translation/lua/modulesNamespaceWithMemberExport.lua b/test/translation/lua/modulesNamespaceWithMemberExport.lua index d6192ed57..020bcf4d5 100644 --- a/test/translation/lua/modulesNamespaceWithMemberExport.lua +++ b/test/translation/lua/modulesNamespaceWithMemberExport.lua @@ -1,7 +1,6 @@ local exports = exports or {}; -local TestSpace; exports.TestSpace = exports.TestSpace or {}; -TestSpace = exports.TestSpace; +local TestSpace = exports.TestSpace; do TestSpace.innerFunc = function() end; diff --git a/test/translation/lua/modulesNamespaceWithMemberNoExport.lua b/test/translation/lua/modulesNamespaceWithMemberNoExport.lua index 1ee06a28b..b0abeb433 100644 --- a/test/translation/lua/modulesNamespaceWithMemberNoExport.lua +++ b/test/translation/lua/modulesNamespaceWithMemberNoExport.lua @@ -1,7 +1,6 @@ local exports = exports or {}; -local TestSpace; exports.TestSpace = exports.TestSpace or {}; -TestSpace = exports.TestSpace; +local TestSpace = exports.TestSpace; do local innerFunc; innerFunc = function() diff --git a/test/translation/lua/modulesVariableNoExport.lua b/test/translation/lua/modulesVariableNoExport.lua index 537c5219e..f20c4efeb 100644 --- a/test/translation/lua/modulesVariableNoExport.lua +++ b/test/translation/lua/modulesVariableNoExport.lua @@ -1,2 +1 @@ -local test; -test = "test"; +local test = "test"; diff --git a/test/translation/lua/namespaceMerge.lua b/test/translation/lua/namespaceMerge.lua index 0bdf40c4f..e585c3c83 100644 --- a/test/translation/lua/namespaceMerge.lua +++ b/test/translation/lua/namespaceMerge.lua @@ -1,4 +1,3 @@ -local mergedClass; MergedClass = MergedClass or {}; MergedClass.__index = MergedClass; MergedClass.new = function(construct, ...) @@ -28,7 +27,7 @@ do MergedClass.namespaceFunc = function() end; end -mergedClass = MergedClass.new(true); +local mergedClass = MergedClass.new(true); mergedClass:methodB(); mergedClass:propertyFunc(); MergedClass:staticMethodB(); diff --git a/test/translation/lua/namespaceNested.lua b/test/translation/lua/namespaceNested.lua index f61e0373f..dd1006ee9 100644 --- a/test/translation/lua/namespaceNested.lua +++ b/test/translation/lua/namespaceNested.lua @@ -1,8 +1,7 @@ myNamespace = myNamespace or {}; do - local myNestedNamespace; myNamespace.myNestedNamespace = myNamespace.myNestedNamespace or {}; - myNestedNamespace = myNamespace.myNestedNamespace; + local myNestedNamespace = myNamespace.myNestedNamespace; do local nsMember; nsMember = function() diff --git a/test/translation/lua/tryCatch.lua b/test/translation/lua/tryCatch.lua index 92250d99f..f3194765e 100644 --- a/test/translation/lua/tryCatch.lua +++ b/test/translation/lua/tryCatch.lua @@ -1,10 +1,8 @@ do local ____TS_try, er = pcall(function() - local a; - a = 42; + local a = 42; end); if not ____TS_try then - local b; - b = "fail"; + local b = "fail"; end end diff --git a/test/translation/lua/tryCatchFinally.lua b/test/translation/lua/tryCatchFinally.lua index 65578d6ee..4b5caf9a6 100644 --- a/test/translation/lua/tryCatchFinally.lua +++ b/test/translation/lua/tryCatchFinally.lua @@ -1,14 +1,11 @@ do local ____TS_try, er = pcall(function() - local a; - a = 42; + local a = 42; end); if not ____TS_try then - local b; - b = "fail"; + local b = "fail"; end do - local c; - c = "finally"; + local c = "finally"; end end diff --git a/test/translation/lua/tryFinally.lua b/test/translation/lua/tryFinally.lua index e23df9fee..a117b8bb2 100644 --- a/test/translation/lua/tryFinally.lua +++ b/test/translation/lua/tryFinally.lua @@ -1,10 +1,8 @@ do pcall(function() - local a; - a = 42; + local a = 42; end); do - local b; - b = "finally"; + local b = "finally"; end end diff --git a/test/translation/lua/tupleReturn.lua b/test/translation/lua/tupleReturn.lua index 077d7edb2..c8d492bec 100644 --- a/test/translation/lua/tupleReturn.lua +++ b/test/translation/lua/tupleReturn.lua @@ -1,10 +1,20 @@ -local a, b, c, d, e, f; tupleReturn = function() return 0, "foobar"; end; +tupleReturn(); +noTupleReturn(); +local a, b = tupleReturn(); +local c, d = table.unpack(noTupleReturn()); +a, b = tupleReturn(); +c, d = table.unpack(noTupleReturn()); +local e = ({tupleReturn()}); +local f = noTupleReturn(); +e = ({tupleReturn()}); +f = noTupleReturn(); +foo(({tupleReturn()})); +foo(noTupleReturn()); tupleReturnFromVar = function() - local r; - r = {1, "baz"}; + local r = {1, "baz"}; return table.unpack(r); end; tupleReturnForward = function() @@ -16,15 +26,3 @@ end; tupleReturnUnpack = function() return table.unpack(tupleNoForward()); end; -tupleReturn(); -noTupleReturn(); -a, b = tupleReturn(); -c, d = table.unpack(noTupleReturn()); -a, b = tupleReturn(); -c, d = table.unpack(noTupleReturn()); -e = ({tupleReturn()}); -f = noTupleReturn(); -e = ({tupleReturn()}); -f = noTupleReturn(); -foo(({tupleReturn()})); -foo(noTupleReturn()); diff --git a/test/translation/lua/typeAssert.lua b/test/translation/lua/typeAssert.lua index 8c9aa72e6..1c7069d36 100644 --- a/test/translation/lua/typeAssert.lua +++ b/test/translation/lua/typeAssert.lua @@ -1,3 +1,2 @@ -local test1, test2; -test1 = 10; -test2 = 10; +local test1 = 10; +local test2 = 10; diff --git a/test/translation/lua/while.lua b/test/translation/lua/while.lua index 763fcf570..a779e9ded 100644 --- a/test/translation/lua/while.lua +++ b/test/translation/lua/while.lua @@ -1,5 +1,4 @@ -local d; -d = 10; +local d = 10; while d > 0 do do d = d - 1; diff --git a/test/unit/assignmentDestructuring.spec.ts b/test/unit/assignmentDestructuring.spec.ts index 2a1a40a97..7f8e10507 100644 --- a/test/unit/assignmentDestructuring.spec.ts +++ b/test/unit/assignmentDestructuring.spec.ts @@ -15,7 +15,7 @@ export class AssignmentDestructuringTests { this.assignmentDestruturingTs, {luaTarget: LuaTarget.Lua51, luaLibImport: LuaLibImportKind.None} ); // Assert - Expect(lua).toBe(`local a, b;\na, b = unpack(myFunc());`); + Expect(lua).toBe(`local a, b = unpack(myFunc());`); } @Test("Assignment destructuring [5.2]") @@ -25,7 +25,7 @@ export class AssignmentDestructuringTests { this.assignmentDestruturingTs, {luaTarget: LuaTarget.Lua52, luaLibImport: LuaLibImportKind.None} ); // Assert - Expect(lua).toBe(`local a, b;\na, b = table.unpack(myFunc());`); + Expect(lua).toBe(`local a, b = table.unpack(myFunc());`); } @Test("Assignment destructuring [JIT]") @@ -35,6 +35,6 @@ export class AssignmentDestructuringTests { this.assignmentDestruturingTs, {luaTarget: LuaTarget.LuaJIT, luaLibImport: LuaLibImportKind.None} ); // Assert - Expect(lua).toBe(`local a, b;\na, b = unpack(myFunc());`); + Expect(lua).toBe(`local a, b = unpack(myFunc());`); } } diff --git a/test/unit/assignments.spec.ts b/test/unit/assignments.spec.ts index a5b13d7ba..94034ab25 100644 --- a/test/unit/assignments.spec.ts +++ b/test/unit/assignments.spec.ts @@ -32,7 +32,7 @@ export class AssignmentTests { @Test("Const assignment") public constAssignment(inp: string, out: string): void { const lua = util.transpileString(`const myvar = ${inp};`); - Expect(lua).toBe(`local myvar;\nmyvar = ${out};`); + Expect(lua).toBe(`local myvar = ${out};`); } @TestCase(`"abc"`, `"abc"`) @@ -44,7 +44,7 @@ export class AssignmentTests { @Test("Let assignment") public letAssignment(inp: string, out: string): void { const lua = util.transpileString(`let myvar = ${inp};`); - Expect(lua).toBe(`local myvar;\nmyvar = ${out};`); + Expect(lua).toBe(`local myvar = ${out};`); } @TestCase(`"abc"`, `"abc"`) @@ -105,7 +105,7 @@ export class AssignmentTests { + `let [a,b] = abc();`; const lua = util.transpileString(code); - Expect(lua).toBe("local a, b;\na, b = abc();"); + Expect(lua).toBe("local a, b = abc();"); } @Test("TupleReturn Single assignment") @@ -116,7 +116,7 @@ export class AssignmentTests { + `a = abc();`; const lua = util.transpileString(code); - Expect(lua).toBe("local a;\na = ({abc()});\na = ({abc()});"); + Expect(lua).toBe("local a = ({abc()});\na = ({abc()});"); } @Test("TupleReturn interface assignment") @@ -128,7 +128,7 @@ export class AssignmentTests { + `let [a,b] = jkl.abc();`; const lua = util.transpileString(code); - Expect(lua).toBe("local a, b;\na, b = jkl:abc();"); + Expect(lua).toBe("local a, b = jkl:abc();"); } @Test("TupleReturn namespace assignment") @@ -140,7 +140,7 @@ export class AssignmentTests { + `let [a,b] = def.abc();`; const lua = util.transpileString(code); - Expect(lua).toBe("local a, b;\na, b = def.abc();"); + Expect(lua).toBe("local a, b = def.abc();"); } @Test("TupleReturn method assignment") @@ -152,7 +152,7 @@ export class AssignmentTests { + `let [a,b] = jkl.abc();`; const lua = util.transpileString(code); - Expect(lua).toBe("local jkl, a, b;\njkl = def.new(true);\na, b = jkl:abc();"); + Expect(lua).toBe("local jkl = def.new(true);\nlocal a, b = jkl:abc();"); } @Test("TupleReturn functional") diff --git a/test/unit/compiler/configuration/options.spec.ts b/test/unit/compiler/configuration/options.spec.ts index 64b6476ab..262b80085 100644 --- a/test/unit/compiler/configuration/options.spec.ts +++ b/test/unit/compiler/configuration/options.spec.ts @@ -24,6 +24,6 @@ export class ObjectLiteralTests const options = {LuaLibImportKind: importKind}; const result = util.transpileString("const a = new Map();", options); - Expect(result).toBe("local a;\na = Map.new(true);"); + Expect(result).toBe("local a = Map.new(true);"); } } diff --git a/test/unit/enum.spec.ts b/test/unit/enum.spec.ts index 5e771adcb..b2a2a09ce 100644 --- a/test/unit/enum.spec.ts +++ b/test/unit/enum.spec.ts @@ -15,7 +15,7 @@ export class EnumTests { const valueOne = TestEnum.MEMBER_ONE; `; - Expect(util.transpileString(testCode)).toBe(`local valueOne;\nvalueOne = "test";`); + Expect(util.transpileString(testCode)).toBe(`local valueOne = "test";`); } @Test("Const enum") @@ -29,7 +29,7 @@ export class EnumTests { const valueOne = TestEnum.MEMBER_ONE; `; - Expect(util.transpileString(testCode)).toBe(`local valueOne;\nvalueOne = "test";`); + Expect(util.transpileString(testCode)).toBe(`local valueOne = "test";`); } @Test("Const enum without initializer") @@ -43,7 +43,7 @@ export class EnumTests { const valueOne = TestEnum.MEMBER_ONE; `; - Expect(util.transpileString(testCode)).toBe(`local valueOne;\nvalueOne = 0;`); + Expect(util.transpileString(testCode)).toBe(`local valueOne = 0;`); } @Test("Const enum without initializer in some values") @@ -58,7 +58,7 @@ export class EnumTests { const valueOne = TestEnum.MEMBER_TWO; `; - Expect(util.transpileString(testCode)).toBe(`local valueOne;\nvalueOne = 4;`); + Expect(util.transpileString(testCode)).toBe(`local valueOne = 4;`); } @Test("Invalid heterogeneous enum") diff --git a/test/unit/expressions.spec.ts b/test/unit/expressions.spec.ts index ef30718ce..f504afa1e 100644 --- a/test/unit/expressions.spec.ts +++ b/test/unit/expressions.spec.ts @@ -13,9 +13,9 @@ export class ExpressionTests { @TestCase("--i", "i = i - 1;") @TestCase("!a", "not a;") @TestCase("-a", "-a;") - @TestCase("let a = delete tbl['test']", "local a;\na = (function()\n tbl.test = nil;\n return true;\nend)();") + @TestCase("let a = delete tbl['test']", "local a = (function()\n tbl.test = nil;\n return true;\nend)();") @TestCase("delete tbl['test']", "tbl.test = nil;") - @TestCase("let a = delete tbl.test", "local a;\na = (function()\n tbl.test = nil;\n return true;\nend)();") + @TestCase("let a = delete tbl.test", "local a = (function()\n tbl.test = nil;\n return true;\nend)();") @TestCase("delete tbl.test", "tbl.test = nil;") @Test("Unary expressions basic") public unaryBasic(input: string, lua: string): void { diff --git a/test/unit/objectLiteral.spec.ts b/test/unit/objectLiteral.spec.ts index bc4930d39..dd94ff419 100644 --- a/test/unit/objectLiteral.spec.ts +++ b/test/unit/objectLiteral.spec.ts @@ -14,7 +14,7 @@ export class ObjectLiteralTests { @Test("Object Literal") public objectLiteral(inp: string, out: string): void { const lua = util.transpileString(`const myvar = ${inp};`); - Expect(lua).toBe(`local myvar;\nmyvar = ${out}`); + Expect(lua).toBe(`local myvar = ${out}`); } @TestCase("3", 3) From 6fabe1422d8b9ed61b348477cdb630b923417ee4 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Fri, 1 Feb 2019 07:47:17 -0700 Subject: [PATCH 15/24] refactored hoisting detection and added command line options to control hoisting --- src/CommandLineParser.ts | 7 ++ src/Compiler.ts | 4 +- src/CompilerOptions.ts | 7 ++ src/LuaTransformer.ts | 76 +++++++++++++++++-- src/LuaTranspiler.ts | 7 +- src/TSHelper.ts | 31 +++----- src/tstl.ts | 2 +- .../configuration/mixed/index.spec.ts | 5 +- 8 files changed, 106 insertions(+), 33 deletions(-) diff --git a/src/CommandLineParser.ts b/src/CommandLineParser.ts index 989abeb14..eff2d6eda 100644 --- a/src/CommandLineParser.ts +++ b/src/CommandLineParser.ts @@ -32,6 +32,13 @@ export const optionDeclarations: YargsOptions = { describe: "Specify if a header will be added to compiled files.", type: "boolean", }, + hoisting: { + alias: "h", + default: "none", + choices: ["none", "full", "required"], + describe: "Specifies how variable hoisting is handled.", + type: "string", + }, }; class CLIError extends Error {} diff --git a/src/Compiler.ts b/src/Compiler.ts index 112bce125..50f3bfb31 100644 --- a/src/Compiler.ts +++ b/src/Compiler.ts @@ -3,7 +3,7 @@ import * as path from "path"; import * as ts from "typescript"; import {parseCommandLine} from "./CommandLineParser"; -import {CompilerOptions, LuaLibImportKind, LuaTarget} from "./CompilerOptions"; +import {CompilerOptions, HoistingMode, LuaLibImportKind, LuaTarget} from "./CompilerOptions"; import {LuaTranspiler} from "./LuaTranspiler"; export function compile(argv: string[]): void { @@ -119,4 +119,4 @@ export function transpileString( const result = transpiler.transpileSourceFile(program.getSourceFile("file.ts")); return result.trim(); -} \ No newline at end of file +} diff --git a/src/CompilerOptions.ts b/src/CompilerOptions.ts index e6e2a2aa1..68627bc4a 100644 --- a/src/CompilerOptions.ts +++ b/src/CompilerOptions.ts @@ -4,6 +4,7 @@ export interface CompilerOptions extends ts.CompilerOptions { noHeader?: boolean; luaTarget?: LuaTarget; luaLibImport?: LuaLibImportKind; + hoisting?: HoistingMode; } export enum LuaLibImportKind { @@ -19,3 +20,9 @@ export enum LuaTarget { Lua53 = "5.3", LuaJIT = "jit", } + +export enum HoistingMode { + None = "none", + Full = "full", + Required = "required", +} diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 6e6fe1971..d4cd34079 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1,7 +1,7 @@ import * as path from "path"; import * as ts from "typescript"; -import {CompilerOptions, LuaLibImportKind, LuaTarget} from "./CompilerOptions"; +import {CompilerOptions, LuaLibImportKind, LuaTarget, HoistingMode} from "./CompilerOptions"; import {DecoratorKind} from "./Decorator"; import * as tstl from "./LuaAST"; import {LuaLib, LuaLibFeature} from "./LuaLib"; @@ -49,6 +49,7 @@ export class LuaTransformer { private genVarCounter: number; private luaLibFeatureSet: Set; + private hoistingCache: Map; private readonly typeValidationCache: Map> = new Map>(); @@ -61,6 +62,9 @@ export class LuaTransformer { if (!this.options.luaTarget) { this.options.luaTarget = LuaTarget.LuaJIT; } + if (!this.options.hoisting) { + this.options.hoisting = HoistingMode.Required; + } this.setupState(); } @@ -72,6 +76,7 @@ export class LuaTransformer { this.scopeStack = []; this.classStack = []; this.luaLibFeatureSet = new Set(); + this.hoistingCache = new Map(); } // TODO make all other methods private??? @@ -3177,12 +3182,68 @@ export class LuaTransformer { return tstl.createIdentifier("self", tsOriginal); } + private identifierNeedsHoisting(identifier: ts.Identifier, scope: Scope) : boolean { + const symbol = this.checker.getSymbolAtLocation(identifier); + if (this.hoistingCache.has(symbol)) { + return this.hoistingCache.get(symbol); + } + + if (identifier.parent + && ts.isFunctionDeclaration(identifier.parent) + && scope.type === ScopeType.File + && !this.isModule) + { + // Dont' hoist global function declarations + this.hoistingCache.set(symbol, false); + return false; + } + + const visitor = (node: ts.Node) => { + if (node.pos < identifier.pos) { + // check for reference before declaration + if (ts.isIdentifier(node) + && node.text === identifier.text + && this.checker.getSymbolAtLocation(node) === symbol) + { + return true; + } + + } else if (node.pos > identifier.pos) { + // check for reference in function that will be hoisted + if (ts.isFunctionDeclaration(node)) { + if (node.body + && this.identifierNeedsHoisting(node.name, scope) + && tsHelper.findFirstReference(identifier, node.body, this.checker)) + { + return true; + } + return false; + } + } + + return ts.forEachChild(node, visitor); + }; + + const result = ts.forEachChild(scope.node, visitor) !== undefined; + this.hoistingCache.set(symbol, result); + return result; + } + private shouldHoistIdentifier(identifier: tstl.Identifier, isVar: boolean): boolean { - if (!identifier.tsOriginal || !ts.isIdentifier(identifier.tsOriginal) || identifier.tsOriginal.pos < 0) { + if (this.options.hoisting === HoistingMode.None + || !identifier.tsOriginal + || !ts.isIdentifier(identifier.tsOriginal) + || identifier.tsOriginal.pos < 0) + { return false; + + } else if (this.options.hoisting === HoistingMode.Full) { + return true; + + } else { + const scope = isVar ? (this.findScope(ScopeType.Function) || this.scopeStack[0]) : this.peekScope(); + return this.identifierNeedsHoisting(identifier.tsOriginal as ts.Identifier, scope); } - const scope = isVar ? (this.findScope(ScopeType.Function) || this.scopeStack[0]) : this.peekScope(); - return tsHelper.needsHoisting(identifier.tsOriginal as ts.Identifier, scope.node, this.checker); } private createLocalOrExportedOrGlobalDeclaration( @@ -3193,6 +3254,7 @@ export class LuaTransformer { ): tstl.Statement[] { const isFunctionDeclaration = tsOriginal && ts.isFunctionDeclaration(tsOriginal); + let hoistFunction = false; let left: tstl.IdentifierOrTableIndexExpression | tstl.IdentifierOrTableIndexExpression[] = lhs; if (this.shouldExportIdentifier(lhs)) { @@ -3201,6 +3263,7 @@ export class LuaTransformer { left = lhs.map(i => this.createExportedIdentifier(i)); } else { left = this.createExportedIdentifier(lhs); + hoistFunction = isFunctionDeclaration && this.shouldHoistIdentifier(lhs, false); } } else { @@ -3215,19 +3278,22 @@ export class LuaTransformer { : this.shouldHoistIdentifier(lhs, isVar); if (hoist) { this.hoistLocals(lhs, isLetOrConst || isFunctionDeclaration ? undefined : ScopeType.Function); + hoistFunction = isFunctionDeclaration; + } else if (rhs && tstl.isFunctionExpression(rhs)) { // separate declaration and assignment to support recursive functions return [ tstl.createVariableDeclarationStatement(lhs, undefined, tsOriginal, parent), tstl.createAssignmentStatement(left, rhs, tsOriginal, parent), ]; + } else { return [tstl.createVariableDeclarationStatement(lhs, rhs, tsOriginal, parent)]; } } } - if (isFunctionDeclaration && this.shouldHoistIdentifier(lhs as tstl.Identifier, false)) { + if (hoistFunction) { // hoist function declarations this.hoistFunctionDeclaration(tstl.createAssignmentStatement(left, rhs, tsOriginal, parent)); return undefined; diff --git a/src/LuaTranspiler.ts b/src/LuaTranspiler.ts index 565685cad..ced680fab 100644 --- a/src/LuaTranspiler.ts +++ b/src/LuaTranspiler.ts @@ -4,7 +4,7 @@ import * as ts from "typescript"; import * as tstl from "./LuaAST"; -import {CompilerOptions, LuaLibImportKind, LuaTarget} from "./CompilerOptions"; +import {CompilerOptions, HoistingMode, LuaLibImportKind, LuaTarget} from "./CompilerOptions"; import {LuaPrinter} from "./LuaPrinter"; import {LuaTransformer} from "./LuaTransformer"; @@ -34,6 +34,9 @@ export class LuaTranspiler { if (options.luaLibImport) { options.luaLibImport = options.luaLibImport.toLocaleLowerCase() as LuaLibImportKind; } + if (options.hoisting) { + options.hoisting = options.hoisting.toLowerCase() as HoistingMode; + } return options; } @@ -142,4 +145,4 @@ export class LuaTranspiler { console.log(`${diagnostic.code}: ${ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n")}`); } } -} \ No newline at end of file +} diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 05bfa7379..185fcd7a0 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -542,34 +542,23 @@ export class TSHelper { return false; } - public static needsHoisting(identifier: ts.Identifier, scope: ts.Node, checker: ts.TypeChecker) : boolean + public static findFirstReference( + identifier: ts.Identifier, + scope: ts.Node, + checker: ts.TypeChecker + ) : ts.Identifier { const symbol = checker.getSymbolAtLocation(identifier); - let result = false; - const functionStack: ts.FunctionDeclaration[] = []; - const visitor = (node: ts.Node) => { - if (ts.isFunctionDeclaration(node)) { - if (node.name !== identifier) { // don't recurse into self - functionStack.push(node); - ts.forEachChild(node, visitor); - functionStack.pop(); - } - return; - } else if (ts.isIdentifier(node) + const visitor = (node: ts.Node) => { + if (ts.isIdentifier(node) && node.text === identifier.text && checker.getSymbolAtLocation(node) === symbol) { - // hoist if referenced before declaration, or inside a local function which itself will be hoisted - result = (node.pos < identifier.pos) - || (functionStack.length > 0 && this.needsHoisting(functionStack[0].name, scope, checker)); - } - - if (!result) { - ts.forEachChild(node, visitor); + return node; } + return ts.forEachChild(node, visitor); }; - ts.forEachChild(scope, visitor); - return result; + return ts.forEachChild(scope, visitor); } } diff --git a/src/tstl.ts b/src/tstl.ts index 7c9dec9ea..34749f743 100644 --- a/src/tstl.ts +++ b/src/tstl.ts @@ -1,5 +1,5 @@ export {parseCommandLine} from "./CommandLineParser"; export {compile, compileFilesWithOptions, transpileString, watchWithOptions} from "./Compiler"; -export {CompilerOptions, LuaLibImportKind, LuaTarget,} from "./CompilerOptions"; +export {CompilerOptions, HoistingMode, LuaLibImportKind, LuaTarget,} from "./CompilerOptions"; export {LuaLibFeature,} from "./LuaLib"; export {LuaTranspiler,} from "./LuaTranspiler"; diff --git a/test/unit/compiler/configuration/mixed/index.spec.ts b/test/unit/compiler/configuration/mixed/index.spec.ts index 376cd8c55..7a59890dd 100644 --- a/test/unit/compiler/configuration/mixed/index.spec.ts +++ b/test/unit/compiler/configuration/mixed/index.spec.ts @@ -3,7 +3,7 @@ import * as fs from "fs"; import * as path from "path"; import * as ts from "typescript"; -import { CompilerOptions, LuaLibImportKind } from "../../../../../src/CompilerOptions"; +import { CompilerOptions, LuaLibImportKind, HoistingMode } from "../../../../../src/CompilerOptions"; import { optionDeclarations, parseCommandLine } from "../../../../../src/CommandLineParser"; export class MixedConfigurationTests { @@ -35,6 +35,7 @@ export class MixedConfigurationTests { // Only present in TSTL dfaults noHeader: optionDeclarations["noHeader"].default, project: tsConfigPath, + hoisting: HoistingMode.None, } as CompilerOptions); } -} \ No newline at end of file +} From baf226e928a0c474a9bed350d2e6ab331c507560 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Fri, 1 Feb 2019 15:07:35 -0700 Subject: [PATCH 16/24] Another full refactor - checking variables for reference-before-declaration when they are first seen to avoid scanning later - caching nested functions in scope to avoid repeat scan later - added error for attempting to hoist when hoisting is off since it's easy to detect now - added tests for hoisting errors --- src/LuaTransformer.ts | 136 +++++++++++++++++++++---------------- src/TSHelper.ts | 53 ++++++++++++--- src/TSTLErrors.ts | 8 +++ test/unit/hoisting.spec.ts | 48 +++++++++++-- 4 files changed, 174 insertions(+), 71 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index d4cd34079..3ad60cb59 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -26,6 +26,7 @@ interface Scope { node: ts.Node; hoistedLocals: tstl.Identifier[]; hoistedFunctions: tstl.Statement[]; + nestedFunctionsCache?: ts.FunctionDeclaration[]; } export class LuaTransformer { @@ -49,7 +50,7 @@ export class LuaTransformer { private genVarCounter: number; private luaLibFeatureSet: Set; - private hoistingCache: Map; + private symbolReferencedBeforeDeclaration: Map; private readonly typeValidationCache: Map> = new Map>(); @@ -76,7 +77,7 @@ export class LuaTransformer { this.scopeStack = []; this.classStack = []; this.luaLibFeatureSet = new Set(); - this.hoistingCache = new Map(); + this.symbolReferencedBeforeDeclaration = new Map(); } // TODO make all other methods private??? @@ -3036,6 +3037,18 @@ export class LuaTransformer { // But this should be changed to retun tstl.createNilLiteral() // at some point. } + + // Track identifiers seen before they are declared + const symbol = this.checker.getSymbolAtLocation(expression); + if (symbol && !this.symbolReferencedBeforeDeclaration.has(symbol)) { + const firstDeclaration = tsHelper.getFirstDeclaration(symbol, this.currentSourceFile); + const referencedBeforeDeclaration = firstDeclaration && expression.pos < firstDeclaration.pos; + this.symbolReferencedBeforeDeclaration.set(symbol, referencedBeforeDeclaration); + if (this.options.hoisting === HoistingMode.None && referencedBeforeDeclaration) { + throw TSTLErrors.ReferencedBeforeDeclaration(expression); + } + } + let escapedText = expression.escapedText as string; const underScoreCharCode = "_".charCodeAt(0); if (escapedText.length >= 3 && escapedText.charCodeAt(0) === underScoreCharCode && @@ -3182,54 +3195,65 @@ export class LuaTransformer { return tstl.createIdentifier("self", tsOriginal); } - private identifierNeedsHoisting(identifier: ts.Identifier, scope: Scope) : boolean { - const symbol = this.checker.getSymbolAtLocation(identifier); - if (this.hoistingCache.has(symbol)) { - return this.hoistingCache.get(symbol); + private isReferencedBeforeDeclaration(symbol: ts.Symbol): boolean { + let referencedBeforeDeclaration = this.symbolReferencedBeforeDeclaration.get(symbol); + if (referencedBeforeDeclaration === undefined) { + const declaration = tsHelper.getFirstDeclaration(symbol, this.currentSourceFile); + if (declaration) { + const firstReference = tsHelper.findFirstReference(symbol, declaration.parent, this.checker); + referencedBeforeDeclaration = firstReference.pos < declaration.pos; + } else { + referencedBeforeDeclaration = false; + } + this.symbolReferencedBeforeDeclaration.set(symbol, referencedBeforeDeclaration); } + return referencedBeforeDeclaration; + } + private identifierNeedsHoisting(identifier: ts.Identifier, scope: Scope) : boolean { if (identifier.parent && ts.isFunctionDeclaration(identifier.parent) - && scope.type === ScopeType.File + && identifier.parent.parent + && ts.isSourceFile(identifier.parent.parent) && !this.isModule) { - // Dont' hoist global function declarations - this.hoistingCache.set(symbol, false); + // Don't hoist global function declarations in non-modules return false; } - const visitor = (node: ts.Node) => { - if (node.pos < identifier.pos) { - // check for reference before declaration - if (ts.isIdentifier(node) - && node.text === identifier.text - && this.checker.getSymbolAtLocation(node) === symbol) - { - return true; - } + if (this.options.hoisting === HoistingMode.Full) { + return true; + } - } else if (node.pos > identifier.pos) { - // check for reference in function that will be hoisted - if (ts.isFunctionDeclaration(node)) { - if (node.body - && this.identifierNeedsHoisting(node.name, scope) - && tsHelper.findFirstReference(identifier, node.body, this.checker)) - { - return true; - } - return false; - } - } + const symbol = this.checker.getSymbolAtLocation(identifier); + if (this.isReferencedBeforeDeclaration(symbol)) { + return true; + } - return ts.forEachChild(node, visitor); - }; + const declarations = symbol.getDeclarations(); + if (!declarations || declarations.length === 0) { + return false; + } - const result = ts.forEachChild(scope.node, visitor) !== undefined; - this.hoistingCache.set(symbol, result); - return result; + // Check for nested functions that reference the identifier and will be hoisted + if (!scope.nestedFunctionsCache) { + scope.nestedFunctionsCache = tsHelper.findNodes(scope.node, ts.isFunctionDeclaration, false); + } + for (const func of scope.nestedFunctionsCache) { + if (func.name && func.body + && func.pos > declarations[0].pos // No need to check functions before identifier was declared + && declarations.indexOf(func) < 0 // Prevent recursing into self + && this.identifierNeedsHoisting(func.name, scope) + && tsHelper.findFirstReference(symbol, func.body, this.checker)) + { + return true; + } + } + + return false; } - private shouldHoistIdentifier(identifier: tstl.Identifier, isVar: boolean): boolean { + private shouldHoistIdentifier(identifier: tstl.Identifier, scope: Scope): boolean { if (this.options.hoisting === HoistingMode.None || !identifier.tsOriginal || !ts.isIdentifier(identifier.tsOriginal) @@ -3237,12 +3261,8 @@ export class LuaTransformer { { return false; - } else if (this.options.hoisting === HoistingMode.Full) { - return true; - } else { - const scope = isVar ? (this.findScope(ScopeType.Function) || this.scopeStack[0]) : this.peekScope(); - return this.identifierNeedsHoisting(identifier.tsOriginal as ts.Identifier, scope); + return this.identifierNeedsHoisting(identifier.tsOriginal, scope); } } @@ -3263,21 +3283,28 @@ export class LuaTransformer { left = lhs.map(i => this.createExportedIdentifier(i)); } else { left = this.createExportedIdentifier(lhs); - hoistFunction = isFunctionDeclaration && this.shouldHoistIdentifier(lhs, false); + hoistFunction = isFunctionDeclaration && this.shouldHoistIdentifier(lhs, this.peekScope()); } } else { const insideFunction = this.findScope(ScopeType.Function) !== undefined; - const isLetOrConst = tsOriginal && ts.isVariableDeclaration(tsOriginal) - && tsOriginal.parent && (tsOriginal.parent.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) !== 0; - if (this.isModule || this.currentNamespace || insideFunction || isLetOrConst) { + let isLetOrConst = false; + let isFirstDeclaration = true; // var can have multiple declarations for the same variable :/ + if (tsOriginal && ts.isVariableDeclaration(tsOriginal)) { + isLetOrConst = tsOriginal.parent + && (tsOriginal.parent.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) !== 0; + isFirstDeclaration = isLetOrConst || tsHelper.isFirstDeclaration(tsOriginal, this.checker); + } + if ((this.isModule || this.currentNamespace || insideFunction || isLetOrConst) && isFirstDeclaration) { // local - const isVar = !(isLetOrConst || isFunctionDeclaration); + const scope = isLetOrConst || isFunctionDeclaration + ? this.peekScope() + : this.findScope(ScopeType.Function, ScopeType.File); const hoist = Array.isArray(lhs) - ? lhs.some(i => this.shouldHoistIdentifier(i, !isLetOrConst)) - : this.shouldHoistIdentifier(lhs, isVar); + ? lhs.some(i => this.shouldHoistIdentifier(i, scope)) + : this.shouldHoistIdentifier(lhs, scope); if (hoist) { - this.hoistLocals(lhs, isLetOrConst || isFunctionDeclaration ? undefined : ScopeType.Function); + this.hoistLocals(lhs, scope); hoistFunction = isFunctionDeclaration; } else if (rhs && tstl.isFunctionExpression(rhs)) { @@ -3421,14 +3448,9 @@ export class LuaTransformer { return [scope, hoistedStatements]; } - protected hoistLocals(locals: tstl.Identifier | tstl.Identifier[], scopeType?: ScopeType): void { - let scope: Scope; - if (!scopeType) { + protected hoistLocals(locals: tstl.Identifier | tstl.Identifier[], scope?: Scope): void { + if (!scope) { scope = this.peekScope(); - - } else { - // find first scope with desired type (default to file scope) - scope = this.scopeStack.slice().reverse().find(s => s.type === scopeType) || this.scopeStack[0]; } if (Array.isArray(locals)) { @@ -3450,7 +3472,7 @@ export class LuaTransformer { parent?: tstl.Node ): tstl.AssignmentStatement | tstl.VariableDeclarationStatement { - if (this.shouldHoistIdentifier(variable, false)) { + if (this.shouldHoistIdentifier(variable, this.peekScope())) { this.hoistLocals(variable); return tstl.createAssignmentStatement(variable, initializer, tsOriginal, parent); } else { diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 185fcd7a0..d221778d0 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -542,18 +542,32 @@ export class TSHelper { return false; } - public static findFirstReference( - identifier: ts.Identifier, - scope: ts.Node, - checker: ts.TypeChecker - ) : ts.Identifier - { - const symbol = checker.getSymbolAtLocation(identifier); + public static getFirstDeclaration(symbol: ts.Symbol, sourceFile?: ts.SourceFile): ts.Declaration | undefined { + let declarations = symbol.getDeclarations(); + if (!declarations) { + return undefined; + } + if (sourceFile) { + declarations = declarations.filter(d => this.findFirstNodeAbove(d, ts.isSourceFile) === sourceFile); + } + return declarations.length > 0 + ? declarations.reduce((p, c) => p.pos < c.pos ? p : c) + : undefined; + } + public static isFirstDeclaration(node: ts.VariableDeclaration, checker: ts.TypeChecker): boolean { + const symbol = checker.getSymbolAtLocation(node.name); + if (!symbol) { + return false; + } + const firstDeclaration = this.getFirstDeclaration(symbol); + return firstDeclaration === node; + } + + public static findFirstReference(symbol: ts.Symbol, scope: ts.Node, checker: ts.TypeChecker) : ts.Identifier + { const visitor = (node: ts.Node) => { - if (ts.isIdentifier(node) - && node.text === identifier.text - && checker.getSymbolAtLocation(node) === symbol) + if (checker.getSymbolAtLocation(node) === symbol) { return node; } @@ -561,4 +575,23 @@ export class TSHelper { }; return ts.forEachChild(scope, visitor); } + + public static findNodes( + root: ts.Node, + filter: (node: ts.Node) => node is T, + recurseIntoResults: boolean + ): T[] { + const results: T[] = []; + const visitor = (node: ts.Node) => { + if (filter(node)) { + results.push(node); + if (!recurseIntoResults) { + return; + } + } + ts.forEachChild(node, visitor); + }; + ts.forEachChild(root, visitor); + return results; + } } diff --git a/src/TSTLErrors.ts b/src/TSTLErrors.ts index e7f55d599..b5719bc90 100644 --- a/src/TSTLErrors.ts +++ b/src/TSTLErrors.ts @@ -119,4 +119,12 @@ export class TSTLErrors { "the TupleReturn decorator.", node); }; + + public static ReferencedBeforeDeclaration = (node: ts.Identifier) => { + return new TranspileError( + `Identifier "${node.text}" was referenced before it was declared. The declaration ` + + "must be moved before the identifier's use, or hoisting must be enabled.", + node + ); + } } diff --git a/test/unit/hoisting.spec.ts b/test/unit/hoisting.spec.ts index 2927ac8cf..314dfd7e9 100644 --- a/test/unit/hoisting.spec.ts +++ b/test/unit/hoisting.spec.ts @@ -1,8 +1,17 @@ +import * as ts from "typescript"; import { Expect, Test, TestCase } from "alsatian"; import * as util from "../src/util"; +import { CompilerOptions, LuaLibImportKind, LuaTarget, HoistingMode } from "../../src/CompilerOptions"; +import { TranspileError } from "../../src/TranspileError"; export class HoistingTests { + private static readonly hoistingCompilerOptions: CompilerOptions = { + hoisting: HoistingMode.Required, + luaLibImport: LuaLibImportKind.Require, + luaTarget: LuaTarget.Lua53, + target: ts.ScriptTarget.ES2015, + }; @Test("Var Hoisting") public varHoisting(): void { @@ -10,7 +19,7 @@ export class HoistingTests { `foo = "foo"; var foo; return foo;`; - const result = util.transpileAndExecute(code); + const result = util.transpileAndExecute(code, HoistingTests.hoistingCompilerOptions); Expect(result).toBe("foo"); } @@ -33,7 +42,7 @@ export class HoistingTests { ${varType} foo = "foo"; setBar(); return foo;`; - const result = util.transpileAndExecute(code); + const result = util.transpileAndExecute(code, HoistingTests.hoistingCompilerOptions); Expect(result).toBe("foo"); } @@ -56,7 +65,7 @@ export class HoistingTests { `const foo = bar(); function bar() { return "bar"; } return foo;`; - const result = util.transpileAndExecute(code); + const result = util.transpileAndExecute(code, HoistingTests.hoistingCompilerOptions); Expect(result).toBe("bar"); } @@ -117,7 +126,7 @@ export class HoistingTests { return bar; } return foo();`; - const result = util.transpileAndExecute(code); + const result = util.transpileAndExecute(code, HoistingTests.hoistingCompilerOptions); Expect(result).toBe(expectResult); } @@ -192,4 +201,35 @@ export class HoistingTests { const result = util.transpileExecuteAndReturnExport(code, "foo"); Expect(result).toBe("foo"); } + + @TestCase(`foo = "foo"; var foo;`, "foo") + @TestCase(`foo = "foo"; export var foo;`, "foo") + @TestCase(`function setBar() { const bar = foo; } let foo = "foo";`, "foo") + @TestCase(`function setBar() { const bar = foo; } const foo = "foo";`, "foo") + @TestCase(`function setBar() { const bar = foo; } export let foo = "foo";`, "foo") + @TestCase(`function setBar() { const bar = foo; } export const foo = "foo";`, "foo") + @TestCase(`const foo = bar(); function bar() { return "bar"; }`, "bar") + @TestCase(`export const foo = bar(); function bar() { return "bar"; }`, "bar") + @TestCase(`const foo = bar(); export function bar() { return "bar"; }`, "bar") + @TestCase(`function bar() { return NS.foo; } namespace NS { export let foo = "foo"; }`, "NS") + @TestCase( + `export namespace O { export function f() { return I.foo; } namespace I { export let foo = "foo"; } }`, + "I" + ) + @TestCase(`function makeFoo() { return new Foo(); } class Foo {}`, "Foo") + @TestCase(`function bar() { return E.A; } enum E { A = "foo" }`, "E") + @Test("No Hoisting") + public noHoisting(code: string, identifier: string): void { + const compilerOptions: CompilerOptions = { + hoisting: HoistingMode.None, + luaLibImport: LuaLibImportKind.Require, + luaTarget: LuaTarget.Lua53, + target: ts.ScriptTarget.ES2015, + }; + Expect(() => util.transpileString(code, compilerOptions)).toThrowError( + TranspileError, + `Identifier "${identifier}" was referenced before it was declared. The declaration ` + + "must be moved before the identifier's use, or hoisting must be enabled." + ); + } } From 67c44b654ce7a5346c92435d495b5845fc0a6b38 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Fri, 1 Feb 2019 15:32:22 -0700 Subject: [PATCH 17/24] change ScopeType to bitfields --- src/LuaTransformer.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 3ad60cb59..a5cbd599d 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -12,12 +12,12 @@ export type StatementVisitResult = tstl.Statement | tstl.Statement[] | undefined export type ExpressionVisitResult = tstl.Expression | undefined; export enum ScopeType { - File, - Function, - Switch, - Loop, - Conditional, - Block, + File = 0x1, + Function = 0x2, + Switch = 0x4, + Loop = 0x8, + Conditional = 0x10, + Block = 0x20, } interface Scope { @@ -1503,7 +1503,7 @@ export class LuaTransformer { } public transformBreakStatement(breakStatement: ts.BreakStatement): StatementVisitResult { - const breakableScope = this.findScope(ScopeType.Loop, ScopeType.Switch); + const breakableScope = this.findScope(ScopeType.Loop | ScopeType.Switch); if (breakableScope.type === ScopeType.Switch) { return tstl.createGotoStatement(`____TS_switch${breakableScope.id}_end`); } else { @@ -3299,7 +3299,7 @@ export class LuaTransformer { // local const scope = isLetOrConst || isFunctionDeclaration ? this.peekScope() - : this.findScope(ScopeType.Function, ScopeType.File); + : this.findScope(ScopeType.Function | ScopeType.File); const hoist = Array.isArray(lhs) ? lhs.some(i => this.shouldHoistIdentifier(i, scope)) : this.shouldHoistIdentifier(lhs, scope); @@ -3419,8 +3419,8 @@ export class LuaTransformer { return tstl.createBinaryExpression(expression, tstl.createNumericLiteral(1), tstl.SyntaxKind.AdditionOperator); } - protected findScope(...scopeTypes: ScopeType[]): Scope | undefined { - return this.scopeStack.slice().reverse().find(s => scopeTypes.find(t => s.type === t) !== undefined); + protected findScope(scopeTypes: ScopeType): Scope | undefined { + return this.scopeStack.slice().reverse().find(s => (scopeTypes & s.type) !== 0); } protected peekScope(): Scope { From 31c6890c051e28a1f96fb75ce235104a983565f1 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Sun, 3 Feb 2019 16:39:28 -0700 Subject: [PATCH 18/24] rewrote hoisting to defer all logic to popScope, so that there's no need for additional scanning --- src/CommandLineParser.ts | 10 +- src/Compiler.ts | 2 +- src/CompilerOptions.ts | 8 +- src/LuaAST.ts | 26 +- src/LuaPrinter.ts | 9 +- src/LuaTransformer.ts | 374 ++++++++++-------- src/LuaTranspiler.ts | 5 +- src/TSHelper.ts | 31 -- src/tstl.ts | 2 +- .../configuration/mixed/index.spec.ts | 4 +- test/unit/hoisting.spec.ts | 19 +- 11 files changed, 240 insertions(+), 250 deletions(-) diff --git a/src/CommandLineParser.ts b/src/CommandLineParser.ts index eff2d6eda..cf1a349e2 100644 --- a/src/CommandLineParser.ts +++ b/src/CommandLineParser.ts @@ -32,12 +32,10 @@ export const optionDeclarations: YargsOptions = { describe: "Specify if a header will be added to compiled files.", type: "boolean", }, - hoisting: { - alias: "h", - default: "none", - choices: ["none", "full", "required"], - describe: "Specifies how variable hoisting is handled.", - type: "string", + noHoisting: { + default: false, + describe: "Disables hoisting.", + type: "boolean", }, }; diff --git a/src/Compiler.ts b/src/Compiler.ts index 50f3bfb31..9dfeadf30 100644 --- a/src/Compiler.ts +++ b/src/Compiler.ts @@ -3,7 +3,7 @@ import * as path from "path"; import * as ts from "typescript"; import {parseCommandLine} from "./CommandLineParser"; -import {CompilerOptions, HoistingMode, LuaLibImportKind, LuaTarget} from "./CompilerOptions"; +import {CompilerOptions, LuaLibImportKind, LuaTarget} from "./CompilerOptions"; import {LuaTranspiler} from "./LuaTranspiler"; export function compile(argv: string[]): void { diff --git a/src/CompilerOptions.ts b/src/CompilerOptions.ts index 68627bc4a..24fae4374 100644 --- a/src/CompilerOptions.ts +++ b/src/CompilerOptions.ts @@ -4,7 +4,7 @@ export interface CompilerOptions extends ts.CompilerOptions { noHeader?: boolean; luaTarget?: LuaTarget; luaLibImport?: LuaLibImportKind; - hoisting?: HoistingMode; + noHoisting?: boolean; } export enum LuaLibImportKind { @@ -20,9 +20,3 @@ export enum LuaTarget { Lua53 = "5.3", LuaJIT = "jit", } - -export enum HoistingMode { - None = "none", - Full = "full", - Required = "required", -} diff --git a/src/LuaAST.ts b/src/LuaAST.ts index cba7fb475..5975be2d1 100644 --- a/src/LuaAST.ts +++ b/src/LuaAST.ts @@ -106,7 +106,6 @@ export interface TextRange { export interface Node extends TextRange { kind: SyntaxKind; parent?: Node; - tsOriginal?: ts.Node; } export function createNode(kind: SyntaxKind, tsOriginal?: ts.Node, parent?: Node): Node { @@ -116,7 +115,7 @@ export function createNode(kind: SyntaxKind, tsOriginal?: ts.Node, parent?: Node pos = tsOriginal.pos; end = tsOriginal.end; } - return {kind, parent, pos, end, tsOriginal}; + return {kind, parent, pos, end}; } export function cloneNode(node: T): T { @@ -126,7 +125,6 @@ export function cloneNode(node: T): T { export function setNodeOriginal(node: T, tsOriginal: ts.Node): T { node.pos = tsOriginal.pos; node.end = tsOriginal.end; - node.tsOriginal = tsOriginal; return node; } @@ -141,9 +139,6 @@ export function setParent(node: Node | Node[] | undefined, parent: Node): void n.pos = parent.pos; n.end = parent.end; } - if (!n.tsOriginal) { - n.tsOriginal = parent.tsOriginal; - } }); } else { node.parent = parent; @@ -151,9 +146,6 @@ export function setParent(node: Node | Node[] | undefined, parent: Node): void node.pos = parent.pos; node.end = parent.end; } - if (!node.tsOriginal) { - node.tsOriginal = parent.tsOriginal; - } } } @@ -198,6 +190,7 @@ export interface VariableDeclarationStatement extends Statement { kind: SyntaxKind.VariableDeclarationStatement; left: Identifier[]; right?: Expression[]; + local: boolean; } export function isVariableDeclarationStatement(node: Node): node is VariableDeclarationStatement { @@ -208,7 +201,8 @@ export function createVariableDeclarationStatement( left: Identifier | Identifier[], right?: Expression | Expression[], tsOriginal?: ts.Node, - parent?: Node + parent?: Node, + local = true ): VariableDeclarationStatement { const statement = createNode( @@ -228,6 +222,7 @@ export function createVariableDeclarationStatement( } else if (right) { statement.right = [right]; } + statement.local = local; return statement; } @@ -236,6 +231,7 @@ export interface AssignmentStatement extends Statement { kind: SyntaxKind.AssignmentStatement; left: IdentifierOrTableIndexExpression[]; right: Expression[]; + hoisted?: boolean; } export function isAssignmentStatement(node: Node): node is AssignmentStatement { @@ -799,15 +795,23 @@ export function createMethodCallExpression( export interface Identifier extends Expression { kind: SyntaxKind.Identifier; text: string; + symbol?: ts.Symbol; } export function isIdentifier(node: Node): node is Identifier { return node.kind === SyntaxKind.Identifier; } -export function createIdentifier(text: string | ts.__String, tsOriginal?: ts.Node, parent?: Node): Identifier { +export function createIdentifier( + text: string | ts.__String, + tsOriginal?: ts.Node, + symbol?: ts.Symbol, + parent?: Node +): Identifier +{ const expression = createNode(SyntaxKind.Identifier, tsOriginal, parent) as Identifier; expression.text = text as string; + expression.symbol = symbol; return expression; } diff --git a/src/LuaPrinter.ts b/src/LuaPrinter.ts index 0f509c593..c9b8578c9 100644 --- a/src/LuaPrinter.ts +++ b/src/LuaPrinter.ts @@ -128,7 +128,11 @@ export class LuaPrinter { } private printVariableDeclarationStatement(statement: tstl.VariableDeclarationStatement): string { - const left = this.indent(`local ${statement.left.map(e => this.printExpression(e)).join(", ")}`); + if (!statement.local && !statement.right) { + return ""; + } + const prefix = statement.local ? "local " : ""; + const left = this.indent(`${prefix}${statement.left.map(e => this.printExpression(e)).join(", ")}`); if (statement.right) { return left + ` = ${statement.right.map(e => this.printExpression(e)).join(", ")};\n`; } else { @@ -137,6 +141,9 @@ export class LuaPrinter { } private printVariableAssignmentStatement(statement: tstl.AssignmentStatement): string { + if (statement.hoisted) { + return ""; + } return this.indent( `${statement.left.map(e => this.printExpression(e)).join(", ")} = ` + `${statement.right.map(e => this.printExpression(e)).join(", ")};\n`); diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 13ad3fa68..5eebd8f1b 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1,7 +1,7 @@ import * as path from "path"; import * as ts from "typescript"; -import {CompilerOptions, LuaLibImportKind, LuaTarget, HoistingMode} from "./CompilerOptions"; +import {CompilerOptions, LuaLibImportKind, LuaTarget} from "./CompilerOptions"; import {DecoratorKind} from "./Decorator"; import * as tstl from "./LuaAST"; import {LuaLib, LuaLibFeature} from "./LuaLib"; @@ -20,13 +20,26 @@ export enum ScopeType { Block = 0x20, } +interface SymbolInfo { + firstSeenAtPos: number; +} + +interface VariableDeclarationInfo { + symbols: ts.Symbol[]; + declaration: tstl.VariableDeclarationStatement; +} + +interface FunctionDefinitionInfo { + referencedSymbols: Set; + assignment?: tstl.AssignmentStatement; +} + interface Scope { type: ScopeType; id: number; - node: ts.Node; - hoistedLocals: tstl.Identifier[]; - hoistedFunctions: tstl.Statement[]; - nestedFunctionsCache?: ts.FunctionDeclaration[]; + referencedSymbols?: Set; + variableDeclarations?: VariableDeclarationInfo[]; + functionDefinitions?: Map; } export class LuaTransformer { @@ -50,7 +63,7 @@ export class LuaTransformer { private genVarCounter: number; private luaLibFeatureSet: Set; - private symbolReferencedBeforeDeclaration: Map; + private symbolInfo: Map; private readonly typeValidationCache: Map> = new Map>(); @@ -63,9 +76,6 @@ export class LuaTransformer { if (!this.options.luaTarget) { this.options.luaTarget = LuaTarget.LuaJIT; } - if (!this.options.hoisting) { - this.options.hoisting = HoistingMode.Required; - } this.setupState(); } @@ -77,7 +87,7 @@ export class LuaTransformer { this.scopeStack = []; this.classStack = []; this.luaLibFeatureSet = new Set(); - this.symbolReferencedBeforeDeclaration = new Map(); + this.symbolInfo = new Map(); } // TODO make all other methods private??? @@ -234,19 +244,18 @@ export class LuaTransformer { result.push(requireStatement); filteredElements.forEach(importSpecifier => { - const nameIdentifier = this.transformIdentifier(importSpecifier.name); if (importSpecifier.propertyName) { const propertyIdentifier = this.transformIdentifier(importSpecifier.propertyName); const propertyName = tstl.createStringLiteral(propertyIdentifier.text); const renamedImport = this.createHoistedVariableDeclarationStatement( - nameIdentifier, + importSpecifier.name, tstl.createTableIndexExpression(importUniqueName, propertyName), importSpecifier); result.push(renamedImport); } else { - const name = tstl.createStringLiteral(nameIdentifier.text); + const name = tstl.createStringLiteral(importSpecifier.name.text); const namedImport = this.createHoistedVariableDeclarationStatement( - nameIdentifier, + importSpecifier.name, tstl.createTableIndexExpression(importUniqueName, name), importSpecifier ); @@ -256,7 +265,7 @@ export class LuaTransformer { return result; } else if (ts.isNamespaceImport(imports)) { const requireStatement = this.createHoistedVariableDeclarationStatement( - this.transformIdentifier(imports.name), + imports.name, requireCall, statement ); @@ -611,16 +620,16 @@ export class LuaTransformer { this.createSelfIdentifier() ); - bodyStatements.push(...this.transformFunctionBody(statement.parameters, statement.body, restParamName)); - - const body: tstl.Block = tstl.createBlock(bodyStatements); + const [body] = this.transformFunctionBody(statement.parameters, statement.body, restParamName); + bodyStatements.push(...body); + const block: tstl.Block = tstl.createBlock(bodyStatements); const result = tstl.createAssignmentStatement( tstl.createTableIndexExpression( this.addExportToIdentifier(className), tstl.createStringLiteral("constructor")), - tstl.createFunctionExpression(body, params, dotsLiteral, restParamName, undefined, undefined), + tstl.createFunctionExpression(block, params, dotsLiteral, restParamName, undefined, undefined), statement); this.classStack.pop(); @@ -636,8 +645,9 @@ export class LuaTransformer { { const name = this.transformIdentifier(getAccessor.name as ts.Identifier); + const [body] = this.transformFunctionBody(getAccessor.parameters, getAccessor.body); const accessorFunction = tstl.createFunctionExpression( - tstl.createBlock(this.transformFunctionBody(getAccessor.parameters, getAccessor.body)), + tstl.createBlock(body), [this.createSelfIdentifier()] ); @@ -659,8 +669,9 @@ export class LuaTransformer { const [params, dot, restParam] = this.transformParameters(setAccessor.parameters, this.createSelfIdentifier()); + const [body] = this.transformFunctionBody(setAccessor.parameters, setAccessor.body, restParam); const accessorFunction = tstl.createFunctionExpression( - tstl.createBlock(this.transformFunctionBody(setAccessor.parameters, setAccessor.body, restParam)), + tstl.createBlock(body), params, dot, restParam @@ -695,8 +706,9 @@ export class LuaTransformer { : undefined; const [paramNames, dots, restParamName] = this.transformParameters(node.parameters, context); + const [body] = this.transformFunctionBody(node.parameters, node.body, restParamName); const functionExpression = tstl.createFunctionExpression( - tstl.createBlock(this.transformFunctionBody(node.parameters, node.body, restParamName)), + tstl.createBlock(body), paramNames, dots, restParamName @@ -753,7 +765,7 @@ export class LuaTransformer { parameters: ts.NodeArray, body: ts.Block, spreadIdentifier?: tstl.Identifier - ): tstl.Statement[] + ): [tstl.Statement[], Scope] { this.pushScope(ScopeType.Function, body); @@ -786,10 +798,10 @@ export class LuaTransformer { const bodyStatements = this.transformStatements(body.statements); - const [, hoistedStatements] = this.popScope(); + const [scope, hoistedStatements] = this.popScope(); bodyStatements.unshift(...hoistedStatements); - return headerStatements.concat(bodyStatements); + return [headerStatements.concat(bodyStatements), scope]; } public transformParameterDefaultValueDeclaration(declaration: ts.ParameterDeclaration): tstl.Statement { @@ -885,7 +897,7 @@ export class LuaTransformer { // local innerNS = outerNS.innerNS const localDeclaration = this.createHoistedVariableDeclarationStatement( - this.transformIdentifier(statement.name as ts.Identifier), + statement.name as ts.Identifier, tstl.createTableIndexExpression( this.transformIdentifier(this.currentNamespace.name as ts.Identifier), tstl.createStringLiteral(this.transformIdentifier(statement.name as ts.Identifier).text))); @@ -905,7 +917,7 @@ export class LuaTransformer { // local NS = exports.NS const localDeclaration = this.createHoistedVariableDeclarationStatement( - this.transformIdentifier(statement.name as ts.Identifier), + statement.name as ts.Identifier, this.createExportedIdentifier(this.transformIdentifier(statement.name as ts.Identifier))); result.push(localDeclaration); @@ -1036,10 +1048,24 @@ export class LuaTransformer { const [params, dotsLiteral, restParamName] = this.transformParameters(functionDeclaration.parameters, context); const name = this.transformIdentifier(functionDeclaration.name); - const body = tstl.createBlock( - this.transformFunctionBody(functionDeclaration.parameters, functionDeclaration.body, restParamName) + const [body, functionScope] = this.transformFunctionBody( + functionDeclaration.parameters, + functionDeclaration.body, + restParamName ); - const functionExpression = tstl.createFunctionExpression(body, params, dotsLiteral, restParamName); + const block = tstl.createBlock(body); + const functionExpression = tstl.createFunctionExpression(block, params, dotsLiteral, restParamName); + + // Remember symbols referenced in this function for hoisting later + if (!this.options.noHoisting && functionDeclaration.name) { + const symbol = this.checker.getSymbolAtLocation(functionDeclaration.name); + if (symbol) { + const scope = this.peekScope(); + if (!scope.functionDefinitions) { scope.functionDefinitions = new Map(); } + const functionInfo = {referencedSymbols: functionScope.referencedSymbols || new Set()}; + scope.functionDefinitions.set(symbol, functionInfo); + } + } return this.createLocalOrExportedOrGlobalDeclaration(name, functionExpression, functionDeclaration); } @@ -1301,10 +1327,7 @@ export class LuaTransformer { public transformForOfInitializer(initializer: ts.ForInitializer, expression: tstl.Expression): tstl.Statement { if (ts.isVariableDeclarationList(initializer)) { // Declaration of new variable - this.pushScope(ScopeType.Loop, initializer); // Counter hoisting - probably a better way to handle this const variableDeclarations = this.transformVariableDeclaration(initializer.declarations[0]); - const [, hoistedStatements] = this.popScope(); - variableDeclarations.unshift(...hoistedStatements); if (ts.isArrayBindingPattern(initializer.declarations[0].name)) { expression = this.createUnpackCall(expression, initializer); } @@ -2401,7 +2424,7 @@ export class LuaTransformer { ); const body = ts.isBlock(node.body) ? node.body : ts.createBlock([ts.createReturn(node.body)]); - const transformedBody = this.transformFunctionBody(node.parameters, body, spreadIdentifier); + const [transformedBody] = this.transformFunctionBody(node.parameters, body, spreadIdentifier); return tstl.createFunctionExpression( tstl.createBlock(transformedBody), @@ -3122,14 +3145,24 @@ export class LuaTransformer { // at some point. } - // Track identifiers seen before they are declared const symbol = this.checker.getSymbolAtLocation(expression); - if (symbol && !this.symbolReferencedBeforeDeclaration.has(symbol)) { - const firstDeclaration = tsHelper.getFirstDeclaration(symbol, this.currentSourceFile); - const referencedBeforeDeclaration = firstDeclaration && expression.pos < firstDeclaration.pos; - this.symbolReferencedBeforeDeclaration.set(symbol, referencedBeforeDeclaration); - if (this.options.hoisting === HoistingMode.None && referencedBeforeDeclaration) { - throw TSTLErrors.ReferencedBeforeDeclaration(expression); + if (symbol) { + if (this.options.noHoisting) { + // Check for reference-before-declaration + const declaration = tsHelper.getFirstDeclaration(symbol, this.currentSourceFile); + if (declaration && expression.pos < declaration.pos) { + throw TSTLErrors.ReferencedBeforeDeclaration(expression); + } + + } else { + // Track symbols seen in scope + if (!this.symbolInfo.has(symbol)) { + this.symbolInfo.set(symbol, {firstSeenAtPos: expression.pos}); + } + this.scopeStack.forEach(s => { + if (!s.referencedSymbols) { s.referencedSymbols = new Set(); } + s.referencedSymbols.add(symbol); + }); } } @@ -3143,7 +3176,7 @@ export class LuaTransformer { if (this.luaKeywords.has(escapedText)) { throw TSTLErrors.KeywordIdentifier(expression); } - return tstl.createIdentifier(escapedText, expression); + return tstl.createIdentifier(escapedText, expression, symbol); } public transformIdentifierExpression(expression: ts.Identifier): tstl.IdentifierOrTableIndexExpression { @@ -3154,7 +3187,7 @@ export class LuaTransformer { } public isIdentifierExported(identifierName: string | ts.__String): boolean { - if (!this.isModule && !this.currentNamespace) { + if (!this.currentNamespace && !(this.isModule && this.peekScope().type === ScopeType.File)) { return false; } const currentScope = this.currentNamespace ? this.currentNamespace : this.currentSourceFile; @@ -3265,7 +3298,7 @@ export class LuaTransformer { } private shouldExportIdentifier(identifier: tstl.Identifier | tstl.Identifier[]): boolean { - if (!this.isModule && !this.currentNamespace) { + if (!this.currentNamespace && !(this.isModule && this.peekScope().type === ScopeType.File)) { return false; } if (Array.isArray(identifier)) { @@ -3279,77 +3312,6 @@ export class LuaTransformer { return tstl.createIdentifier("self", tsOriginal); } - private isReferencedBeforeDeclaration(symbol: ts.Symbol): boolean { - let referencedBeforeDeclaration = this.symbolReferencedBeforeDeclaration.get(symbol); - if (referencedBeforeDeclaration === undefined) { - const declaration = tsHelper.getFirstDeclaration(symbol, this.currentSourceFile); - if (declaration) { - const firstReference = tsHelper.findFirstReference(symbol, declaration.parent, this.checker); - referencedBeforeDeclaration = firstReference.pos < declaration.pos; - } else { - referencedBeforeDeclaration = false; - } - this.symbolReferencedBeforeDeclaration.set(symbol, referencedBeforeDeclaration); - } - return referencedBeforeDeclaration; - } - - private identifierNeedsHoisting(identifier: ts.Identifier, scope: Scope) : boolean { - if (identifier.parent - && ts.isFunctionDeclaration(identifier.parent) - && identifier.parent.parent - && ts.isSourceFile(identifier.parent.parent) - && !this.isModule) - { - // Don't hoist global function declarations in non-modules - return false; - } - - if (this.options.hoisting === HoistingMode.Full) { - return true; - } - - const symbol = this.checker.getSymbolAtLocation(identifier); - if (this.isReferencedBeforeDeclaration(symbol)) { - return true; - } - - const declarations = symbol.getDeclarations(); - if (!declarations || declarations.length === 0) { - return false; - } - - // Check for nested functions that reference the identifier and will be hoisted - if (!scope.nestedFunctionsCache) { - scope.nestedFunctionsCache = tsHelper.findNodes(scope.node, ts.isFunctionDeclaration, false); - } - for (const func of scope.nestedFunctionsCache) { - if (func.name && func.body - && func.pos > declarations[0].pos // No need to check functions before identifier was declared - && declarations.indexOf(func) < 0 // Prevent recursing into self - && this.identifierNeedsHoisting(func.name, scope) - && tsHelper.findFirstReference(symbol, func.body, this.checker)) - { - return true; - } - } - - return false; - } - - private shouldHoistIdentifier(identifier: tstl.Identifier, scope: Scope): boolean { - if (this.options.hoisting === HoistingMode.None - || !identifier.tsOriginal - || !ts.isIdentifier(identifier.tsOriginal) - || identifier.tsOriginal.pos < 0) - { - return false; - - } else { - return this.identifierNeedsHoisting(identifier.tsOriginal, scope); - } - } - private createLocalOrExportedOrGlobalDeclaration( lhs: tstl.Identifier | tstl.Identifier[], rhs?: tstl.Expression, @@ -3357,17 +3319,31 @@ export class LuaTransformer { parent?: tstl.Node ): tstl.Statement[] { - const isFunctionDeclaration = tsOriginal && ts.isFunctionDeclaration(tsOriginal); - let hoistFunction = false; + let declaration: tstl.VariableDeclarationStatement | undefined; + let assignment: tstl.AssignmentStatement | undefined; + + const functionDeclaration = tsOriginal && ts.isFunctionDeclaration(tsOriginal) ? tsOriginal : undefined; - let left: tstl.IdentifierOrTableIndexExpression | tstl.IdentifierOrTableIndexExpression[] = lhs; if (this.shouldExportIdentifier(lhs)) { // exported - if (Array.isArray(lhs)) { - left = lhs.map(i => this.createExportedIdentifier(i)); + if (!rhs) { + return []; + + } else if (Array.isArray(lhs)) { + assignment = tstl.createAssignmentStatement( + lhs.map(i => this.createExportedIdentifier(i)), + rhs, + tsOriginal, + parent + ); + } else { - left = this.createExportedIdentifier(lhs); - hoistFunction = isFunctionDeclaration && this.shouldHoistIdentifier(lhs, this.peekScope()); + assignment = tstl.createAssignmentStatement( + this.createExportedIdentifier(lhs), + rhs, + tsOriginal, + parent + ); } } else { @@ -3381,39 +3357,50 @@ export class LuaTransformer { } if ((this.isModule || this.currentNamespace || insideFunction || isLetOrConst) && isFirstDeclaration) { // local - const scope = isLetOrConst || isFunctionDeclaration - ? this.peekScope() - : this.findScope(ScopeType.Function | ScopeType.File); - const hoist = Array.isArray(lhs) - ? lhs.some(i => this.shouldHoistIdentifier(i, scope)) - : this.shouldHoistIdentifier(lhs, scope); - if (hoist) { - this.hoistLocals(lhs, scope); - hoistFunction = isFunctionDeclaration; - - } else if (rhs && tstl.isFunctionExpression(rhs)) { - // separate declaration and assignment to support recursive functions - return [ - tstl.createVariableDeclarationStatement(lhs, undefined, tsOriginal, parent), - tstl.createAssignmentStatement(left, rhs, tsOriginal, parent), - ]; + if (rhs && tstl.isFunctionExpression(rhs)) { + declaration = tstl.createVariableDeclarationStatement(lhs, undefined, tsOriginal, parent); + assignment = tstl.createAssignmentStatement(lhs, rhs, tsOriginal, parent); } else { - return [tstl.createVariableDeclarationStatement(lhs, rhs, tsOriginal, parent)]; + declaration = tstl.createVariableDeclarationStatement(lhs, rhs, tsOriginal, parent); } + + if (!this.options.noHoisting) { + let symbols = Array.isArray(lhs) ? lhs.map(i => i.symbol) : [lhs.symbol]; + symbols = symbols.filter(i => i !== undefined); + + const scope = isLetOrConst || functionDeclaration + ? this.peekScope() + : this.findScope(ScopeType.Function | ScopeType.File); + + // Remember local variable declarations for hoisting later + if (!scope.variableDeclarations) { scope.variableDeclarations = []; } + scope.variableDeclarations.push({declaration, symbols}); + } + + } else if (rhs) { + // global + assignment = tstl.createAssignmentStatement(lhs, rhs, tsOriginal, parent); + + } else { + return []; } } - if (hoistFunction) { - // hoist function declarations - this.hoistFunctionDeclaration(tstl.createAssignmentStatement(left, rhs, tsOriginal, parent)); - return undefined; + // Remember function definitions for hoisting later + if (!this.options.noHoisting && functionDeclaration && functionDeclaration.name) { + const functionSymbol = this.checker.getSymbolAtLocation(functionDeclaration.name); + if (functionSymbol) { + this.peekScope().functionDefinitions.get(functionSymbol).assignment = assignment; + } } - if (rhs) { - return [tstl.createAssignmentStatement(left, rhs, tsOriginal, parent)]; + if (declaration && assignment) { + return [declaration, assignment]; + } else if (declaration) { + return [declaration]; } else { - return []; + return [assignment]; } } @@ -3512,56 +3499,97 @@ export class LuaTransformer { } protected pushScope(scopeType: ScopeType, node: ts.Node): void { - this.scopeStack.push({type: scopeType, id: this.genVarCounter, node, hoistedLocals: [], hoistedFunctions: []}); + this.scopeStack.push({ + type: scopeType, + id: this.genVarCounter, + }); this.genVarCounter++; } - protected popScope(): [Scope, tstl.Statement[]] { - const scope = this.scopeStack.pop(); + private shouldHoist(symbol: ts.Symbol, scope: Scope): boolean { + const symbolInfo = this.symbolInfo.get(symbol); + if (!symbolInfo) { + return false; + } - const hoistedStatements: tstl.Statement[] = []; + const declaration = tsHelper.getFirstDeclaration(symbol, this.currentSourceFile); + if (!declaration) { + return false; + } - // hoisted locals - if (scope.hoistedLocals.length > 0) { - hoistedStatements.push(tstl.createVariableDeclarationStatement(scope.hoistedLocals)); + if (symbolInfo.firstSeenAtPos < declaration.pos) { + return true; } - // hoisted function declarations - hoistedStatements.push(...scope.hoistedFunctions); + if (scope.functionDefinitions) { + for (const functionSymbolAndInfo of scope.functionDefinitions) { + const functionSymbol = functionSymbolAndInfo[0]; + const functionInfo = functionSymbolAndInfo[1]; + if (functionSymbol !== symbol + && symbolInfo.firstSeenAtPos < functionInfo.assignment.pos + && functionInfo.referencedSymbols.has(symbol) + && this.shouldHoist(functionSymbol, scope)) + { + return true; + } + } + } - return [scope, hoistedStatements]; + return false; } - protected hoistLocals(locals: tstl.Identifier | tstl.Identifier[], scope?: Scope): void { - if (!scope) { - scope = this.peekScope(); + protected popScope(): [Scope, tstl.Statement[]] { + const scope = this.scopeStack.pop(); + + const hoistedStatements: tstl.Statement[] = []; + + // Hoist variable declarations + if (scope.variableDeclarations) { + for (const variableInfo of scope.variableDeclarations) { + if (variableInfo.symbols.some(s => this.shouldHoist(s, scope))) { + hoistedStatements.push(tstl.createVariableDeclarationStatement(variableInfo.declaration.left)); + variableInfo.declaration.local = false; + } + } } - if (Array.isArray(locals)) { - scope.hoistedLocals.push(...locals); - } else { - scope.hoistedLocals.push(locals); + // Hoist function definitions + if (scope.functionDefinitions) { + for (const functionSymbolAndInfo of scope.functionDefinitions) { + const functionSymbol = functionSymbolAndInfo[0]; + const functionInfo = functionSymbolAndInfo[1]; + if (this.shouldHoist(functionSymbol, scope)) { + const statement = tstl.createAssignmentStatement( + functionInfo.assignment.left, + functionInfo.assignment.right, + undefined, + functionInfo.assignment.parent + ); + hoistedStatements.push(statement); + functionInfo.assignment.hoisted = true; + } + } } - } - protected hoistFunctionDeclaration(func: tstl.Statement): void { - const scope = this.peekScope(); - scope.hoistedFunctions.push(func); + return [scope, hoistedStatements]; } protected createHoistedVariableDeclarationStatement( - variable: tstl.Identifier, + identifier: ts.Identifier, initializer?: tstl.Expression, tsOriginal?: ts.Node, parent?: tstl.Node ): tstl.AssignmentStatement | tstl.VariableDeclarationStatement { - if (this.shouldHoistIdentifier(variable, this.peekScope())) { - this.hoistLocals(variable); - return tstl.createAssignmentStatement(variable, initializer, tsOriginal, parent); - } else { - return tstl.createVariableDeclarationStatement(variable, initializer, tsOriginal, parent); + const variable = this.transformIdentifier(identifier); + const declaration = tstl.createVariableDeclarationStatement(variable, initializer, tsOriginal, parent); + const symbol = this.checker.getSymbolAtLocation(identifier); + if (symbol) { + const scope = this.peekScope(); + if (!scope.variableDeclarations) { scope.variableDeclarations = []; } + scope.variableDeclarations.push({symbols: [symbol], declaration}); } + return declaration; } private statementVisitResultToStatementArray(visitResult: StatementVisitResult): tstl.Statement[] { diff --git a/src/LuaTranspiler.ts b/src/LuaTranspiler.ts index ced680fab..13badd2c9 100644 --- a/src/LuaTranspiler.ts +++ b/src/LuaTranspiler.ts @@ -4,7 +4,7 @@ import * as ts from "typescript"; import * as tstl from "./LuaAST"; -import {CompilerOptions, HoistingMode, LuaLibImportKind, LuaTarget} from "./CompilerOptions"; +import {CompilerOptions, LuaLibImportKind, LuaTarget} from "./CompilerOptions"; import {LuaPrinter} from "./LuaPrinter"; import {LuaTransformer} from "./LuaTransformer"; @@ -34,9 +34,6 @@ export class LuaTranspiler { if (options.luaLibImport) { options.luaLibImport = options.luaLibImport.toLocaleLowerCase() as LuaLibImportKind; } - if (options.hoisting) { - options.hoisting = options.hoisting.toLowerCase() as HoistingMode; - } return options; } diff --git a/src/TSHelper.ts b/src/TSHelper.ts index d221778d0..d9cd42249 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -563,35 +563,4 @@ export class TSHelper { const firstDeclaration = this.getFirstDeclaration(symbol); return firstDeclaration === node; } - - public static findFirstReference(symbol: ts.Symbol, scope: ts.Node, checker: ts.TypeChecker) : ts.Identifier - { - const visitor = (node: ts.Node) => { - if (checker.getSymbolAtLocation(node) === symbol) - { - return node; - } - return ts.forEachChild(node, visitor); - }; - return ts.forEachChild(scope, visitor); - } - - public static findNodes( - root: ts.Node, - filter: (node: ts.Node) => node is T, - recurseIntoResults: boolean - ): T[] { - const results: T[] = []; - const visitor = (node: ts.Node) => { - if (filter(node)) { - results.push(node); - if (!recurseIntoResults) { - return; - } - } - ts.forEachChild(node, visitor); - }; - ts.forEachChild(root, visitor); - return results; - } } diff --git a/src/tstl.ts b/src/tstl.ts index 34749f743..7c9dec9ea 100644 --- a/src/tstl.ts +++ b/src/tstl.ts @@ -1,5 +1,5 @@ export {parseCommandLine} from "./CommandLineParser"; export {compile, compileFilesWithOptions, transpileString, watchWithOptions} from "./Compiler"; -export {CompilerOptions, HoistingMode, LuaLibImportKind, LuaTarget,} from "./CompilerOptions"; +export {CompilerOptions, LuaLibImportKind, LuaTarget,} from "./CompilerOptions"; export {LuaLibFeature,} from "./LuaLib"; export {LuaTranspiler,} from "./LuaTranspiler"; diff --git a/test/unit/compiler/configuration/mixed/index.spec.ts b/test/unit/compiler/configuration/mixed/index.spec.ts index 7a59890dd..1268e1ba5 100644 --- a/test/unit/compiler/configuration/mixed/index.spec.ts +++ b/test/unit/compiler/configuration/mixed/index.spec.ts @@ -3,7 +3,7 @@ import * as fs from "fs"; import * as path from "path"; import * as ts from "typescript"; -import { CompilerOptions, LuaLibImportKind, HoistingMode } from "../../../../../src/CompilerOptions"; +import { CompilerOptions, LuaLibImportKind } from "../../../../../src/CompilerOptions"; import { optionDeclarations, parseCommandLine } from "../../../../../src/CommandLineParser"; export class MixedConfigurationTests { @@ -35,7 +35,7 @@ export class MixedConfigurationTests { // Only present in TSTL dfaults noHeader: optionDeclarations["noHeader"].default, project: tsConfigPath, - hoisting: HoistingMode.None, + noHoisting: optionDeclarations["noHoisting"].default, } as CompilerOptions); } } diff --git a/test/unit/hoisting.spec.ts b/test/unit/hoisting.spec.ts index 314dfd7e9..8c3ebcaf7 100644 --- a/test/unit/hoisting.spec.ts +++ b/test/unit/hoisting.spec.ts @@ -2,24 +2,17 @@ import * as ts from "typescript"; import { Expect, Test, TestCase } from "alsatian"; import * as util from "../src/util"; -import { CompilerOptions, LuaLibImportKind, LuaTarget, HoistingMode } from "../../src/CompilerOptions"; +import { CompilerOptions, LuaLibImportKind, LuaTarget } from "../../src/CompilerOptions"; import { TranspileError } from "../../src/TranspileError"; export class HoistingTests { - private static readonly hoistingCompilerOptions: CompilerOptions = { - hoisting: HoistingMode.Required, - luaLibImport: LuaLibImportKind.Require, - luaTarget: LuaTarget.Lua53, - target: ts.ScriptTarget.ES2015, - }; - @Test("Var Hoisting") public varHoisting(): void { const code = `foo = "foo"; var foo; return foo;`; - const result = util.transpileAndExecute(code, HoistingTests.hoistingCompilerOptions); + const result = util.transpileAndExecute(code); Expect(result).toBe("foo"); } @@ -42,7 +35,7 @@ export class HoistingTests { ${varType} foo = "foo"; setBar(); return foo;`; - const result = util.transpileAndExecute(code, HoistingTests.hoistingCompilerOptions); + const result = util.transpileAndExecute(code); Expect(result).toBe("foo"); } @@ -65,7 +58,7 @@ export class HoistingTests { `const foo = bar(); function bar() { return "bar"; } return foo;`; - const result = util.transpileAndExecute(code, HoistingTests.hoistingCompilerOptions); + const result = util.transpileAndExecute(code); Expect(result).toBe("bar"); } @@ -126,7 +119,7 @@ export class HoistingTests { return bar; } return foo();`; - const result = util.transpileAndExecute(code, HoistingTests.hoistingCompilerOptions); + const result = util.transpileAndExecute(code); Expect(result).toBe(expectResult); } @@ -221,7 +214,7 @@ export class HoistingTests { @Test("No Hoisting") public noHoisting(code: string, identifier: string): void { const compilerOptions: CompilerOptions = { - hoisting: HoistingMode.None, + noHoisting: true, luaLibImport: LuaLibImportKind.Require, luaTarget: LuaTarget.Lua53, target: ts.ScriptTarget.ES2015, From 1a846b82e64a870f8a03566690b8b8f25ecd3295 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Mon, 4 Feb 2019 06:49:18 -0700 Subject: [PATCH 19/24] reworked hoisting to avoid printer hacks --- src/LuaAST.ts | 6 +- src/LuaPrinter.ts | 9 +-- src/LuaTransformer.ts | 169 +++++++++++++++++++++++++----------------- 3 files changed, 104 insertions(+), 80 deletions(-) diff --git a/src/LuaAST.ts b/src/LuaAST.ts index 5975be2d1..8b32e7013 100644 --- a/src/LuaAST.ts +++ b/src/LuaAST.ts @@ -190,7 +190,6 @@ export interface VariableDeclarationStatement extends Statement { kind: SyntaxKind.VariableDeclarationStatement; left: Identifier[]; right?: Expression[]; - local: boolean; } export function isVariableDeclarationStatement(node: Node): node is VariableDeclarationStatement { @@ -201,8 +200,7 @@ export function createVariableDeclarationStatement( left: Identifier | Identifier[], right?: Expression | Expression[], tsOriginal?: ts.Node, - parent?: Node, - local = true + parent?: Node ): VariableDeclarationStatement { const statement = createNode( @@ -222,7 +220,6 @@ export function createVariableDeclarationStatement( } else if (right) { statement.right = [right]; } - statement.local = local; return statement; } @@ -231,7 +228,6 @@ export interface AssignmentStatement extends Statement { kind: SyntaxKind.AssignmentStatement; left: IdentifierOrTableIndexExpression[]; right: Expression[]; - hoisted?: boolean; } export function isAssignmentStatement(node: Node): node is AssignmentStatement { diff --git a/src/LuaPrinter.ts b/src/LuaPrinter.ts index c9b8578c9..0f509c593 100644 --- a/src/LuaPrinter.ts +++ b/src/LuaPrinter.ts @@ -128,11 +128,7 @@ export class LuaPrinter { } private printVariableDeclarationStatement(statement: tstl.VariableDeclarationStatement): string { - if (!statement.local && !statement.right) { - return ""; - } - const prefix = statement.local ? "local " : ""; - const left = this.indent(`${prefix}${statement.left.map(e => this.printExpression(e)).join(", ")}`); + const left = this.indent(`local ${statement.left.map(e => this.printExpression(e)).join(", ")}`); if (statement.right) { return left + ` = ${statement.right.map(e => this.printExpression(e)).join(", ")};\n`; } else { @@ -141,9 +137,6 @@ export class LuaPrinter { } private printVariableAssignmentStatement(statement: tstl.AssignmentStatement): string { - if (statement.hoisted) { - return ""; - } return this.indent( `${statement.left.map(e => this.printExpression(e)).join(", ")} = ` + `${statement.right.map(e => this.printExpression(e)).join(", ")};\n`); diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 5eebd8f1b..7a67d50a3 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -7,6 +7,7 @@ import * as tstl from "./LuaAST"; import {LuaLib, LuaLibFeature} from "./LuaLib"; import {ContextType, TSHelper as tsHelper} from "./TSHelper"; import {TSTLErrors} from "./TSTLErrors"; +import { isVariableDeclaration } from "typescript"; export type StatementVisitResult = tstl.Statement | tstl.Statement[] | undefined; export type ExpressionVisitResult = tstl.Expression | undefined; @@ -98,9 +99,8 @@ export class LuaTransformer { this.currentSourceFile = node; this.isModule = tsHelper.isFileModule(node); - const statements = this.transformStatements(node.statements); - const [, hoistedStatements] = this.popScope(); - statements.unshift(...hoistedStatements); + const statements = this.performHoisting(this.transformStatements(node.statements)); + this.popScope(); if (this.isModule) { statements.unshift( @@ -194,17 +194,15 @@ export class LuaTransformer { public transformBlock(block: ts.Block): tstl.Block { this.pushScope(ScopeType.Block, block); - const statements = this.transformStatements(block.statements); - const [, hoistedStatements] = this.popScope(); - statements.unshift(...hoistedStatements); + const statements = this.performHoisting(this.transformStatements(block.statements)); + this.popScope(); return tstl.createBlock(statements, block); } public transformBlockAsDoStatement(block: ts.Block): tstl.DoStatement { this.pushScope(ScopeType.Block, block); - const statements = this.transformStatements(block.statements); - const [, hoistedStatements] = this.popScope(); - statements.unshift(...hoistedStatements); + const statements = this.performHoisting(this.transformStatements(block.statements)); + this.popScope(); return tstl.createDoStatement(statements, block); } @@ -247,14 +245,14 @@ export class LuaTransformer { if (importSpecifier.propertyName) { const propertyIdentifier = this.transformIdentifier(importSpecifier.propertyName); const propertyName = tstl.createStringLiteral(propertyIdentifier.text); - const renamedImport = this.createHoistedVariableDeclarationStatement( + const renamedImport = this.createHoistableVariableDeclarationStatement( importSpecifier.name, tstl.createTableIndexExpression(importUniqueName, propertyName), importSpecifier); result.push(renamedImport); } else { const name = tstl.createStringLiteral(importSpecifier.name.text); - const namedImport = this.createHoistedVariableDeclarationStatement( + const namedImport = this.createHoistableVariableDeclarationStatement( importSpecifier.name, tstl.createTableIndexExpression(importUniqueName, name), importSpecifier @@ -264,7 +262,7 @@ export class LuaTransformer { }); return result; } else if (ts.isNamespaceImport(imports)) { - const requireStatement = this.createHoistedVariableDeclarationStatement( + const requireStatement = this.createHoistableVariableDeclarationStatement( imports.name, requireCall, statement @@ -796,10 +794,9 @@ export class LuaTransformer { headerStatements.push(tstl.createVariableDeclarationStatement(spreadIdentifier, spreadTable)); } - const bodyStatements = this.transformStatements(body.statements); + const bodyStatements = this.performHoisting(this.transformStatements(body.statements)); - const [scope, hoistedStatements] = this.popScope(); - bodyStatements.unshift(...hoistedStatements); + const scope = this.popScope(); return [headerStatements.concat(bodyStatements), scope]; } @@ -896,7 +893,7 @@ export class LuaTransformer { result.push(namespaceDeclaration); // local innerNS = outerNS.innerNS - const localDeclaration = this.createHoistedVariableDeclarationStatement( + const localDeclaration = this.createHoistableVariableDeclarationStatement( statement.name as ts.Identifier, tstl.createTableIndexExpression( this.transformIdentifier(this.currentNamespace.name as ts.Identifier), @@ -916,7 +913,7 @@ export class LuaTransformer { result.push(namespaceDeclaration); // local NS = exports.NS - const localDeclaration = this.createHoistedVariableDeclarationStatement( + const localDeclaration = this.createHoistableVariableDeclarationStatement( statement.name as ts.Identifier, this.createExportedIdentifier(this.transformIdentifier(statement.name as ts.Identifier))); @@ -944,9 +941,8 @@ export class LuaTransformer { // Transform moduleblock to block and visit it if (statement.body && ts.isModuleBlock(statement.body)) { this.pushScope(ScopeType.Block, statement); - const statements = this.transformStatements(statement.body.statements); - const [, hoistedStatements] = this.popScope(); - statements.unshift(...hoistedStatements); + const statements = this.performHoisting(this.transformStatements(statement.body.statements)); + this.popScope(); result.push(tstl.createDoStatement(statements)); } @@ -1258,18 +1254,16 @@ export class LuaTransformer { public transformIfStatement(statement: ts.IfStatement): tstl.IfStatement { this.pushScope(ScopeType.Conditional, statement.thenStatement); const condition = this.transformExpression(statement.expression); - const statements = this.transformBlockOrStatement(statement.thenStatement); - const [, hoistedStatements] = this.popScope(); - statements.unshift(...hoistedStatements); + const statements = this.performHoisting(this.transformBlockOrStatement(statement.thenStatement)); + this.popScope(); const ifBlock = tstl.createBlock(statements); if (statement.elseStatement) { if (ts.isIfStatement(statement.elseStatement)) { return tstl.createIfStatement(condition, ifBlock, this.transformIfStatement(statement.elseStatement)); } else { this.pushScope(ScopeType.Conditional, statement.elseStatement); - const elseStatements = this.transformBlockOrStatement(statement.elseStatement); - const [, hoistedStatements] = this.popScope(); - elseStatements.unshift(...hoistedStatements); + const elseStatements = this.performHoisting(this.transformBlockOrStatement(statement.elseStatement)); + this.popScope(); const elseBlock = tstl.createBlock(elseStatements); return tstl.createIfStatement(condition, ifBlock, elseBlock); } @@ -1354,9 +1348,8 @@ export class LuaTransformer { ): tstl.Statement[] { this.pushScope(ScopeType.Loop, loop.statement); - const body = this.transformBlockOrStatement(loop.statement); - const [scope, hoistedStatements] = this.popScope(); - body.unshift(...hoistedStatements); + const body = this.performHoisting(this.transformBlockOrStatement(loop.statement)); + const scope = this.popScope(); const scopeId = scope.id; if (this.options.luaTarget === LuaTarget.Lua51) { @@ -1567,7 +1560,7 @@ export class LuaTransformer { const switchVariable = tstl.createIdentifier(switchName); const switchVariableDeclaration = tstl.createVariableDeclarationStatement(switchVariable, expression); - const statements: tstl.Statement[] = [switchVariableDeclaration]; + let statements: tstl.Statement[] = [switchVariableDeclaration]; const caseClauses = statement.caseBlock.clauses.filter(c => ts.isCaseClause(c)) as ts.CaseClause[]; @@ -1603,8 +1596,8 @@ export class LuaTransformer { statements.push(tstl.createLabelStatement(`${switchName}_end`)); - const [, hoistedStatements] = this.popScope(); - statements.unshift(...hoistedStatements); + statements = this.performHoisting(statements); + this.popScope(); return statements; } @@ -3522,12 +3515,10 @@ export class LuaTransformer { } if (scope.functionDefinitions) { - for (const functionSymbolAndInfo of scope.functionDefinitions) { - const functionSymbol = functionSymbolAndInfo[0]; - const functionInfo = functionSymbolAndInfo[1]; - if (functionSymbol !== symbol - && symbolInfo.firstSeenAtPos < functionInfo.assignment.pos - && functionInfo.referencedSymbols.has(symbol) + for (const [functionSymbol, functionDefinition] of scope.functionDefinitions) { + if (functionSymbol !== symbol // Don't recurse into self + && declaration.pos < functionDefinition.assignment.pos // Ignore functions before symbol declaration + && functionDefinition.referencedSymbols.has(symbol) && this.shouldHoist(functionSymbol, scope)) { return true; @@ -3538,43 +3529,85 @@ export class LuaTransformer { return false; } - protected popScope(): [Scope, tstl.Statement[]] { - const scope = this.scopeStack.pop(); - - const hoistedStatements: tstl.Statement[] = []; + protected replaceStatementInParent(oldNode: tstl.Statement, newNode?: tstl.Statement): void { + if (!oldNode.parent) { + throw new Error("node has not yet been assigned a parent"); + } - // Hoist variable declarations - if (scope.variableDeclarations) { - for (const variableInfo of scope.variableDeclarations) { - if (variableInfo.symbols.some(s => this.shouldHoist(s, scope))) { - hoistedStatements.push(tstl.createVariableDeclarationStatement(variableInfo.declaration.left)); - variableInfo.declaration.local = false; - } + if (tstl.isBlock(oldNode.parent) || tstl.isDoStatement(oldNode.parent)) { + if (newNode) { + oldNode.parent.statements.splice(oldNode.parent.statements.indexOf(oldNode), 1, newNode); + } else { + oldNode.parent.statements.splice(oldNode.parent.statements.indexOf(oldNode), 1); } + } else { + throw new Error("unexpected parent type"); } + } + + protected performHoisting(statements: tstl.Statement[]): tstl.Statement[] { + if (this.options.noHoisting) { + return statements; + } + + const scope = this.peekScope(); + const result = statements.slice(); // Hoist function definitions if (scope.functionDefinitions) { - for (const functionSymbolAndInfo of scope.functionDefinitions) { - const functionSymbol = functionSymbolAndInfo[0]; - const functionInfo = functionSymbolAndInfo[1]; + const hoistedFunctions: tstl.AssignmentStatement[] = []; + for (const [functionSymbol, functionDefinition] of scope.functionDefinitions) { if (this.shouldHoist(functionSymbol, scope)) { - const statement = tstl.createAssignmentStatement( - functionInfo.assignment.left, - functionInfo.assignment.right, - undefined, - functionInfo.assignment.parent - ); - hoistedStatements.push(statement); - functionInfo.assignment.hoisted = true; + const i = result.indexOf(functionDefinition.assignment); + result.splice(i, 1); + hoistedFunctions.push(functionDefinition.assignment); + } + } + if (hoistedFunctions.length > 0) { + result.unshift(...hoistedFunctions); + } + } + + // Hoist variable declarations + if (scope.variableDeclarations) { + const hoistedLocals: tstl.Identifier[] = []; + for (const variableDeclaration of scope.variableDeclarations) { + if (variableDeclaration.symbols.some(s => this.shouldHoist(s, scope))) { + let assignment: tstl.AssignmentStatement | undefined; + if (variableDeclaration.declaration.right) { + assignment = tstl.createAssignmentStatement( + variableDeclaration.declaration.left, + variableDeclaration.declaration.right + ); + } + const i = result.indexOf(variableDeclaration.declaration); + if (i >= 0) { + if (assignment) { + result.splice(i, 1, assignment); + } else { + result.splice(i, 1); + } + } else { + // Special case for 'var's declared in child scopes + this.replaceStatementInParent(variableDeclaration.declaration, assignment); + } + hoistedLocals.push(...variableDeclaration.declaration.left); } } + if (hoistedLocals.length > 0) { + result.unshift(tstl.createVariableDeclarationStatement(hoistedLocals)); + } } - return [scope, hoistedStatements]; + return result; + } + + protected popScope(): Scope { + const scope = this.scopeStack.pop(); + return scope; } - protected createHoistedVariableDeclarationStatement( + protected createHoistableVariableDeclarationStatement( identifier: ts.Identifier, initializer?: tstl.Expression, tsOriginal?: ts.Node, @@ -3583,11 +3616,13 @@ export class LuaTransformer { { const variable = this.transformIdentifier(identifier); const declaration = tstl.createVariableDeclarationStatement(variable, initializer, tsOriginal, parent); - const symbol = this.checker.getSymbolAtLocation(identifier); - if (symbol) { - const scope = this.peekScope(); - if (!scope.variableDeclarations) { scope.variableDeclarations = []; } - scope.variableDeclarations.push({symbols: [symbol], declaration}); + if (!this.options.noHoisting) { + const symbol = this.checker.getSymbolAtLocation(identifier); + if (symbol) { + const scope = this.peekScope(); + if (!scope.variableDeclarations) { scope.variableDeclarations = []; } + scope.variableDeclarations.push({symbols: [symbol], declaration}); + } } return declaration; } From 798bb27ba90f23810379d886f9b2ac23260c925e Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Mon, 4 Feb 2019 14:08:44 -0700 Subject: [PATCH 20/24] removing unnecessary symbol lookups --- src/LuaTransformer.ts | 61 +++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 37 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 7a67d50a3..e4750d0e6 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -25,11 +25,6 @@ interface SymbolInfo { firstSeenAtPos: number; } -interface VariableDeclarationInfo { - symbols: ts.Symbol[]; - declaration: tstl.VariableDeclarationStatement; -} - interface FunctionDefinitionInfo { referencedSymbols: Set; assignment?: tstl.AssignmentStatement; @@ -39,7 +34,7 @@ interface Scope { type: ScopeType; id: number; referencedSymbols?: Set; - variableDeclarations?: VariableDeclarationInfo[]; + variableDeclarations?: tstl.VariableDeclarationStatement[]; functionDefinitions?: Map; } @@ -1053,14 +1048,11 @@ export class LuaTransformer { const functionExpression = tstl.createFunctionExpression(block, params, dotsLiteral, restParamName); // Remember symbols referenced in this function for hoisting later - if (!this.options.noHoisting && functionDeclaration.name) { - const symbol = this.checker.getSymbolAtLocation(functionDeclaration.name); - if (symbol) { - const scope = this.peekScope(); - if (!scope.functionDefinitions) { scope.functionDefinitions = new Map(); } - const functionInfo = {referencedSymbols: functionScope.referencedSymbols || new Set()}; - scope.functionDefinitions.set(symbol, functionInfo); - } + if (!this.options.noHoisting && name.symbol) { + const scope = this.peekScope(); + if (!scope.functionDefinitions) { scope.functionDefinitions = new Map(); } + const functionInfo = {referencedSymbols: functionScope.referencedSymbols || new Set()}; + scope.functionDefinitions.set(name.symbol, functionInfo); } return this.createLocalOrExportedOrGlobalDeclaration(name, functionExpression, functionDeclaration); @@ -3359,16 +3351,13 @@ export class LuaTransformer { } if (!this.options.noHoisting) { - let symbols = Array.isArray(lhs) ? lhs.map(i => i.symbol) : [lhs.symbol]; - symbols = symbols.filter(i => i !== undefined); - + // Remember local variable declarations for hoisting later const scope = isLetOrConst || functionDeclaration ? this.peekScope() : this.findScope(ScopeType.Function | ScopeType.File); - // Remember local variable declarations for hoisting later if (!scope.variableDeclarations) { scope.variableDeclarations = []; } - scope.variableDeclarations.push({declaration, symbols}); + scope.variableDeclarations.push(declaration); } } else if (rhs) { @@ -3380,9 +3369,9 @@ export class LuaTransformer { } } - // Remember function definitions for hoisting later - if (!this.options.noHoisting && functionDeclaration && functionDeclaration.name) { - const functionSymbol = this.checker.getSymbolAtLocation(functionDeclaration.name); + if (!this.options.noHoisting && functionDeclaration) { + // Remember function definitions for hoisting later + const functionSymbol = (lhs as tstl.Identifier).symbol; if (functionSymbol) { this.peekScope().functionDefinitions.get(functionSymbol).assignment = assignment; } @@ -3571,16 +3560,17 @@ export class LuaTransformer { // Hoist variable declarations if (scope.variableDeclarations) { const hoistedLocals: tstl.Identifier[] = []; - for (const variableDeclaration of scope.variableDeclarations) { - if (variableDeclaration.symbols.some(s => this.shouldHoist(s, scope))) { + for (const declaration of scope.variableDeclarations) { + const symbols = declaration.left.map(i => i.symbol); + if (symbols.some(s => this.shouldHoist(s, scope))) { let assignment: tstl.AssignmentStatement | undefined; - if (variableDeclaration.declaration.right) { + if (declaration.right) { assignment = tstl.createAssignmentStatement( - variableDeclaration.declaration.left, - variableDeclaration.declaration.right + declaration.left, + declaration.right ); } - const i = result.indexOf(variableDeclaration.declaration); + const i = result.indexOf(declaration); if (i >= 0) { if (assignment) { result.splice(i, 1, assignment); @@ -3589,9 +3579,9 @@ export class LuaTransformer { } } else { // Special case for 'var's declared in child scopes - this.replaceStatementInParent(variableDeclaration.declaration, assignment); + this.replaceStatementInParent(declaration, assignment); } - hoistedLocals.push(...variableDeclaration.declaration.left); + hoistedLocals.push(...declaration.left); } } if (hoistedLocals.length > 0) { @@ -3616,13 +3606,10 @@ export class LuaTransformer { { const variable = this.transformIdentifier(identifier); const declaration = tstl.createVariableDeclarationStatement(variable, initializer, tsOriginal, parent); - if (!this.options.noHoisting) { - const symbol = this.checker.getSymbolAtLocation(identifier); - if (symbol) { - const scope = this.peekScope(); - if (!scope.variableDeclarations) { scope.variableDeclarations = []; } - scope.variableDeclarations.push({symbols: [symbol], declaration}); - } + if (!this.options.noHoisting && variable.symbol) { + const scope = this.peekScope(); + if (!scope.variableDeclarations) { scope.variableDeclarations = []; } + scope.variableDeclarations.push(declaration); } return declaration; } From c9c564bed8e160d8c504062a595011bc0d32f7d8 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Mon, 4 Feb 2019 14:29:50 -0700 Subject: [PATCH 21/24] removing accidentally added import --- src/LuaTransformer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 50ecffdff..f5e2ab345 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -7,7 +7,6 @@ import * as tstl from "./LuaAST"; import {LuaLib, LuaLibFeature} from "./LuaLib"; import {ContextType, TSHelper as tsHelper} from "./TSHelper"; import {TSTLErrors} from "./TSTLErrors"; -import { isVariableDeclaration } from "typescript"; export type StatementVisitResult = tstl.Statement | tstl.Statement[] | undefined; export type ExpressionVisitResult = tstl.Expression | undefined; From d6e83e2735d91001487deb61ed9aa2b35adeecf4 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Tue, 5 Feb 2019 07:06:01 -0700 Subject: [PATCH 22/24] replaced ts.Symbol stored in identifiers with generic ids --- src/LuaAST.ts | 8 +++--- src/LuaTransformer.ts | 60 ++++++++++++++++++++++++++----------------- 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/src/LuaAST.ts b/src/LuaAST.ts index 8b32e7013..424d09057 100644 --- a/src/LuaAST.ts +++ b/src/LuaAST.ts @@ -97,6 +97,8 @@ export type BinaryOperator = export type Operator = UnaryOperator | BinaryOperator; +export type SymbolId = number; + // TODO For future sourcemap support? export interface TextRange { pos: number; @@ -791,7 +793,7 @@ export function createMethodCallExpression( export interface Identifier extends Expression { kind: SyntaxKind.Identifier; text: string; - symbol?: ts.Symbol; + symbolId?: SymbolId; } export function isIdentifier(node: Node): node is Identifier { @@ -801,13 +803,13 @@ export function isIdentifier(node: Node): node is Identifier { export function createIdentifier( text: string | ts.__String, tsOriginal?: ts.Node, - symbol?: ts.Symbol, + symbolId?: SymbolId, parent?: Node ): Identifier { const expression = createNode(SyntaxKind.Identifier, tsOriginal, parent) as Identifier; expression.text = text as string; - expression.symbol = symbol; + expression.symbolId = symbolId; return expression; } diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index f5e2ab345..823f52030 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -21,20 +21,21 @@ export enum ScopeType { } interface SymbolInfo { + symbol: ts.Symbol; firstSeenAtPos: number; } interface FunctionDefinitionInfo { - referencedSymbols: Set; + referencedSymbols: Set; assignment?: tstl.AssignmentStatement; } interface Scope { type: ScopeType; id: number; - referencedSymbols?: Set; + referencedSymbols?: Set; variableDeclarations?: tstl.VariableDeclarationStatement[]; - functionDefinitions?: Map; + functionDefinitions?: Map; } export class LuaTransformer { @@ -58,7 +59,10 @@ export class LuaTransformer { private genVarCounter: number; private luaLibFeatureSet: Set; - private symbolInfo: Map; + + private symbolInfo: Map; + private symbolIds: Map; + private genSymbolIdCounter: number; private readonly typeValidationCache: Map> = new Map>(); @@ -82,7 +86,9 @@ export class LuaTransformer { this.scopeStack = []; this.classStack = []; this.luaLibFeatureSet = new Set(); + this.symbolIds = new Map(); this.symbolInfo = new Map(); + this.genSymbolIdCounter = 1; } // TODO make all other methods private??? @@ -1047,11 +1053,11 @@ export class LuaTransformer { const functionExpression = tstl.createFunctionExpression(block, params, dotsLiteral, restParamName); // Remember symbols referenced in this function for hoisting later - if (!this.options.noHoisting && name.symbol) { + if (!this.options.noHoisting && name.symbolId !== undefined) { const scope = this.peekScope(); if (!scope.functionDefinitions) { scope.functionDefinitions = new Map(); } const functionInfo = {referencedSymbols: functionScope.referencedSymbols || new Set()}; - scope.functionDefinitions.set(name.symbol, functionInfo); + scope.functionDefinitions.set(name.symbolId, functionInfo); } return this.createLocalOrExportedOrGlobalDeclaration(name, functionExpression, functionDeclaration); @@ -3149,6 +3155,7 @@ export class LuaTransformer { } const symbol = this.checker.getSymbolAtLocation(expression); + let symbolId: number | undefined; if (symbol) { if (this.options.noHoisting) { // Check for reference-before-declaration @@ -3159,12 +3166,17 @@ export class LuaTransformer { } else { // Track symbols seen in scope - if (!this.symbolInfo.has(symbol)) { - this.symbolInfo.set(symbol, {firstSeenAtPos: expression.pos}); + if (!this.symbolIds.has(symbol)) { + symbolId = this.genSymbolIdCounter++; + const symbolInfo: SymbolInfo = {symbol, firstSeenAtPos: expression.pos}; + this.symbolIds.set(symbol, symbolId); + this.symbolInfo.set(symbolId, symbolInfo); + } else { + symbolId = this.symbolIds.get(symbol); } this.scopeStack.forEach(s => { if (!s.referencedSymbols) { s.referencedSymbols = new Set(); } - s.referencedSymbols.add(symbol); + s.referencedSymbols.add(symbolId); }); } } @@ -3179,7 +3191,7 @@ export class LuaTransformer { if (this.luaKeywords.has(escapedText)) { throw TSTLErrors.KeywordIdentifier(expression); } - return tstl.createIdentifier(escapedText, expression, symbol); + return tstl.createIdentifier(escapedText, expression, symbolId); } public transformIdentifierExpression(expression: ts.Identifier): tstl.IdentifierOrTableIndexExpression { @@ -3389,9 +3401,9 @@ export class LuaTransformer { if (!this.options.noHoisting && functionDeclaration) { // Remember function definitions for hoisting later - const functionSymbol = (lhs as tstl.Identifier).symbol; - if (functionSymbol) { - this.peekScope().functionDefinitions.get(functionSymbol).assignment = assignment; + const functionSymbolId = (lhs as tstl.Identifier).symbolId; + if (functionSymbolId !== undefined) { + this.peekScope().functionDefinitions.get(functionSymbolId).assignment = assignment; } } @@ -3506,13 +3518,13 @@ export class LuaTransformer { this.genVarCounter++; } - private shouldHoist(symbol: ts.Symbol, scope: Scope): boolean { - const symbolInfo = this.symbolInfo.get(symbol); + private shouldHoist(symbolId: tstl.SymbolId, scope: Scope): boolean { + const symbolInfo = this.symbolInfo.get(symbolId); if (!symbolInfo) { return false; } - const declaration = tsHelper.getFirstDeclaration(symbol, this.currentSourceFile); + const declaration = tsHelper.getFirstDeclaration(symbolInfo.symbol, this.currentSourceFile); if (!declaration) { return false; } @@ -3522,11 +3534,11 @@ export class LuaTransformer { } if (scope.functionDefinitions) { - for (const [functionSymbol, functionDefinition] of scope.functionDefinitions) { - if (functionSymbol !== symbol // Don't recurse into self + for (const [functionSymbolId, functionDefinition] of scope.functionDefinitions) { + if (functionSymbolId !== symbolId // Don't recurse into self && declaration.pos < functionDefinition.assignment.pos // Ignore functions before symbol declaration - && functionDefinition.referencedSymbols.has(symbol) - && this.shouldHoist(functionSymbol, scope)) + && functionDefinition.referencedSymbols.has(symbolId) + && this.shouldHoist(functionSymbolId, scope)) { return true; } @@ -3563,8 +3575,8 @@ export class LuaTransformer { // Hoist function definitions if (scope.functionDefinitions) { const hoistedFunctions: tstl.AssignmentStatement[] = []; - for (const [functionSymbol, functionDefinition] of scope.functionDefinitions) { - if (this.shouldHoist(functionSymbol, scope)) { + for (const [functionSymbolId, functionDefinition] of scope.functionDefinitions) { + if (this.shouldHoist(functionSymbolId, scope)) { const i = result.indexOf(functionDefinition.assignment); result.splice(i, 1); hoistedFunctions.push(functionDefinition.assignment); @@ -3579,7 +3591,7 @@ export class LuaTransformer { if (scope.variableDeclarations) { const hoistedLocals: tstl.Identifier[] = []; for (const declaration of scope.variableDeclarations) { - const symbols = declaration.left.map(i => i.symbol); + const symbols = declaration.left.map(i => i.symbolId).filter(s => s !== undefined); if (symbols.some(s => this.shouldHoist(s, scope))) { let assignment: tstl.AssignmentStatement | undefined; if (declaration.right) { @@ -3624,7 +3636,7 @@ export class LuaTransformer { { const variable = this.transformIdentifier(identifier); const declaration = tstl.createVariableDeclarationStatement(variable, initializer, tsOriginal, parent); - if (!this.options.noHoisting && variable.symbol) { + if (!this.options.noHoisting && variable.symbolId) { const scope = this.peekScope(); if (!scope.variableDeclarations) { scope.variableDeclarations = []; } scope.variableDeclarations.push(declaration); From 922380f992eb40194ea46ad7c00c3621a836fd29 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Tue, 5 Feb 2019 15:37:35 -0700 Subject: [PATCH 23/24] split out stuff to separate functions --- src/LuaTransformer.ts | 158 ++++++++++++++++++++++++------------------ 1 file changed, 89 insertions(+), 69 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 823f52030..d8dca7114 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -3154,33 +3154,6 @@ export class LuaTransformer { // at some point. } - const symbol = this.checker.getSymbolAtLocation(expression); - let symbolId: number | undefined; - if (symbol) { - if (this.options.noHoisting) { - // Check for reference-before-declaration - const declaration = tsHelper.getFirstDeclaration(symbol, this.currentSourceFile); - if (declaration && expression.pos < declaration.pos) { - throw TSTLErrors.ReferencedBeforeDeclaration(expression); - } - - } else { - // Track symbols seen in scope - if (!this.symbolIds.has(symbol)) { - symbolId = this.genSymbolIdCounter++; - const symbolInfo: SymbolInfo = {symbol, firstSeenAtPos: expression.pos}; - this.symbolIds.set(symbol, symbolId); - this.symbolInfo.set(symbolId, symbolInfo); - } else { - symbolId = this.symbolIds.get(symbol); - } - this.scopeStack.forEach(s => { - if (!s.referencedSymbols) { s.referencedSymbols = new Set(); } - s.referencedSymbols.add(symbolId); - }); - } - } - let escapedText = expression.escapedText as string; const underScoreCharCode = "_".charCodeAt(0); if (escapedText.length >= 3 && escapedText.charCodeAt(0) === underScoreCharCode && @@ -3191,6 +3164,8 @@ export class LuaTransformer { if (this.luaKeywords.has(escapedText)) { throw TSTLErrors.KeywordIdentifier(expression); } + + const symbolId = this.getIdentifierSymbolId(expression); return tstl.createIdentifier(escapedText, expression, symbolId); } @@ -3502,6 +3477,38 @@ export class LuaTransformer { return tstl.createBinaryExpression(expression, tstl.createNumericLiteral(1), tstl.SyntaxKind.AdditionOperator); } + private getIdentifierSymbolId(identifier: ts.Identifier): tstl.SymbolId { + const symbol = this.checker.getSymbolAtLocation(identifier); + let symbolId: number | undefined; + if (symbol) { + if (this.options.noHoisting) { + // Check for reference-before-declaration + const declaration = tsHelper.getFirstDeclaration(symbol, this.currentSourceFile); + if (declaration && identifier.pos < declaration.pos) { + throw TSTLErrors.ReferencedBeforeDeclaration(identifier); + } + + } else { + // Track first time symbols are seen + if (!this.symbolIds.has(symbol)) { + symbolId = this.genSymbolIdCounter++; + const symbolInfo: SymbolInfo = {symbol, firstSeenAtPos: identifier.pos}; + this.symbolIds.set(symbol, symbolId); + this.symbolInfo.set(symbolId, symbolInfo); + } else { + symbolId = this.symbolIds.get(symbol); + } + + //Mark symbol as seen in all current scopes + this.scopeStack.forEach(s => { + if (!s.referencedSymbols) { s.referencedSymbols = new Set(); } + s.referencedSymbols.add(symbolId); + }); + } + } + return symbolId; + } + protected findScope(scopeTypes: ScopeType): Scope | undefined { return this.scopeStack.slice().reverse().find(s => (scopeTypes & s.type) !== 0); } @@ -3564,60 +3571,73 @@ export class LuaTransformer { } } - protected performHoisting(statements: tstl.Statement[]): tstl.Statement[] { - if (this.options.noHoisting) { + protected hoistFunctionDefinitions(scope: Scope, statements: tstl.Statement[]): tstl.Statement[] { + if (!scope.functionDefinitions) { return statements; } - const scope = this.peekScope(); const result = statements.slice(); - - // Hoist function definitions - if (scope.functionDefinitions) { - const hoistedFunctions: tstl.AssignmentStatement[] = []; - for (const [functionSymbolId, functionDefinition] of scope.functionDefinitions) { - if (this.shouldHoist(functionSymbolId, scope)) { - const i = result.indexOf(functionDefinition.assignment); - result.splice(i, 1); - hoistedFunctions.push(functionDefinition.assignment); - } - } - if (hoistedFunctions.length > 0) { - result.unshift(...hoistedFunctions); + const hoistedFunctions: tstl.AssignmentStatement[] = []; + for (const [functionSymbolId, functionDefinition] of scope.functionDefinitions) { + if (this.shouldHoist(functionSymbolId, scope)) { + const i = result.indexOf(functionDefinition.assignment); + result.splice(i, 1); + hoistedFunctions.push(functionDefinition.assignment); } } + if (hoistedFunctions.length > 0) { + result.unshift(...hoistedFunctions); + } + return result; + } - // Hoist variable declarations - if (scope.variableDeclarations) { - const hoistedLocals: tstl.Identifier[] = []; - for (const declaration of scope.variableDeclarations) { - const symbols = declaration.left.map(i => i.symbolId).filter(s => s !== undefined); - if (symbols.some(s => this.shouldHoist(s, scope))) { - let assignment: tstl.AssignmentStatement | undefined; - if (declaration.right) { - assignment = tstl.createAssignmentStatement( - declaration.left, - declaration.right - ); - } - const i = result.indexOf(declaration); - if (i >= 0) { - if (assignment) { - result.splice(i, 1, assignment); - } else { - result.splice(i, 1); - } + protected hoistVariableDeclarations(scope: Scope, statements: tstl.Statement[]): tstl.Statement[] { + if (!scope.variableDeclarations) { + return statements; + } + + const result = statements.slice(); + const hoistedLocals: tstl.Identifier[] = []; + for (const declaration of scope.variableDeclarations) { + const symbols = declaration.left.map(i => i.symbolId).filter(s => s !== undefined); + if (symbols.some(s => this.shouldHoist(s, scope))) { + let assignment: tstl.AssignmentStatement | undefined; + if (declaration.right) { + assignment = tstl.createAssignmentStatement( + declaration.left, + declaration.right + ); + } + const i = result.indexOf(declaration); + if (i >= 0) { + if (assignment) { + result.splice(i, 1, assignment); } else { - // Special case for 'var's declared in child scopes - this.replaceStatementInParent(declaration, assignment); + result.splice(i, 1); } - hoistedLocals.push(...declaration.left); + } else { + // Special case for 'var's declared in child scopes + this.replaceStatementInParent(declaration, assignment); } + hoistedLocals.push(...declaration.left); } - if (hoistedLocals.length > 0) { - result.unshift(tstl.createVariableDeclarationStatement(hoistedLocals)); - } } + if (hoistedLocals.length > 0) { + result.unshift(tstl.createVariableDeclarationStatement(hoistedLocals)); + } + return result; + } + + protected performHoisting(statements: tstl.Statement[]): tstl.Statement[] { + if (this.options.noHoisting) { + return statements; + } + + const scope = this.peekScope(); + + let result = this.hoistFunctionDefinitions(scope, statements); + + result = this.hoistVariableDeclarations(scope, result); return result; } From 01191da456bf9e239d13c49cb69218b7f92e20b2 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Wed, 6 Feb 2019 11:22:48 -0700 Subject: [PATCH 24/24] reverted bad logic which broke referencing exports inside functions --- src/LuaTransformer.ts | 4 ++-- test/unit/class.spec.ts | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index d8dca7114..038cea273 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -3177,7 +3177,7 @@ export class LuaTransformer { } public isIdentifierExported(identifierName: string | ts.__String): boolean { - if (!this.currentNamespace && !(this.isModule && this.peekScope().type === ScopeType.File)) { + if (!this.isModule && !this.currentNamespace) { return false; } const currentScope = this.currentNamespace ? this.currentNamespace : this.currentSourceFile; @@ -3288,7 +3288,7 @@ export class LuaTransformer { } private shouldExportIdentifier(identifier: tstl.Identifier | tstl.Identifier[]): boolean { - if (!this.currentNamespace && !(this.isModule && this.peekScope().type === ScopeType.File)) { + if (!this.isModule && !this.currentNamespace) { return false; } if (Array.isArray(identifier)) { diff --git a/test/unit/class.spec.ts b/test/unit/class.spec.ts index 928770ad6..384d12fdd 100644 --- a/test/unit/class.spec.ts +++ b/test/unit/class.spec.ts @@ -660,4 +660,20 @@ export class ClassTests { Expect(result).toBe(8); } + + @Test("Exported class super call") + public exportedClassSupercAll(): void { + const code = + `export class Foo { + prop: string; + constructor(prop: string) { this.prop = prop; } + } + export class Bar extends Foo { + constructor() { + super("bar"); + } + } + export const baz = (new Bar()).prop;`; + Expect(util.transpileExecuteAndReturnExport(code, "baz")).toBe("bar"); + } }