From da0de8bf0375d2a8c82a82dd7f1ed05134975a4e Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Sat, 21 Aug 2021 03:59:51 -0700 Subject: [PATCH 01/24] feat: switch statements for lua 5.1 --- src/transformation/visitors/break-continue.ts | 2 +- src/transformation/visitors/switch.ts | 89 ++++++++++++------- test/unit/__snapshots__/switch.spec.ts.snap | 45 +++------- test/unit/switch.spec.ts | 10 --- 4 files changed, 69 insertions(+), 77 deletions(-) diff --git a/src/transformation/visitors/break-continue.ts b/src/transformation/visitors/break-continue.ts index 9211d4165..c415a5805 100644 --- a/src/transformation/visitors/break-continue.ts +++ b/src/transformation/visitors/break-continue.ts @@ -8,7 +8,7 @@ import { findScope, ScopeType } from "../utils/scope"; export const transformBreakStatement: FunctionVisitor = (breakStatement, context) => { const breakableScope = findScope(context, ScopeType.Loop | ScopeType.Switch); if (breakableScope?.type === ScopeType.Switch) { - return lua.createGotoStatement(`____switch${breakableScope.id}_end`); + return undefined; } else { return lua.createBreakStatement(breakStatement); } diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index a82ff0bf0..fe1528091 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -1,56 +1,81 @@ import * as ts from "typescript"; -import { LuaTarget } from "../../CompilerOptions"; import * as lua from "../../LuaAST"; import { FunctionVisitor } from "../context"; -import { unsupportedForTarget } from "../utils/diagnostics"; import { performHoisting, popScope, pushScope, ScopeType } from "../utils/scope"; -export const transformSwitchStatement: FunctionVisitor = (statement, context) => { - if (context.luaTarget === LuaTarget.Universal || context.luaTarget === LuaTarget.Lua51) { - context.diagnostics.push(unsupportedForTarget(statement, "Switch statements", LuaTarget.Lua51)); +const containsBreakStatement = (statements: ts.Node[]): boolean => { + for (const s of statements) { + if ( + ts.isSwitchStatement(s) || + ts.isWhileStatement(s) || + ts.isDoStatement(s) || + ts.isForStatement(s) || + ts.isForInStatement(s) || + ts.isForOfStatement(s) + ) { + // Ignore: Break statements are valid as children of these + // statements without breaking the clause + } else if (ts.isBreakStatement(s)) { + return true; + } else if (containsBreakStatement(s.getChildren())) { + return true; + } } + return false; +}; + +export const transformSwitchStatement: FunctionVisitor = (statement, context) => { const scope = pushScope(context, ScopeType.Switch); // Give the switch a unique name to prevent nested switches from acting up. const switchName = `____switch${scope.id}`; const switchVariable = lua.createIdentifier(switchName); - let statements: lua.Statement[] = []; + // Collect the fallthrough bodies for each case as defined by the switch. + const caseBody: lua.Statement[][] = []; + for (let i = 0; i < statement.caseBlock.clauses.length; i++) { + const end = statement.caseBlock.clauses + .slice(i) + .findIndex(clause => containsBreakStatement([...clause.statements])); + caseBody[i] = statement.caseBlock.clauses + .slice(i, end >= 0 ? end + i + 1 : undefined) + .reduce( + (statements, clause) => [ + ...statements, + lua.createDoStatement(context.transformStatements(clause.statements)), + ], + [] + ); + } // Starting from the back, concatenating ifs into one big if/elseif statement - const concatenatedIf = statement.caseBlock.clauses.reduceRight((previousCondition, clause, index) => { - if (ts.isDefaultClause(clause)) { - // Skip default clause here (needs to be included to ensure index lines up with index later) - return previousCondition; - } - - // If the clause condition holds, go to the correct label - const condition = lua.createBinaryExpression( - switchVariable, - context.transformExpression(clause.expression), - lua.SyntaxKind.EqualityOperator - ); + const defaultIndex = statement.caseBlock.clauses.findIndex(c => ts.isDefaultClause(c)); + const concatenatedIf = statement.caseBlock.clauses.reduceRight( + (previousCondition, clause, index) => { + if (ts.isDefaultClause(clause)) { + // Skip default clause here (needs to be included to ensure index lines up with index later) + return previousCondition; + } - const goto = lua.createGotoStatement(`${switchName}_case_${index}`); - return lua.createIfStatement(condition, lua.createBlock([goto]), previousCondition); - }, undefined as lua.IfStatement | undefined); + // If the clause condition holds, go to the correct label + const condition = lua.createBinaryExpression( + switchVariable, + context.transformExpression(clause.expression), + lua.SyntaxKind.EqualityOperator + ); - if (concatenatedIf) { - statements.push(concatenatedIf); - } + return lua.createIfStatement(condition, lua.createBlock(caseBody[index]), previousCondition); + }, + defaultIndex >= 0 ? lua.createBlock(caseBody[defaultIndex]) : undefined + ); - const hasDefaultCase = statement.caseBlock.clauses.some(ts.isDefaultClause); - statements.push(lua.createGotoStatement(`${switchName}_${hasDefaultCase ? "case_default" : "end"}`)); + let statements: lua.Statement[] = []; - for (const [index, clause] of statement.caseBlock.clauses.entries()) { - const labelName = `${switchName}_case_${ts.isCaseClause(clause) ? index : "default"}`; - statements.push(lua.createLabelStatement(labelName)); - statements.push(lua.createDoStatement(context.transformStatements(clause.statements))); + if (concatenatedIf) { + statements.push(concatenatedIf as unknown as lua.IfStatement); } - statements.push(lua.createLabelStatement(`${switchName}_end`)); - statements = performHoisting(context, statements); popScope(context); diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap index 19a763566..3b91d867d 100644 --- a/test/unit/__snapshots__/switch.spec.ts.snap +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -1,52 +1,29 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`switch not allowed in 5.1: code 1`] = ` -"local ____exports = {} -function ____exports.__main(self) - local ____switch3 = \\"abc\\" - goto ____switch3_end - ::____switch3_end:: -end -return ____exports" -`; - -exports[`switch not allowed in 5.1: diagnostics 1`] = `"main.ts(2,9): error TSTL: Switch statements is/are not supported for target Lua 5.1."`; - exports[`switch uses elseif 1`] = ` "local ____exports = {} function ____exports.__main(self) local result = -1 local ____switch3 = 2 if ____switch3 == 0 then - goto ____switch3_case_0 - elseif ____switch3 == 1 then - goto ____switch3_case_1 - elseif ____switch3 == 2 then - goto ____switch3_case_2 - end - goto ____switch3_end - ::____switch3_case_0:: - do do - result = 200 - goto ____switch3_end + do + result = 200 + end end - end - ::____switch3_case_1:: - do + elseif ____switch3 == 1 then do - result = 100 - goto ____switch3_end + do + result = 100 + end end - end - ::____switch3_case_2:: - do + elseif ____switch3 == 2 then do - result = 1 - goto ____switch3_end + do + result = 1 + end end end - ::____switch3_end:: return result end return ____exports" diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index c264850c2..de48d7e8a 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -1,5 +1,3 @@ -import * as tstl from "../../src"; -import { unsupportedForTarget } from "../../src/transformation/utils/diagnostics"; import * as util from "../util"; test.each([0, 1, 2, 3])("switch (%p)", inp => { @@ -288,14 +286,6 @@ test("switch uses elseif", () => { .expectToMatchJsResult(); }); -test("switch not allowed in 5.1", () => { - util.testFunction` - switch ("abc") {} - ` - .setOptions({ luaTarget: tstl.LuaTarget.Lua51 }) - .expectDiagnosticsToMatchSnapshot([unsupportedForTarget.code]); -}); - // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/967 test("switch default case not last - first", () => { util.testFunction` From 272b5ac52782d85c2d8562d1ecbd019d21ca7647 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Sun, 22 Aug 2021 11:48:47 -0700 Subject: [PATCH 02/24] refactor: cleanup & switch with only default --- src/transformation/visitors/switch.ts | 38 ++++++++++++++++----------- test/unit/switch.spec.ts | 11 ++++++++ 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index fe1528091..983efbfec 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -49,14 +49,25 @@ export const transformSwitchStatement: FunctionVisitor = (st ); } - // Starting from the back, concatenating ifs into one big if/elseif statement + let statements: lua.Statement[] = []; + + // Default will either be the only statement, or the else in the if chain const defaultIndex = statement.caseBlock.clauses.findIndex(c => ts.isDefaultClause(c)); - const concatenatedIf = statement.caseBlock.clauses.reduceRight( - (previousCondition, clause, index) => { - if (ts.isDefaultClause(clause)) { - // Skip default clause here (needs to be included to ensure index lines up with index later) - return previousCondition; - } + const defaultBody = defaultIndex >= 0 ? caseBody[defaultIndex] : undefined; + if (defaultBody && statement.caseBlock.clauses.length === 1) { + statements.push(lua.createDoStatement(defaultBody)); + } else { + let concatenatedIf: lua.IfStatement | undefined = undefined; + let previousCondition: lua.IfStatement | lua.Block | undefined = defaultBody + ? lua.createBlock(defaultBody) + : undefined; + + // Starting from the back, concatenating ifs into one big if/elseif/[else] statement + for (let i = statement.caseBlock.clauses.length - 1; i >= 0; i--) { + const clause = statement.caseBlock.clauses[i]; + + // Skip default clause to keep index aligned, handle in else block + if (ts.isDefaultClause(clause)) continue; // If the clause condition holds, go to the correct label const condition = lua.createBinaryExpression( @@ -65,15 +76,10 @@ export const transformSwitchStatement: FunctionVisitor = (st lua.SyntaxKind.EqualityOperator ); - return lua.createIfStatement(condition, lua.createBlock(caseBody[index]), previousCondition); - }, - defaultIndex >= 0 ? lua.createBlock(caseBody[defaultIndex]) : undefined - ); - - let statements: lua.Statement[] = []; - - if (concatenatedIf) { - statements.push(concatenatedIf as unknown as lua.IfStatement); + concatenatedIf = lua.createIfStatement(condition, lua.createBlock(caseBody[i]), previousCondition); + previousCondition = concatenatedIf; + } + if (concatenatedIf) statements.push(concatenatedIf); } statements = performHoisting(context, statements); diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index de48d7e8a..3fbc65b32 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -311,6 +311,17 @@ test("switch default case not last - second", () => { `.expectToMatchJsResult(); }); +test("switch default case only", () => { + util.testFunction` + let out = 0; + switch (4 as number) { + default: + out = 1 + } + return out; + `.expectToMatchJsResult(); +}); + test("switch fallthrough enters default", () => { util.testFunction` const out = []; From 741db69bccf451f295eb69defe3702178a9f8440 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Sun, 22 Aug 2021 21:33:23 -0700 Subject: [PATCH 03/24] fix: lexical scoping create scope for switch and remove additional scope for each clause. --- src/transformation/visitors/switch.ts | 7 ++----- test/unit/__snapshots__/switch.spec.ts.snap | 14 +++++--------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 983efbfec..419076177 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -41,10 +41,7 @@ export const transformSwitchStatement: FunctionVisitor = (st caseBody[i] = statement.caseBlock.clauses .slice(i, end >= 0 ? end + i + 1 : undefined) .reduce( - (statements, clause) => [ - ...statements, - lua.createDoStatement(context.transformStatements(clause.statements)), - ], + (statements, clause) => [...statements, ...context.transformStatements(clause.statements)], [] ); } @@ -88,5 +85,5 @@ export const transformSwitchStatement: FunctionVisitor = (st const expression = context.transformExpression(statement.expression); statements.unshift(lua.createVariableDeclarationStatement(switchVariable, expression)); - return statements; + return lua.createDoStatement(statements); }; diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap index 3b91d867d..c0af81f36 100644 --- a/test/unit/__snapshots__/switch.spec.ts.snap +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -4,21 +4,17 @@ exports[`switch uses elseif 1`] = ` "local ____exports = {} function ____exports.__main(self) local result = -1 - local ____switch3 = 2 - if ____switch3 == 0 then - do + do + local ____switch3 = 2 + if ____switch3 == 0 then do result = 200 end - end - elseif ____switch3 == 1 then - do + elseif ____switch3 == 1 then do result = 100 end - end - elseif ____switch3 == 2 then - do + elseif ____switch3 == 2 then do result = 1 end From 9172e889b9f599f64fa810f538b1b128c3eef951 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Mon, 23 Aug 2021 05:00:59 -0700 Subject: [PATCH 04/24] fix: test do not pollute parent scope --- test/unit/switch.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index 3fbc65b32..4fc98e347 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -360,3 +360,15 @@ test("switch fallthrough stops after default", () => { return out; `.expectToMatchJsResult(); }); + +test("switch does not pollute parent scope", () => { + util.testFunction` + let x: number = 0; + let y = 1; + switch (x) { + case 0: + let y = 2; + } + return y; + `.expectToMatchJsResult(); +}); From 504fd1a6b1903f6a013919f95d184ce89bc0b17c Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Mon, 23 Aug 2021 05:24:58 -0700 Subject: [PATCH 05/24] fix: scope switch in repeat-until, emit break --- src/transformation/visitors/break-continue.ts | 2 +- src/transformation/visitors/switch.ts | 3 ++- test/unit/__snapshots__/switch.spec.ts.snap | 7 +++++-- test/unit/switch.spec.ts | 7 ++++++- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/transformation/visitors/break-continue.ts b/src/transformation/visitors/break-continue.ts index c415a5805..5bddd42cf 100644 --- a/src/transformation/visitors/break-continue.ts +++ b/src/transformation/visitors/break-continue.ts @@ -8,7 +8,7 @@ import { findScope, ScopeType } from "../utils/scope"; export const transformBreakStatement: FunctionVisitor = (breakStatement, context) => { const breakableScope = findScope(context, ScopeType.Loop | ScopeType.Switch); if (breakableScope?.type === ScopeType.Switch) { - return undefined; + return lua.createBreakStatement(breakStatement); } else { return lua.createBreakStatement(breakStatement); } diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 419076177..f38fdf13f 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -6,6 +6,7 @@ import { performHoisting, popScope, pushScope, ScopeType } from "../utils/scope" const containsBreakStatement = (statements: ts.Node[]): boolean => { for (const s of statements) { if ( + ts.isIfStatement(s) || ts.isSwitchStatement(s) || ts.isWhileStatement(s) || ts.isDoStatement(s) || @@ -85,5 +86,5 @@ export const transformSwitchStatement: FunctionVisitor = (st const expression = context.transformExpression(statement.expression); statements.unshift(lua.createVariableDeclarationStatement(switchVariable, expression)); - return lua.createDoStatement(statements); + return lua.createRepeatStatement(lua.createBlock(statements), lua.createBooleanLiteral(true)); }; diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap index c0af81f36..89e81a0ae 100644 --- a/test/unit/__snapshots__/switch.spec.ts.snap +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -4,22 +4,25 @@ exports[`switch uses elseif 1`] = ` "local ____exports = {} function ____exports.__main(self) local result = -1 - do + repeat local ____switch3 = 2 if ____switch3 == 0 then do result = 200 + break end elseif ____switch3 == 1 then do result = 100 + break end elseif ____switch3 == 2 then do result = 1 + break end end - end + until true return result end return ____exports" diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index 4fc98e347..c5adf6211 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -207,7 +207,7 @@ test.each([0, 1, 2, 3])("switchWithBrackets (%p)", inp => { `.expectToMatchJsResult(); }); -test.each([0, 1, 2, 3])("switchWithBracketsBreakInConditional (%p)", inp => { +test.each([0, 1, 2, 3, 4])("switchWithBracketsBreakInConditional (%p)", inp => { util.testFunction` let result: number = -1; @@ -223,6 +223,11 @@ test.each([0, 1, 2, 3])("switchWithBracketsBreakInConditional (%p)", inp => { } case 2: { result = 2; + + if (result != 2) break; + } + case 3: { + result = 3; break; } } From 08677934cb40f98ac47282bc77538a19359cc1cf Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Mon, 23 Aug 2021 11:23:49 -0700 Subject: [PATCH 06/24] fix: non duplicating case body switch --- src/transformation/visitors/break-continue.ts | 1 + src/transformation/visitors/switch.ts | 118 ++++++++++-------- test/unit/__snapshots__/switch.spec.ts.snap | 32 ++++- test/unit/switch.spec.ts | 22 ++++ 4 files changed, 121 insertions(+), 52 deletions(-) diff --git a/src/transformation/visitors/break-continue.ts b/src/transformation/visitors/break-continue.ts index 5bddd42cf..17c553740 100644 --- a/src/transformation/visitors/break-continue.ts +++ b/src/transformation/visitors/break-continue.ts @@ -8,6 +8,7 @@ import { findScope, ScopeType } from "../utils/scope"; export const transformBreakStatement: FunctionVisitor = (breakStatement, context) => { const breakableScope = findScope(context, ScopeType.Loop | ScopeType.Switch); if (breakableScope?.type === ScopeType.Switch) { + // Break is handled by the switch statement (see transformSwitchStatement) return lua.createBreakStatement(breakStatement); } else { return lua.createBreakStatement(breakStatement); diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index f38fdf13f..ac1543560 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -3,22 +3,14 @@ import * as lua from "../../LuaAST"; import { FunctionVisitor } from "../context"; import { performHoisting, popScope, pushScope, ScopeType } from "../utils/scope"; -const containsBreakStatement = (statements: ts.Node[]): boolean => { +const containsBreakOrReturn = (statements: ts.Node[]): boolean => { for (const s of statements) { - if ( - ts.isIfStatement(s) || - ts.isSwitchStatement(s) || - ts.isWhileStatement(s) || - ts.isDoStatement(s) || - ts.isForStatement(s) || - ts.isForInStatement(s) || - ts.isForOfStatement(s) - ) { - // Ignore: Break statements are valid as children of these - // statements without breaking the clause - } else if (ts.isBreakStatement(s)) { + if (ts.isBreakStatement(s) || ts.isReturnStatement(s)) { return true; - } else if (containsBreakStatement(s.getChildren())) { + } else if (!ts.isBlock(s)) { + // Can only ensure a break scoped in a block is deterministic + continue; + } else if (containsBreakOrReturn(s.getChildren())) { return true; } } @@ -33,51 +25,77 @@ export const transformSwitchStatement: FunctionVisitor = (st const switchName = `____switch${scope.id}`; const switchVariable = lua.createIdentifier(switchName); - // Collect the fallthrough bodies for each case as defined by the switch. - const caseBody: lua.Statement[][] = []; - for (let i = 0; i < statement.caseBlock.clauses.length; i++) { - const end = statement.caseBlock.clauses - .slice(i) - .findIndex(clause => containsBreakStatement([...clause.statements])); - caseBody[i] = statement.caseBlock.clauses - .slice(i, end >= 0 ? end + i + 1 : undefined) - .reduce( - (statements, clause) => [...statements, ...context.transformStatements(clause.statements)], - [] - ); - } + // Collect the case clause expressions and accounting for deterministic fallthrough + let allExpressions: lua.BinaryExpression; + statement.caseBlock.clauses.forEach(clause => { + if (!ts.isDefaultClause(clause)) { + allExpressions = allExpressions + ? lua.createBinaryExpression( + allExpressions, + lua.createBinaryExpression( + switchVariable, + context.transformExpression(clause.expression), + lua.SyntaxKind.EqualityOperator + ), + lua.SyntaxKind.OrOperator + ) + : lua.createBinaryExpression( + switchVariable, + context.transformExpression(clause.expression), + lua.SyntaxKind.EqualityOperator + ); + } + }); let statements: lua.Statement[] = []; // Default will either be the only statement, or the else in the if chain const defaultIndex = statement.caseBlock.clauses.findIndex(c => ts.isDefaultClause(c)); - const defaultBody = defaultIndex >= 0 ? caseBody[defaultIndex] : undefined; + const defaultBody = defaultIndex >= 0 ? statement.caseBlock.clauses[defaultIndex].statements : undefined; if (defaultBody && statement.caseBlock.clauses.length === 1) { - statements.push(lua.createDoStatement(defaultBody)); + statements.push(lua.createDoStatement(context.transformStatements(defaultBody))); } else { - let concatenatedIf: lua.IfStatement | undefined = undefined; - let previousCondition: lua.IfStatement | lua.Block | undefined = defaultBody - ? lua.createBlock(defaultBody) - : undefined; - - // Starting from the back, concatenating ifs into one big if/elseif/[else] statement - for (let i = statement.caseBlock.clauses.length - 1; i >= 0; i--) { - const clause = statement.caseBlock.clauses[i]; + let previousClause: ts.CaseOrDefaultClause; + let condition: lua.Expression; + statement.caseBlock.clauses.forEach(clause => { + if (!condition || (previousClause && containsBreakOrReturn([...previousClause.statements]))) { + if (ts.isDefaultClause(clause)) { + condition = lua.createUnaryExpression(allExpressions, lua.SyntaxKind.NotOperator); + } else { + condition = lua.createBinaryExpression( + switchVariable, + context.transformExpression(clause.expression), + lua.SyntaxKind.EqualityOperator + ); + } + } else { + if (ts.isDefaultClause(clause)) { + condition = lua.createBinaryExpression( + condition, + lua.createUnaryExpression(allExpressions, lua.SyntaxKind.NotOperator), + lua.SyntaxKind.OrOperator + ); + } else { + condition = lua.createBinaryExpression( + condition, + lua.createBinaryExpression( + switchVariable, + context.transformExpression(clause.expression), + lua.SyntaxKind.EqualityOperator + ), + lua.SyntaxKind.OrOperator + ); + } + } - // Skip default clause to keep index aligned, handle in else block - if (ts.isDefaultClause(clause)) continue; + if (condition && clause.statements.length) { + statements.push( + lua.createIfStatement(condition, lua.createBlock(context.transformStatements(clause.statements))) + ); + } - // If the clause condition holds, go to the correct label - const condition = lua.createBinaryExpression( - switchVariable, - context.transformExpression(clause.expression), - lua.SyntaxKind.EqualityOperator - ); - - concatenatedIf = lua.createIfStatement(condition, lua.createBlock(caseBody[i]), previousCondition); - previousCondition = concatenatedIf; - } - if (concatenatedIf) statements.push(concatenatedIf); + previousClause = clause; + }); } statements = performHoisting(context, statements); diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap index 89e81a0ae..c6f568634 100644 --- a/test/unit/__snapshots__/switch.spec.ts.snap +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -1,5 +1,31 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`switch collapses empty case and minimizes conditions 1`] = ` +"require(\\"lualib_bundle\\"); +local ____exports = {} +function ____exports.__main(self) + local out = {} + repeat + local ____switch3 = 5 + if ((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2) then + __TS__ArrayPush(out, \\"0,1,2\\") + end + if (((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2)) or (____switch3 == 3) then + __TS__ArrayPush(out, \\"3\\") + break + end + if not (((((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2)) or (____switch3 == 3)) or (____switch3 == 4)) then + __TS__ArrayPush(out, \\"default\\") + end + if (not (((((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2)) or (____switch3 == 3)) or (____switch3 == 4))) or (____switch3 == 4) then + __TS__ArrayPush(out, \\"4\\") + end + until true + return out +end +return ____exports" +`; + exports[`switch uses elseif 1`] = ` "local ____exports = {} function ____exports.__main(self) @@ -11,12 +37,14 @@ function ____exports.__main(self) result = 200 break end - elseif ____switch3 == 1 then + end + if (____switch3 == 0) or (____switch3 == 1) then do result = 100 break end - elseif ____switch3 == 2 then + end + if ((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2) then do result = 1 break diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index c5adf6211..89df63c8b 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -377,3 +377,25 @@ test("switch does not pollute parent scope", () => { return y; `.expectToMatchJsResult(); }); + +test("switch collapses empty case and minimizes conditions", () => { + util.testFunction` + const out = []; + switch (5 as number) { + case 0: + case 1: + case 2: + out.push("0,1,2"); + case 3: + out.push("3"); + break; + default: + out.push("default"); + case 4: + out.push("4"); + } + return out; + ` + .expectLuaToMatchSnapshot() + .expectToMatchJsResult(); +}); From f95902d618f248a17905cb593a9414284eb240a6 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Mon, 23 Aug 2021 11:45:38 -0700 Subject: [PATCH 07/24] chore: comments --- src/transformation/visitors/switch.ts | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index ac1543560..57c64647f 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -25,7 +25,7 @@ export const transformSwitchStatement: FunctionVisitor = (st const switchName = `____switch${scope.id}`; const switchVariable = lua.createIdentifier(switchName); - // Collect the case clause expressions and accounting for deterministic fallthrough + // Collect all the expressions into a single expression for use in the default clause let allExpressions: lua.BinaryExpression; statement.caseBlock.clauses.forEach(clause => { if (!ts.isDefaultClause(clause)) { @@ -49,12 +49,18 @@ export const transformSwitchStatement: FunctionVisitor = (st let statements: lua.Statement[] = []; - // Default will either be the only statement, or the else in the if chain - const defaultIndex = statement.caseBlock.clauses.findIndex(c => ts.isDefaultClause(c)); - const defaultBody = defaultIndex >= 0 ? statement.caseBlock.clauses[defaultIndex].statements : undefined; - if (defaultBody && statement.caseBlock.clauses.length === 1) { - statements.push(lua.createDoStatement(context.transformStatements(defaultBody))); + // If the switch only has a default clause, wrap it in a single do. + // Otherwise, we need to generate a set of if statements to emulate the switch. + const clauses = statement.caseBlock.clauses; + if (clauses.length === 1 && ts.isDefaultClause(clauses[0])) { + const defaultClause = clauses[0].statements; + if (defaultClause.length) { + statements.push(lua.createDoStatement(context.transformStatements(defaultClause))); + } } else { + // Build up the condition for each if statement + // Fallthrough is handled by accepting the last condition as an additional or clause + // Default is the not of all known case expressions let previousClause: ts.CaseOrDefaultClause; let condition: lua.Expression; statement.caseBlock.clauses.forEach(clause => { @@ -98,11 +104,14 @@ export const transformSwitchStatement: FunctionVisitor = (st }); } + // Hoist the variable, function, and import statements to the top of the switch statements = performHoisting(context, statements); popScope(context); + // Add the switch expression after hoisting const expression = context.transformExpression(statement.expression); statements.unshift(lua.createVariableDeclarationStatement(switchVariable, expression)); + // Wrap the statements in a repeat until true statement to facilitate dynamic break/returns return lua.createRepeatStatement(lua.createBlock(statements), lua.createBooleanLiteral(true)); }; From b4a2dddbcf6156b832202d4c553b846572e4334a Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Mon, 23 Aug 2021 14:44:12 -0700 Subject: [PATCH 08/24] refactor: handle side-effects plus test --- src/transformation/visitors/switch.ts | 55 +++++++++++++++++---- test/unit/__snapshots__/switch.spec.ts.snap | 26 +++++++--- test/unit/switch.spec.ts | 24 +++++++++ 3 files changed, 88 insertions(+), 17 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 57c64647f..32e424ba9 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -21,9 +21,11 @@ const containsBreakOrReturn = (statements: ts.Node[]): boolean => { export const transformSwitchStatement: FunctionVisitor = (statement, context) => { const scope = pushScope(context, ScopeType.Switch); - // Give the switch a unique name to prevent nested switches from acting up. + // Give the switch and condition accumulator a unique name to prevent nested switches from acting up. const switchName = `____switch${scope.id}`; + const conditionName = `____cond${scope.id}`; const switchVariable = lua.createIdentifier(switchName); + const conditionVariable = lua.createIdentifier(conditionName); // Collect all the expressions into a single expression for use in the default clause let allExpressions: lua.BinaryExpression; @@ -62,11 +64,11 @@ export const transformSwitchStatement: FunctionVisitor = (st // Fallthrough is handled by accepting the last condition as an additional or clause // Default is the not of all known case expressions let previousClause: ts.CaseOrDefaultClause; - let condition: lua.Expression; + let condition: lua.Expression | undefined; statement.caseBlock.clauses.forEach(clause => { if (!condition || (previousClause && containsBreakOrReturn([...previousClause.statements]))) { if (ts.isDefaultClause(clause)) { - condition = lua.createUnaryExpression(allExpressions, lua.SyntaxKind.NotOperator); + // If the default is first, or followed by a break, we can't fall into it, skip. } else { condition = lua.createBinaryExpression( switchVariable, @@ -76,11 +78,7 @@ export const transformSwitchStatement: FunctionVisitor = (st } } else { if (ts.isDefaultClause(clause)) { - condition = lua.createBinaryExpression( - condition, - lua.createUnaryExpression(allExpressions, lua.SyntaxKind.NotOperator), - lua.SyntaxKind.OrOperator - ); + // use the previous condition for the default clause } else { condition = lua.createBinaryExpression( condition, @@ -94,14 +92,50 @@ export const transformSwitchStatement: FunctionVisitor = (st } } - if (condition && clause.statements.length) { + if (clause.statements.length) { + if (!ts.isDefaultClause(clause) && condition) { + statements.push( + lua.createAssignmentStatement( + conditionVariable, + lua.createBinaryExpression(conditionVariable, condition, lua.SyntaxKind.OrOperator) + ) + ); + condition = undefined; + } + statements.push( - lua.createIfStatement(condition, lua.createBlock(context.transformStatements(clause.statements))) + lua.createIfStatement( + conditionVariable, + lua.createBlock(context.transformStatements(clause.statements)) + ) ); } previousClause = clause; }); + + // Amalgamate the default w/ fallthrough clauses and execute if nothing else executed above + const start = clauses.findIndex(c => ts.isDefaultClause(c)); + if (start >= 0) { + const end = statement.caseBlock.clauses + .slice(start) + .findIndex(clause => containsBreakOrReturn([...clause.statements])); + const defaultStatements = statement.caseBlock.clauses + .slice(start, end >= 0 ? end + start + 1 : undefined) + .reduce( + (statements, clause) => [...statements, ...context.transformStatements(clause.statements)], + [] + ); + + if (defaultStatements.length) { + statements.push( + lua.createIfStatement( + lua.createUnaryExpression(conditionVariable, lua.SyntaxKind.NotOperator), + lua.createBlock(defaultStatements) + ) + ); + } + } } // Hoist the variable, function, and import statements to the top of the switch @@ -110,6 +144,7 @@ export const transformSwitchStatement: FunctionVisitor = (st // Add the switch expression after hoisting const expression = context.transformExpression(statement.expression); + statements.unshift(lua.createVariableDeclarationStatement(conditionVariable, lua.createBooleanLiteral(false))); statements.unshift(lua.createVariableDeclarationStatement(switchVariable, expression)); // Wrap the statements in a repeat until true statement to facilitate dynamic break/returns diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap index c6f568634..1b85ee7d5 100644 --- a/test/unit/__snapshots__/switch.spec.ts.snap +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -7,17 +7,25 @@ function ____exports.__main(self) local out = {} repeat local ____switch3 = 5 - if ((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2) then + local ____cond3 = false + ____cond3 = ____cond3 or (((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2)) + if ____cond3 then __TS__ArrayPush(out, \\"0,1,2\\") end - if (((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2)) or (____switch3 == 3) then + ____cond3 = ____cond3 or (____switch3 == 3) + if ____cond3 then __TS__ArrayPush(out, \\"3\\") break end - if not (((((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2)) or (____switch3 == 3)) or (____switch3 == 4)) then + if ____cond3 then __TS__ArrayPush(out, \\"default\\") end - if (not (((((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2)) or (____switch3 == 3)) or (____switch3 == 4))) or (____switch3 == 4) then + ____cond3 = ____cond3 or (____switch3 == 4) + if ____cond3 then + __TS__ArrayPush(out, \\"4\\") + end + if not ____cond3 then + __TS__ArrayPush(out, \\"default\\") __TS__ArrayPush(out, \\"4\\") end until true @@ -32,19 +40,23 @@ function ____exports.__main(self) local result = -1 repeat local ____switch3 = 2 - if ____switch3 == 0 then + local ____cond3 = false + ____cond3 = ____cond3 or (____switch3 == 0) + if ____cond3 then do result = 200 break end end - if (____switch3 == 0) or (____switch3 == 1) then + ____cond3 = ____cond3 or (____switch3 == 1) + if ____cond3 then do result = 100 break end end - if ((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2) then + ____cond3 = ____cond3 or (____switch3 == 2) + if ____cond3 then do result = 1 break diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index 89df63c8b..1409b54a2 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -399,3 +399,27 @@ test("switch collapses empty case and minimizes conditions", () => { .expectLuaToMatchSnapshot() .expectToMatchJsResult(); }); + +test("switch handles side-effects", () => { + util.testFunction` + const out = []; + + let y = 0; + function foo() { + return y++; + } + + let x = 0; + switch (x) { + case foo(): + out.push(1); + case foo(): + out.push(2); + case foo(): + out.push(3); + } + + out.push(y); + return out; + `.expectToMatchJsResult(); +}); From 099495b4c5e66314d801028c24bdb82cbc450806 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Mon, 23 Aug 2021 21:43:54 -0700 Subject: [PATCH 09/24] fix: cleanup & feedback --- src/transformation/visitors/switch.ts | 57 ++++--- test/unit/__snapshots__/switch.spec.ts.snap | 173 +++++++++++++++++++- test/unit/switch.spec.ts | 12 +- 3 files changed, 206 insertions(+), 36 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 32e424ba9..2261e3b7e 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -65,10 +65,11 @@ export const transformSwitchStatement: FunctionVisitor = (st // Default is the not of all known case expressions let previousClause: ts.CaseOrDefaultClause; let condition: lua.Expression | undefined; + let isInitialCondition = true; statement.caseBlock.clauses.forEach(clause => { if (!condition || (previousClause && containsBreakOrReturn([...previousClause.statements]))) { if (ts.isDefaultClause(clause)) { - // If the default is first, or followed by a break, we can't fall into it, skip. + condition = undefined; } else { condition = lua.createBinaryExpression( switchVariable, @@ -76,39 +77,46 @@ export const transformSwitchStatement: FunctionVisitor = (st lua.SyntaxKind.EqualityOperator ); } - } else { - if (ts.isDefaultClause(clause)) { - // use the previous condition for the default clause - } else { - condition = lua.createBinaryExpression( - condition, - lua.createBinaryExpression( - switchVariable, - context.transformExpression(clause.expression), - lua.SyntaxKind.EqualityOperator - ), - lua.SyntaxKind.OrOperator - ); - } + } else if (!ts.isDefaultClause(clause)) { + condition = lua.createBinaryExpression( + condition, + lua.createBinaryExpression( + switchVariable, + context.transformExpression(clause.expression), + lua.SyntaxKind.EqualityOperator + ), + lua.SyntaxKind.OrOperator + ); } if (clause.statements.length) { if (!ts.isDefaultClause(clause) && condition) { statements.push( - lua.createAssignmentStatement( - conditionVariable, - lua.createBinaryExpression(conditionVariable, condition, lua.SyntaxKind.OrOperator) - ) + isInitialCondition + ? lua.createVariableDeclarationStatement(conditionVariable, condition) + : lua.createAssignmentStatement( + conditionVariable, + lua.createBinaryExpression(conditionVariable, condition, lua.SyntaxKind.OrOperator) + ) ); + isInitialCondition = false; condition = undefined; } - statements.push( - lua.createIfStatement( - conditionVariable, - lua.createBlock(context.transformStatements(clause.statements)) + if ( + !( + ts.isDefaultClause(clause) && + previousClause && + containsBreakOrReturn([...previousClause.statements]) ) - ); + ) { + statements.push( + lua.createIfStatement( + conditionVariable, + lua.createBlock(context.transformStatements(clause.statements)) + ) + ); + } } previousClause = clause; @@ -144,7 +152,6 @@ export const transformSwitchStatement: FunctionVisitor = (st // Add the switch expression after hoisting const expression = context.transformExpression(statement.expression); - statements.unshift(lua.createVariableDeclarationStatement(conditionVariable, lua.createBooleanLiteral(false))); statements.unshift(lua.createVariableDeclarationStatement(switchVariable, expression)); // Wrap the statements in a repeat until true statement to facilitate dynamic break/returns diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap index 1b85ee7d5..4f2b7573d 100644 --- a/test/unit/__snapshots__/switch.spec.ts.snap +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -7,8 +7,7 @@ function ____exports.__main(self) local out = {} repeat local ____switch3 = 5 - local ____cond3 = false - ____cond3 = ____cond3 or (((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2)) + local ____cond3 = ((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2) if ____cond3 then __TS__ArrayPush(out, \\"0,1,2\\") end @@ -17,9 +16,6 @@ function ____exports.__main(self) __TS__ArrayPush(out, \\"3\\") break end - if ____cond3 then - __TS__ArrayPush(out, \\"default\\") - end ____cond3 = ____cond3 or (____switch3 == 4) if ____cond3 then __TS__ArrayPush(out, \\"4\\") @@ -34,14 +30,177 @@ end return ____exports" `; +exports[`switch handles side-effects (0) 1`] = ` +"require(\\"lualib_bundle\\"); +local ____exports = {} +function ____exports.__main(self) + local out = {} + local y = 0 + local function foo(self) + return (function() + local ____tmp = y + y = ____tmp + 1 + return ____tmp + end)() + end + local x = 0 + repeat + local ____switch5 = x + local ____cond5 = ____switch5 == foo(nil) + if ____cond5 then + __TS__ArrayPush(out, 1) + end + ____cond5 = ____cond5 or (____switch5 == foo(nil)) + if ____cond5 then + __TS__ArrayPush(out, 2) + end + ____cond5 = ____cond5 or (____switch5 == foo(nil)) + if ____cond5 then + __TS__ArrayPush(out, 3) + end + if ____cond5 then + __TS__ArrayPush(out, \\"default\\") + end + if not ____cond5 then + __TS__ArrayPush(out, \\"default\\") + end + until true + __TS__ArrayPush(out, y) + return out +end +return ____exports" +`; + +exports[`switch handles side-effects (1) 1`] = ` +"require(\\"lualib_bundle\\"); +local ____exports = {} +function ____exports.__main(self) + local out = {} + local y = 0 + local function foo(self) + return (function() + local ____tmp = y + y = ____tmp + 1 + return ____tmp + end)() + end + local x = 1 + repeat + local ____switch5 = x + local ____cond5 = ____switch5 == foo(nil) + if ____cond5 then + __TS__ArrayPush(out, 1) + end + ____cond5 = ____cond5 or (____switch5 == foo(nil)) + if ____cond5 then + __TS__ArrayPush(out, 2) + end + ____cond5 = ____cond5 or (____switch5 == foo(nil)) + if ____cond5 then + __TS__ArrayPush(out, 3) + end + if ____cond5 then + __TS__ArrayPush(out, \\"default\\") + end + if not ____cond5 then + __TS__ArrayPush(out, \\"default\\") + end + until true + __TS__ArrayPush(out, y) + return out +end +return ____exports" +`; + +exports[`switch handles side-effects (2) 1`] = ` +"require(\\"lualib_bundle\\"); +local ____exports = {} +function ____exports.__main(self) + local out = {} + local y = 0 + local function foo(self) + return (function() + local ____tmp = y + y = ____tmp + 1 + return ____tmp + end)() + end + local x = 2 + repeat + local ____switch5 = x + local ____cond5 = ____switch5 == foo(nil) + if ____cond5 then + __TS__ArrayPush(out, 1) + end + ____cond5 = ____cond5 or (____switch5 == foo(nil)) + if ____cond5 then + __TS__ArrayPush(out, 2) + end + ____cond5 = ____cond5 or (____switch5 == foo(nil)) + if ____cond5 then + __TS__ArrayPush(out, 3) + end + if ____cond5 then + __TS__ArrayPush(out, \\"default\\") + end + if not ____cond5 then + __TS__ArrayPush(out, \\"default\\") + end + until true + __TS__ArrayPush(out, y) + return out +end +return ____exports" +`; + +exports[`switch handles side-effects (3) 1`] = ` +"require(\\"lualib_bundle\\"); +local ____exports = {} +function ____exports.__main(self) + local out = {} + local y = 0 + local function foo(self) + return (function() + local ____tmp = y + y = ____tmp + 1 + return ____tmp + end)() + end + local x = 3 + repeat + local ____switch5 = x + local ____cond5 = ____switch5 == foo(nil) + if ____cond5 then + __TS__ArrayPush(out, 1) + end + ____cond5 = ____cond5 or (____switch5 == foo(nil)) + if ____cond5 then + __TS__ArrayPush(out, 2) + end + ____cond5 = ____cond5 or (____switch5 == foo(nil)) + if ____cond5 then + __TS__ArrayPush(out, 3) + end + if ____cond5 then + __TS__ArrayPush(out, \\"default\\") + end + if not ____cond5 then + __TS__ArrayPush(out, \\"default\\") + end + until true + __TS__ArrayPush(out, y) + return out +end +return ____exports" +`; + exports[`switch uses elseif 1`] = ` "local ____exports = {} function ____exports.__main(self) local result = -1 repeat local ____switch3 = 2 - local ____cond3 = false - ____cond3 = ____cond3 or (____switch3 == 0) + local ____cond3 = ____switch3 == 0 if ____cond3 then do result = 200 diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index 1409b54a2..dae3475c3 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -400,16 +400,16 @@ test("switch collapses empty case and minimizes conditions", () => { .expectToMatchJsResult(); }); -test("switch handles side-effects", () => { +test.each([0, 1, 2, 3])("switch handles side-effects (%p)", inp => { util.testFunction` const out = []; - + let y = 0; function foo() { return y++; } - let x = 0; + let x = ${inp} as number; switch (x) { case foo(): out.push(1); @@ -417,9 +417,13 @@ test("switch handles side-effects", () => { out.push(2); case foo(): out.push(3); + default: + out.push("default"); } out.push(y); return out; - `.expectToMatchJsResult(); + ` + .expectLuaToMatchSnapshot() + .expectToMatchJsResult(); }); From 1eea7ff9112462a9b0f1332c941bf1528ff18695 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Mon, 23 Aug 2021 21:51:20 -0700 Subject: [PATCH 10/24] chore: remove dead code --- src/transformation/visitors/switch.ts | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 2261e3b7e..b10d29dc8 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -27,32 +27,9 @@ export const transformSwitchStatement: FunctionVisitor = (st const switchVariable = lua.createIdentifier(switchName); const conditionVariable = lua.createIdentifier(conditionName); - // Collect all the expressions into a single expression for use in the default clause - let allExpressions: lua.BinaryExpression; - statement.caseBlock.clauses.forEach(clause => { - if (!ts.isDefaultClause(clause)) { - allExpressions = allExpressions - ? lua.createBinaryExpression( - allExpressions, - lua.createBinaryExpression( - switchVariable, - context.transformExpression(clause.expression), - lua.SyntaxKind.EqualityOperator - ), - lua.SyntaxKind.OrOperator - ) - : lua.createBinaryExpression( - switchVariable, - context.transformExpression(clause.expression), - lua.SyntaxKind.EqualityOperator - ); - } - }); - - let statements: lua.Statement[] = []; - // If the switch only has a default clause, wrap it in a single do. // Otherwise, we need to generate a set of if statements to emulate the switch. + let statements: lua.Statement[] = []; const clauses = statement.caseBlock.clauses; if (clauses.length === 1 && ts.isDefaultClause(clauses[0])) { const defaultClause = clauses[0].statements; From 288c2a35a95924b1089a21c0b0faf49bbf380fa1 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Mon, 23 Aug 2021 22:00:04 -0700 Subject: [PATCH 11/24] refactor: cleanup redundent break statement --- src/transformation/visitors/break-continue.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/transformation/visitors/break-continue.ts b/src/transformation/visitors/break-continue.ts index 17c553740..9e476b17e 100644 --- a/src/transformation/visitors/break-continue.ts +++ b/src/transformation/visitors/break-continue.ts @@ -6,13 +6,8 @@ import { unsupportedForTarget } from "../utils/diagnostics"; import { findScope, ScopeType } from "../utils/scope"; export const transformBreakStatement: FunctionVisitor = (breakStatement, context) => { - const breakableScope = findScope(context, ScopeType.Loop | ScopeType.Switch); - if (breakableScope?.type === ScopeType.Switch) { - // Break is handled by the switch statement (see transformSwitchStatement) - return lua.createBreakStatement(breakStatement); - } else { - return lua.createBreakStatement(breakStatement); - } + void context; + return lua.createBreakStatement(breakStatement); }; export const transformContinueStatement: FunctionVisitor = (statement, context) => { From 8d1f74a105e1994e5ee6a7df0a2851d217fae06b Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Tue, 24 Aug 2021 11:27:39 -0700 Subject: [PATCH 12/24] refactor: cleanup for maintainability and review --- src/transformation/visitors/switch.ts | 97 ++++----- test/unit/__snapshots__/switch.spec.ts.snap | 212 +++----------------- test/unit/switch.spec.ts | 37 +++- 3 files changed, 103 insertions(+), 243 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index b10d29dc8..5587a7ef2 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -1,5 +1,6 @@ import * as ts from "typescript"; import * as lua from "../../LuaAST"; +import { assert } from "../../utils"; import { FunctionVisitor } from "../context"; import { performHoisting, popScope, pushScope, ScopeType } from "../utils/scope"; @@ -40,64 +41,68 @@ export const transformSwitchStatement: FunctionVisitor = (st // Build up the condition for each if statement // Fallthrough is handled by accepting the last condition as an additional or clause // Default is the not of all known case expressions - let previousClause: ts.CaseOrDefaultClause; - let condition: lua.Expression | undefined; let isInitialCondition = true; - statement.caseBlock.clauses.forEach(clause => { - if (!condition || (previousClause && containsBreakOrReturn([...previousClause.statements]))) { - if (ts.isDefaultClause(clause)) { - condition = undefined; + let condition: lua.Expression | undefined = undefined; + for (let i = 0; i < statement.caseBlock.clauses.length; i++) { + const clause = statement.caseBlock.clauses[i]; + const previousClause: ts.CaseOrDefaultClause | undefined = statement.caseBlock.clauses[i - 1]; + + // Skip redundant default clauses, will be handled in final default case + if (i === 0 && ts.isDefaultClause(clause)) continue; + if (ts.isDefaultClause(clause) && previousClause && containsBreakOrReturn([...previousClause.statements])) { + continue; + } + + // Compute the condition for the if statement + if (!ts.isDefaultClause(clause)) { + if (condition) { + // Coalesce skipped statements + condition = lua.createBinaryExpression( + condition, + lua.createBinaryExpression( + switchVariable, + context.transformExpression(clause.expression), + lua.SyntaxKind.EqualityOperator + ), + lua.SyntaxKind.OrOperator + ); } else { + // Next condition condition = lua.createBinaryExpression( switchVariable, context.transformExpression(clause.expression), lua.SyntaxKind.EqualityOperator ); } - } else if (!ts.isDefaultClause(clause)) { - condition = lua.createBinaryExpression( - condition, - lua.createBinaryExpression( - switchVariable, - context.transformExpression(clause.expression), - lua.SyntaxKind.EqualityOperator - ), - lua.SyntaxKind.OrOperator - ); - } - if (clause.statements.length) { - if (!ts.isDefaultClause(clause) && condition) { - statements.push( - isInitialCondition - ? lua.createVariableDeclarationStatement(conditionVariable, condition) - : lua.createAssignmentStatement( - conditionVariable, - lua.createBinaryExpression(conditionVariable, condition, lua.SyntaxKind.OrOperator) - ) - ); - isInitialCondition = false; - condition = undefined; - } + // Skip empty clauses + if (clause.statements.length === 0) continue; - if ( - !( - ts.isDefaultClause(clause) && - previousClause && - containsBreakOrReturn([...previousClause.statements]) - ) - ) { - statements.push( - lua.createIfStatement( - conditionVariable, - lua.createBlock(context.transformStatements(clause.statements)) - ) - ); - } + // Declare or assign condition variable + statements.push( + isInitialCondition + ? lua.createVariableDeclarationStatement(conditionVariable, condition) + : lua.createAssignmentStatement( + conditionVariable, + lua.createBinaryExpression(conditionVariable, condition, lua.SyntaxKind.OrOperator) + ) + ); + isInitialCondition = false; + } else { + assert(!isInitialCondition, "Default clause should never be the initial condition"); } - previousClause = clause; - }); + // Push if statement for case + statements.push( + lua.createIfStatement( + conditionVariable, + lua.createBlock(context.transformStatements(clause.statements)) + ) + ); + + // Clear condition for next clause + condition = undefined; + } // Amalgamate the default w/ fallthrough clauses and execute if nothing else executed above const start = clauses.findIndex(c => ts.isDefaultClause(c)); diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap index 4f2b7573d..931080a1d 100644 --- a/test/unit/__snapshots__/switch.spec.ts.snap +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -1,228 +1,66 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`switch collapses empty case and minimizes conditions 1`] = ` +exports[`switch produces optimal output 1`] = ` "require(\\"lualib_bundle\\"); local ____exports = {} function ____exports.__main(self) local out = {} repeat - local ____switch3 = 5 + local ____switch3 = 0 local ____cond3 = ((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2) if ____cond3 then __TS__ArrayPush(out, \\"0,1,2\\") end - ____cond3 = ____cond3 or (____switch3 == 3) if ____cond3 then - __TS__ArrayPush(out, \\"3\\") - break + __TS__ArrayPush(out, \\"default\\") end - ____cond3 = ____cond3 or (____switch3 == 4) + ____cond3 = ____cond3 or (____switch3 == 3) if ____cond3 then - __TS__ArrayPush(out, \\"4\\") + __TS__ArrayPush(out, \\"3\\") end if not ____cond3 then __TS__ArrayPush(out, \\"default\\") - __TS__ArrayPush(out, \\"4\\") - end - until true - return out -end -return ____exports" -`; - -exports[`switch handles side-effects (0) 1`] = ` -"require(\\"lualib_bundle\\"); -local ____exports = {} -function ____exports.__main(self) - local out = {} - local y = 0 - local function foo(self) - return (function() - local ____tmp = y - y = ____tmp + 1 - return ____tmp - end)() - end - local x = 0 - repeat - local ____switch5 = x - local ____cond5 = ____switch5 == foo(nil) - if ____cond5 then - __TS__ArrayPush(out, 1) - end - ____cond5 = ____cond5 or (____switch5 == foo(nil)) - if ____cond5 then - __TS__ArrayPush(out, 2) - end - ____cond5 = ____cond5 or (____switch5 == foo(nil)) - if ____cond5 then - __TS__ArrayPush(out, 3) - end - if ____cond5 then - __TS__ArrayPush(out, \\"default\\") - end - if not ____cond5 then - __TS__ArrayPush(out, \\"default\\") - end - until true - __TS__ArrayPush(out, y) - return out -end -return ____exports" -`; - -exports[`switch handles side-effects (1) 1`] = ` -"require(\\"lualib_bundle\\"); -local ____exports = {} -function ____exports.__main(self) - local out = {} - local y = 0 - local function foo(self) - return (function() - local ____tmp = y - y = ____tmp + 1 - return ____tmp - end)() - end - local x = 1 - repeat - local ____switch5 = x - local ____cond5 = ____switch5 == foo(nil) - if ____cond5 then - __TS__ArrayPush(out, 1) - end - ____cond5 = ____cond5 or (____switch5 == foo(nil)) - if ____cond5 then - __TS__ArrayPush(out, 2) - end - ____cond5 = ____cond5 or (____switch5 == foo(nil)) - if ____cond5 then - __TS__ArrayPush(out, 3) - end - if ____cond5 then - __TS__ArrayPush(out, \\"default\\") - end - if not ____cond5 then - __TS__ArrayPush(out, \\"default\\") + __TS__ArrayPush(out, \\"3\\") end until true - __TS__ArrayPush(out, y) - return out -end -return ____exports" -`; - -exports[`switch handles side-effects (2) 1`] = ` -"require(\\"lualib_bundle\\"); -local ____exports = {} -function ____exports.__main(self) - local out = {} - local y = 0 - local function foo(self) - return (function() - local ____tmp = y - y = ____tmp + 1 - return ____tmp - end)() - end - local x = 2 repeat - local ____switch5 = x - local ____cond5 = ____switch5 == foo(nil) - if ____cond5 then - __TS__ArrayPush(out, 1) - end - ____cond5 = ____cond5 or (____switch5 == foo(nil)) - if ____cond5 then - __TS__ArrayPush(out, 2) - end - ____cond5 = ____cond5 or (____switch5 == foo(nil)) - if ____cond5 then - __TS__ArrayPush(out, 3) - end - if ____cond5 then - __TS__ArrayPush(out, \\"default\\") + local ____switch4 = 2 + local ____cond4 = (____switch4 == 0) or (____switch4 == 1) + if ____cond4 then + __TS__ArrayPush(out, \\"0,1\\") + break end - if not ____cond5 then + if not ____cond4 then __TS__ArrayPush(out, \\"default\\") + __TS__ArrayPush(out, \\"0,1\\") + break end until true - __TS__ArrayPush(out, y) - return out -end -return ____exports" -`; - -exports[`switch handles side-effects (3) 1`] = ` -"require(\\"lualib_bundle\\"); -local ____exports = {} -function ____exports.__main(self) - local out = {} - local y = 0 - local function foo(self) - return (function() - local ____tmp = y - y = ____tmp + 1 - return ____tmp - end)() - end - local x = 3 repeat - local ____switch5 = x - local ____cond5 = ____switch5 == foo(nil) + local ____switch5 = 5 + local ____cond5 = ____switch5 == 0 if ____cond5 then - __TS__ArrayPush(out, 1) + __TS__ArrayPush(out, \\"0\\") end - ____cond5 = ____cond5 or (____switch5 == foo(nil)) + ____cond5 = ____cond5 or (____switch5 == 1) if ____cond5 then - __TS__ArrayPush(out, 2) + __TS__ArrayPush(out, \\"1\\") + break end - ____cond5 = ____cond5 or (____switch5 == foo(nil)) + ____cond5 = ____cond5 or (____switch5 == 2) if ____cond5 then - __TS__ArrayPush(out, 3) + __TS__ArrayPush(out, \\"0,1,2\\") end + ____cond5 = ____cond5 or (____switch5 == 3) if ____cond5 then - __TS__ArrayPush(out, \\"default\\") + __TS__ArrayPush(out, \\"3\\") + break end if not ____cond5 then __TS__ArrayPush(out, \\"default\\") end until true - __TS__ArrayPush(out, y) return out end return ____exports" `; - -exports[`switch uses elseif 1`] = ` -"local ____exports = {} -function ____exports.__main(self) - local result = -1 - repeat - local ____switch3 = 2 - local ____cond3 = ____switch3 == 0 - if ____cond3 then - do - result = 200 - break - end - end - ____cond3 = ____cond3 or (____switch3 == 1) - if ____cond3 then - do - result = 100 - break - end - end - ____cond3 = ____cond3 or (____switch3 == 2) - if ____cond3 then - do - result = 1 - break - end - end - until true - return result -end -return ____exports" -`; diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index dae3475c3..8f26faa3a 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -264,7 +264,7 @@ test.each([0, 1, 2, 3])("switchWithBracketsBreakInInternalLoop (%p)", inp => { `.expectToMatchJsResult(); }); -test("switch uses elseif", () => { +test("switch executes only one", () => { util.testFunction` let result: number = -1; @@ -286,9 +286,7 @@ test("switch uses elseif", () => { } return result; - ` - .expectLuaToMatchSnapshot() - .expectToMatchJsResult(); + `.expectToMatchJsResult(); }); // https://github.com/TypeScriptToLua/TypeScriptToLua/issues/967 @@ -378,12 +376,35 @@ test("switch does not pollute parent scope", () => { `.expectToMatchJsResult(); }); -test("switch collapses empty case and minimizes conditions", () => { +test("switch produces optimal output", () => { util.testFunction` const out = []; + switch (0 as number) { + case 0: + case 1: + case 2: + out.push("0,1,2"); + default: + out.push("default"); + case 3: + out.push("3"); + } + + switch (2 as number) { + default: + out.push("default"); + case 0: + case 1: + out.push("0,1"); + break; + } + switch (5 as number) { case 0: + out.push("0"); case 1: + out.push("1"); + break; case 2: out.push("0,1,2"); case 3: @@ -391,8 +412,6 @@ test("switch collapses empty case and minimizes conditions", () => { break; default: out.push("default"); - case 4: - out.push("4"); } return out; ` @@ -423,7 +442,5 @@ test.each([0, 1, 2, 3])("switch handles side-effects (%p)", inp => { out.push(y); return out; - ` - .expectLuaToMatchSnapshot() - .expectToMatchJsResult(); + `.expectToMatchJsResult(); }); From fd7a77ab5c756b2a881d8b4009c5e6519d601ef0 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Tue, 24 Aug 2021 12:17:52 -0700 Subject: [PATCH 13/24] chore: remove dead comment --- src/transformation/visitors/switch.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 5587a7ef2..1555b9368 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -39,8 +39,6 @@ export const transformSwitchStatement: FunctionVisitor = (st } } else { // Build up the condition for each if statement - // Fallthrough is handled by accepting the last condition as an additional or clause - // Default is the not of all known case expressions let isInitialCondition = true; let condition: lua.Expression | undefined = undefined; for (let i = 0; i < statement.caseBlock.clauses.length; i++) { From 09806b9db5a540b3997f9f548e45d0c6b492d050 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Wed, 25 Aug 2021 12:54:49 -0700 Subject: [PATCH 14/24] fix: handle no initial statment fallthrough --- src/transformation/visitors/switch.ts | 38 +++++++++++++------- test/unit/__snapshots__/switch.spec.ts.snap | 40 +++++++++++++++++++++ test/unit/switch.spec.ts | 15 ++++++++ 3 files changed, 80 insertions(+), 13 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 1555b9368..87dc7a59a 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -1,6 +1,5 @@ import * as ts from "typescript"; import * as lua from "../../LuaAST"; -import { assert } from "../../utils"; import { FunctionVisitor } from "../context"; import { performHoisting, popScope, pushScope, ScopeType } from "../utils/scope"; @@ -86,8 +85,15 @@ export const transformSwitchStatement: FunctionVisitor = (st ) ); isInitialCondition = false; - } else { - assert(!isInitialCondition, "Default clause should never be the initial condition"); + } else if (isInitialCondition) { + // If we have fallen through to the default here, we need to initialize the default condition + statements.push( + lua.createVariableDeclarationStatement( + conditionVariable, + condition ?? lua.createBooleanLiteral(false) + ) + ); + isInitialCondition = false; } // Push if statement for case @@ -102,19 +108,25 @@ export const transformSwitchStatement: FunctionVisitor = (st condition = undefined; } - // Amalgamate the default w/ fallthrough clauses and execute if nothing else executed above + // If no conditions above match, we need to create the final default case code-path + // As we only handle fallthrough into defaults in the previous if statement chain const start = clauses.findIndex(c => ts.isDefaultClause(c)); if (start >= 0) { - const end = statement.caseBlock.clauses - .slice(start) - .findIndex(clause => containsBreakOrReturn([...clause.statements])); - const defaultStatements = statement.caseBlock.clauses - .slice(start, end >= 0 ? end + start + 1 : undefined) - .reduce( - (statements, clause) => [...statements, ...context.transformStatements(clause.statements)], - [] - ); + // Find the last clause that we can fallthrough to + const end = statement.caseBlock.clauses.findIndex( + (clause, index) => index >= start && containsBreakOrReturn([...clause.statements]) + ); + + // Combine the default and all fallthrough statements + const defaultStatements: lua.Statement[] = []; + for (const { statements } of statement.caseBlock.clauses.slice( + start, + end >= 0 ? end + start + 1 : undefined + )) { + defaultStatements.push(...context.transformStatements(statements)); + } + // Add the default clause if it has any statements if (defaultStatements.length) { statements.push( lua.createIfStatement( diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap index 931080a1d..29e2c8f31 100644 --- a/test/unit/__snapshots__/switch.spec.ts.snap +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -1,5 +1,45 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`switch empty fallthrough to default (0) 1`] = ` +"require(\\"lualib_bundle\\"); +local ____exports = {} +function ____exports.__main(self) + local out = {} + repeat + local ____switch3 = 0 + local ____cond3 = ____switch3 == 1 + if ____cond3 then + __TS__ArrayPush(out, \\"default\\") + end + if not ____cond3 then + __TS__ArrayPush(out, \\"default\\") + end + until true + return out +end +return ____exports" +`; + +exports[`switch empty fallthrough to default (1) 1`] = ` +"require(\\"lualib_bundle\\"); +local ____exports = {} +function ____exports.__main(self) + local out = {} + repeat + local ____switch3 = 1 + local ____cond3 = ____switch3 == 1 + if ____cond3 then + __TS__ArrayPush(out, \\"default\\") + end + if not ____cond3 then + __TS__ArrayPush(out, \\"default\\") + end + until true + return out +end +return ____exports" +`; + exports[`switch produces optimal output 1`] = ` "require(\\"lualib_bundle\\"); local ____exports = {} diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index 8f26faa3a..a143f8d73 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -364,6 +364,21 @@ test("switch fallthrough stops after default", () => { `.expectToMatchJsResult(); }); +test.each([0, 1])("switch empty fallthrough to default (%p)", inp => { + util.testFunction` + const out = []; + switch (${inp} as number) { + case 1: + default: + out.push("default"); + + } + return out; + ` + .expectLuaToMatchSnapshot() + .expectToMatchJsResult(); +}); + test("switch does not pollute parent scope", () => { util.testFunction` let x: number = 0; From 854af79a7e379015f818552cbd94b1b9662cbb1a Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Wed, 25 Aug 2021 13:37:03 -0700 Subject: [PATCH 15/24] test: async case --- test/unit/switch.spec.ts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index a143f8d73..0d8540bc1 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -459,3 +459,31 @@ test.each([0, 1, 2, 3])("switch handles side-effects (%p)", inp => { return out; `.expectToMatchJsResult(); }); + +test.each([0, 1, 2, 3])("switch handles async side-effects (%p)", inp => { + util.testFunction` + (async () => { + const out = []; + + let y = 0; + async function foo() { + return new Promise((resolve) => y++ && resolve(0)); + } + + let x = ${inp} as number; + switch (x) { + case await foo(): + out.push(1); + case await foo(): + out.push(2); + case await foo(): + out.push(3); + default: + out.push("default"); + } + + out.push(y); + return out; + })(); + `.expectToMatchJsResult(); +}); From 18cc6c2c6cfdbc539b45d0c251348b18003bfe25 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Wed, 25 Aug 2021 16:03:08 -0700 Subject: [PATCH 16/24] fix: containsBreakOrReturn & final default fallthrough --- src/transformation/visitors/switch.ts | 15 +++----- test/unit/__snapshots__/switch.spec.ts.snap | 42 +++++++++++++-------- test/unit/switch.spec.ts | 6 ++- 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 87dc7a59a..1b2ab5be2 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -7,10 +7,9 @@ const containsBreakOrReturn = (statements: ts.Node[]): boolean => { for (const s of statements) { if (ts.isBreakStatement(s) || ts.isReturnStatement(s)) { return true; - } else if (!ts.isBlock(s)) { - // Can only ensure a break scoped in a block is deterministic - continue; - } else if (containsBreakOrReturn(s.getChildren())) { + } else if (ts.isBlock(s) && containsBreakOrReturn(s.getChildren())) { + return true; + } else if (s.kind === ts.SyntaxKind.SyntaxList && containsBreakOrReturn(s.getChildren())) { return true; } } @@ -119,12 +118,8 @@ export const transformSwitchStatement: FunctionVisitor = (st // Combine the default and all fallthrough statements const defaultStatements: lua.Statement[] = []; - for (const { statements } of statement.caseBlock.clauses.slice( - start, - end >= 0 ? end + start + 1 : undefined - )) { - defaultStatements.push(...context.transformStatements(statements)); - } + const clauses = statement.caseBlock.clauses.slice(start, end >= 0 ? end + 1 : undefined); + clauses.forEach(c => defaultStatements.push(...context.transformStatements(c.statements))); // Add the default clause if it has any statements if (defaultStatements.length) { diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap index 29e2c8f31..d23458b07 100644 --- a/test/unit/__snapshots__/switch.spec.ts.snap +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -56,47 +56,57 @@ function ____exports.__main(self) end ____cond3 = ____cond3 or (____switch3 == 3) if ____cond3 then - __TS__ArrayPush(out, \\"3\\") + do + __TS__ArrayPush(out, \\"3\\") + break + end + end + ____cond3 = ____cond3 or (____switch3 == 4) + if ____cond3 then + __TS__ArrayPush(out, \\"4\\") end if not ____cond3 then __TS__ArrayPush(out, \\"default\\") - __TS__ArrayPush(out, \\"3\\") + do + __TS__ArrayPush(out, \\"3\\") + break + end end until true repeat - local ____switch4 = 2 - local ____cond4 = (____switch4 == 0) or (____switch4 == 1) - if ____cond4 then + local ____switch6 = 2 + local ____cond6 = (____switch6 == 0) or (____switch6 == 1) + if ____cond6 then __TS__ArrayPush(out, \\"0,1\\") break end - if not ____cond4 then + if not ____cond6 then __TS__ArrayPush(out, \\"default\\") __TS__ArrayPush(out, \\"0,1\\") break end until true repeat - local ____switch5 = 5 - local ____cond5 = ____switch5 == 0 - if ____cond5 then + local ____switch7 = 5 + local ____cond7 = ____switch7 == 0 + if ____cond7 then __TS__ArrayPush(out, \\"0\\") end - ____cond5 = ____cond5 or (____switch5 == 1) - if ____cond5 then + ____cond7 = ____cond7 or (____switch7 == 1) + if ____cond7 then __TS__ArrayPush(out, \\"1\\") break end - ____cond5 = ____cond5 or (____switch5 == 2) - if ____cond5 then + ____cond7 = ____cond7 or (____switch7 == 2) + if ____cond7 then __TS__ArrayPush(out, \\"0,1,2\\") end - ____cond5 = ____cond5 or (____switch5 == 3) - if ____cond5 then + ____cond7 = ____cond7 or (____switch7 == 3) + if ____cond7 then __TS__ArrayPush(out, \\"3\\") break end - if not ____cond5 then + if not ____cond7 then __TS__ArrayPush(out, \\"default\\") end until true diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index 0d8540bc1..05fad8038 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -401,8 +401,12 @@ test("switch produces optimal output", () => { out.push("0,1,2"); default: out.push("default"); - case 3: + case 3: { out.push("3"); + break; + } + case 4: + out.push("4"); } switch (2 as number) { From 84be99d24a8922fa4ae58f76da73b6130e7bdb45 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Wed, 25 Aug 2021 16:16:09 -0700 Subject: [PATCH 17/24] refactor: coalesceCondition function --- src/transformation/visitors/switch.ts | 49 ++++++++++++++++----------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 1b2ab5be2..217508cc7 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -1,6 +1,6 @@ import * as ts from "typescript"; import * as lua from "../../LuaAST"; -import { FunctionVisitor } from "../context"; +import { FunctionVisitor, TransformationContext } from "../context"; import { performHoisting, popScope, pushScope, ScopeType } from "../utils/scope"; const containsBreakOrReturn = (statements: ts.Node[]): boolean => { @@ -17,6 +17,33 @@ const containsBreakOrReturn = (statements: ts.Node[]): boolean => { return false; }; +const coalesceCondition = ( + condition: lua.Expression | undefined, + switchVariable: lua.Identifier, + expression: ts.Expression, + context: TransformationContext +): lua.Expression => { + // Coalesce skipped statements + if (condition) { + return lua.createBinaryExpression( + condition, + lua.createBinaryExpression( + switchVariable, + context.transformExpression(expression), + lua.SyntaxKind.EqualityOperator + ), + lua.SyntaxKind.OrOperator + ); + } + + // Next condition + return lua.createBinaryExpression( + switchVariable, + context.transformExpression(expression), + lua.SyntaxKind.EqualityOperator + ); +}; + export const transformSwitchStatement: FunctionVisitor = (statement, context) => { const scope = pushScope(context, ScopeType.Switch); @@ -51,25 +78,7 @@ export const transformSwitchStatement: FunctionVisitor = (st // Compute the condition for the if statement if (!ts.isDefaultClause(clause)) { - if (condition) { - // Coalesce skipped statements - condition = lua.createBinaryExpression( - condition, - lua.createBinaryExpression( - switchVariable, - context.transformExpression(clause.expression), - lua.SyntaxKind.EqualityOperator - ), - lua.SyntaxKind.OrOperator - ); - } else { - // Next condition - condition = lua.createBinaryExpression( - switchVariable, - context.transformExpression(clause.expression), - lua.SyntaxKind.EqualityOperator - ); - } + condition = coalesceCondition(condition, switchVariable, clause.expression, context); // Skip empty clauses if (clause.statements.length === 0) continue; From afc9398d0d280376df52f71571220585295835f3 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Wed, 25 Aug 2021 16:37:19 -0700 Subject: [PATCH 18/24] fix: do not copy statements to check for break --- src/transformation/visitors/switch.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 217508cc7..3af2a49fd 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -3,8 +3,8 @@ import * as lua from "../../LuaAST"; import { FunctionVisitor, TransformationContext } from "../context"; import { performHoisting, popScope, pushScope, ScopeType } from "../utils/scope"; -const containsBreakOrReturn = (statements: ts.Node[]): boolean => { - for (const s of statements) { +const containsBreakOrReturn = (nodes: Iterable): boolean => { + for (const s of nodes) { if (ts.isBreakStatement(s) || ts.isReturnStatement(s)) { return true; } else if (ts.isBlock(s) && containsBreakOrReturn(s.getChildren())) { @@ -72,7 +72,7 @@ export const transformSwitchStatement: FunctionVisitor = (st // Skip redundant default clauses, will be handled in final default case if (i === 0 && ts.isDefaultClause(clause)) continue; - if (ts.isDefaultClause(clause) && previousClause && containsBreakOrReturn([...previousClause.statements])) { + if (ts.isDefaultClause(clause) && previousClause && containsBreakOrReturn(previousClause.statements)) { continue; } @@ -122,7 +122,7 @@ export const transformSwitchStatement: FunctionVisitor = (st if (start >= 0) { // Find the last clause that we can fallthrough to const end = statement.caseBlock.clauses.findIndex( - (clause, index) => index >= start && containsBreakOrReturn([...clause.statements]) + (clause, index) => index >= start && containsBreakOrReturn(clause.statements) ); // Combine the default and all fallthrough statements From e710600bf5e7a3c318699dad0525f3c1d5cc9a73 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Wed, 25 Aug 2021 16:53:19 -0700 Subject: [PATCH 19/24] refactor: cleanup clauses access --- src/transformation/visitors/switch.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 3af2a49fd..5dc00effc 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -66,9 +66,9 @@ export const transformSwitchStatement: FunctionVisitor = (st // Build up the condition for each if statement let isInitialCondition = true; let condition: lua.Expression | undefined = undefined; - for (let i = 0; i < statement.caseBlock.clauses.length; i++) { - const clause = statement.caseBlock.clauses[i]; - const previousClause: ts.CaseOrDefaultClause | undefined = statement.caseBlock.clauses[i - 1]; + for (let i = 0; i < clauses.length; i++) { + const clause = clauses[i]; + const previousClause: ts.CaseOrDefaultClause | undefined = clauses[i - 1]; // Skip redundant default clauses, will be handled in final default case if (i === 0 && ts.isDefaultClause(clause)) continue; @@ -121,14 +121,15 @@ export const transformSwitchStatement: FunctionVisitor = (st const start = clauses.findIndex(c => ts.isDefaultClause(c)); if (start >= 0) { // Find the last clause that we can fallthrough to - const end = statement.caseBlock.clauses.findIndex( + const end = clauses.findIndex( (clause, index) => index >= start && containsBreakOrReturn(clause.statements) ); // Combine the default and all fallthrough statements const defaultStatements: lua.Statement[] = []; - const clauses = statement.caseBlock.clauses.slice(start, end >= 0 ? end + 1 : undefined); - clauses.forEach(c => defaultStatements.push(...context.transformStatements(c.statements))); + clauses + .slice(start, end >= 0 ? end + 1 : undefined) + .forEach(c => defaultStatements.push(...context.transformStatements(c.statements))); // Add the default clause if it has any statements if (defaultStatements.length) { From ef92a2e00b20c6a9469690e5d61824b0bb1f17c3 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Wed, 25 Aug 2021 16:57:35 -0700 Subject: [PATCH 20/24] chore(nit): comment --- src/transformation/visitors/switch.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 5dc00effc..78e9919e5 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -116,8 +116,8 @@ export const transformSwitchStatement: FunctionVisitor = (st condition = undefined; } - // If no conditions above match, we need to create the final default case code-path - // As we only handle fallthrough into defaults in the previous if statement chain + // If no conditions above match, we need to create the final default case code-path, + // as we only handle fallthrough into defaults in the previous if statement chain const start = clauses.findIndex(c => ts.isDefaultClause(c)); if (start >= 0) { // Find the last clause that we can fallthrough to From 275030b12df64fb04d8a819d4e96edbcbb9c2683 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Thu, 26 Aug 2021 06:12:56 -0700 Subject: [PATCH 21/24] fix: ensure final clause is evaluated --- src/transformation/visitors/switch.ts | 4 ++-- test/unit/switch.spec.ts | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 78e9919e5..0ed380e5b 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -80,8 +80,8 @@ export const transformSwitchStatement: FunctionVisitor = (st if (!ts.isDefaultClause(clause)) { condition = coalesceCondition(condition, switchVariable, clause.expression, context); - // Skip empty clauses - if (clause.statements.length === 0) continue; + // Skip empty clauses unless final clause (i.e side-effects) + if (i !== clauses.length - 1 && clause.statements.length === 0) continue; // Declare or assign condition variable statements.push( diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index 05fad8038..da9f868e1 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -457,6 +457,7 @@ test.each([0, 1, 2, 3])("switch handles side-effects (%p)", inp => { out.push(3); default: out.push("default"); + case foo(): } out.push(y); From 76166df40c1fd53d71495dbe7332467c965a8c51 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Thu, 26 Aug 2021 08:39:36 -0700 Subject: [PATCH 22/24] refactor: simplify and cleanup output --- src/transformation/visitors/switch.ts | 46 ++++++----- test/unit/__snapshots__/switch.spec.ts.snap | 62 ++++----------- test/unit/switch.spec.ts | 86 ++++++++------------- 3 files changed, 76 insertions(+), 118 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 0ed380e5b..155ba21b8 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -93,24 +93,32 @@ export const transformSwitchStatement: FunctionVisitor = (st ) ); isInitialCondition = false; - } else if (isInitialCondition) { - // If we have fallen through to the default here, we need to initialize the default condition - statements.push( - lua.createVariableDeclarationStatement( - conditionVariable, - condition ?? lua.createBooleanLiteral(false) - ) - ); - isInitialCondition = false; + } else { + // If the default is not fallen into skip it and handle in the final default output instead + if (previousClause && containsBreakOrReturn(previousClause.statements)) { + continue; + } + + // If the default is proceeded by empty clauses and will be emitted we may need to initialize the condition + if (isInitialCondition) { + statements.push( + lua.createVariableDeclarationStatement( + conditionVariable, + condition ?? lua.createBooleanLiteral(false) + ) + ); + isInitialCondition = false; + } + } + + // Transform the clause and append the final break statement if necessary + const clauseStatements = context.transformStatements(clause.statements); + if (i === clauses.length - 1 && !containsBreakOrReturn(clause.statements)) { + clauseStatements.push(lua.createBreakStatement()); } // Push if statement for case - statements.push( - lua.createIfStatement( - conditionVariable, - lua.createBlock(context.transformStatements(clause.statements)) - ) - ); + statements.push(lua.createIfStatement(conditionVariable, lua.createBlock(clauseStatements))); // Clear condition for next clause condition = undefined; @@ -132,13 +140,9 @@ export const transformSwitchStatement: FunctionVisitor = (st .forEach(c => defaultStatements.push(...context.transformStatements(c.statements))); // Add the default clause if it has any statements + // The switch will always break on the final clause and skip execution if valid to do so if (defaultStatements.length) { - statements.push( - lua.createIfStatement( - lua.createUnaryExpression(conditionVariable, lua.SyntaxKind.NotOperator), - lua.createBlock(defaultStatements) - ) - ); + statements.push(lua.createDoStatement(defaultStatements)); } } } diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap index d23458b07..975b33186 100644 --- a/test/unit/__snapshots__/switch.spec.ts.snap +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -10,8 +10,9 @@ function ____exports.__main(self) local ____cond3 = ____switch3 == 1 if ____cond3 then __TS__ArrayPush(out, \\"default\\") + break end - if not ____cond3 then + do __TS__ArrayPush(out, \\"default\\") end until true @@ -30,8 +31,9 @@ function ____exports.__main(self) local ____cond3 = ____switch3 == 1 if ____cond3 then __TS__ArrayPush(out, \\"default\\") + break end - if not ____cond3 then + do __TS__ArrayPush(out, \\"default\\") end until true @@ -44,15 +46,14 @@ exports[`switch produces optimal output 1`] = ` "require(\\"lualib_bundle\\"); local ____exports = {} function ____exports.__main(self) + local x = 0 local out = {} repeat local ____switch3 = 0 local ____cond3 = ((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2) if ____cond3 then __TS__ArrayPush(out, \\"0,1,2\\") - end - if ____cond3 then - __TS__ArrayPush(out, \\"default\\") + break end ____cond3 = ____cond3 or (____switch3 == 3) if ____cond3 then @@ -63,53 +64,24 @@ function ____exports.__main(self) end ____cond3 = ____cond3 or (____switch3 == 4) if ____cond3 then - __TS__ArrayPush(out, \\"4\\") + break end - if not ____cond3 then - __TS__ArrayPush(out, \\"default\\") + do + x = x + 1 + __TS__ArrayPush( + out, + \\"default = \\" .. tostring(x) + ) do __TS__ArrayPush(out, \\"3\\") break end end until true - repeat - local ____switch6 = 2 - local ____cond6 = (____switch6 == 0) or (____switch6 == 1) - if ____cond6 then - __TS__ArrayPush(out, \\"0,1\\") - break - end - if not ____cond6 then - __TS__ArrayPush(out, \\"default\\") - __TS__ArrayPush(out, \\"0,1\\") - break - end - until true - repeat - local ____switch7 = 5 - local ____cond7 = ____switch7 == 0 - if ____cond7 then - __TS__ArrayPush(out, \\"0\\") - end - ____cond7 = ____cond7 or (____switch7 == 1) - if ____cond7 then - __TS__ArrayPush(out, \\"1\\") - break - end - ____cond7 = ____cond7 or (____switch7 == 2) - if ____cond7 then - __TS__ArrayPush(out, \\"0,1,2\\") - end - ____cond7 = ____cond7 or (____switch7 == 3) - if ____cond7 then - __TS__ArrayPush(out, \\"3\\") - break - end - if not ____cond7 then - __TS__ArrayPush(out, \\"default\\") - end - until true + __TS__ArrayPush( + out, + tostring(x) + ) return out end return ____exports" diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index da9f868e1..55ac0c5d2 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -264,10 +264,9 @@ test.each([0, 1, 2, 3])("switchWithBracketsBreakInInternalLoop (%p)", inp => { `.expectToMatchJsResult(); }); -test("switch executes only one", () => { +test("switch executes only one clause", () => { util.testFunction` let result: number = -1; - switch (2 as number) { case 0: { result = 200; @@ -284,7 +283,6 @@ test("switch executes only one", () => { break; } } - return result; `.expectToMatchJsResult(); }); @@ -391,54 +389,7 @@ test("switch does not pollute parent scope", () => { `.expectToMatchJsResult(); }); -test("switch produces optimal output", () => { - util.testFunction` - const out = []; - switch (0 as number) { - case 0: - case 1: - case 2: - out.push("0,1,2"); - default: - out.push("default"); - case 3: { - out.push("3"); - break; - } - case 4: - out.push("4"); - } - - switch (2 as number) { - default: - out.push("default"); - case 0: - case 1: - out.push("0,1"); - break; - } - - switch (5 as number) { - case 0: - out.push("0"); - case 1: - out.push("1"); - break; - case 2: - out.push("0,1,2"); - case 3: - out.push("3"); - break; - default: - out.push("default"); - } - return out; - ` - .expectLuaToMatchSnapshot() - .expectToMatchJsResult(); -}); - -test.each([0, 1, 2, 3])("switch handles side-effects (%p)", inp => { +test.each([0, 1, 2, 3, 4])("switch handles side-effects (%p)", inp => { util.testFunction` const out = []; @@ -465,7 +416,7 @@ test.each([0, 1, 2, 3])("switch handles side-effects (%p)", inp => { `.expectToMatchJsResult(); }); -test.each([0, 1, 2, 3])("switch handles async side-effects (%p)", inp => { +test.each([0, 1, 2, 3, 4])("switch handles async side-effects (%p)", inp => { util.testFunction` (async () => { const out = []; @@ -485,6 +436,7 @@ test.each([0, 1, 2, 3])("switch handles async side-effects (%p)", inp => { out.push(3); default: out.push("default"); + case await foo(): } out.push(y); @@ -492,3 +444,33 @@ test.each([0, 1, 2, 3])("switch handles async side-effects (%p)", inp => { })(); `.expectToMatchJsResult(); }); + +const optimalOutput = (c: number) => util.testFunction` + let x: number = 0; + const out = []; + switch (${c} as number) { + case 0: + case 1: + case 2: + out.push("0,1,2"); + break; + default: + x++; + out.push("default = " + x); + case 3: { + out.push("3"); + break; + } + case 4: + } + out.push(x.toString()); + return out; +`; + +test("switch produces optimal output", () => { + optimalOutput(0).expectLuaToMatchSnapshot(); +}); + +test.each([0, 1, 2, 3, 4, 5])("switch produces valid optimal output (%p)", inp => { + optimalOutput(inp).expectToMatchJsResult(); +}); From cc07df34808c4d1e4b80d042f665db5e680e05c4 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Fri, 27 Aug 2021 20:12:49 -0700 Subject: [PATCH 23/24] fix: remove redundant final default clause --- src/transformation/visitors/switch.ts | 8 +++----- test/unit/__snapshots__/switch.spec.ts.snap | 8 -------- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 155ba21b8..59f4a41c9 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -94,11 +94,6 @@ export const transformSwitchStatement: FunctionVisitor = (st ); isInitialCondition = false; } else { - // If the default is not fallen into skip it and handle in the final default output instead - if (previousClause && containsBreakOrReturn(previousClause.statements)) { - continue; - } - // If the default is proceeded by empty clauses and will be emitted we may need to initialize the condition if (isInitialCondition) { statements.push( @@ -109,6 +104,9 @@ export const transformSwitchStatement: FunctionVisitor = (st ); isInitialCondition = false; } + + // Skip default clause if is the last clause, as it will fallthrough to the final default below + if (i === clauses.length - 1) continue; } // Transform the clause and append the final break statement if necessary diff --git a/test/unit/__snapshots__/switch.spec.ts.snap b/test/unit/__snapshots__/switch.spec.ts.snap index 975b33186..0fa333d93 100644 --- a/test/unit/__snapshots__/switch.spec.ts.snap +++ b/test/unit/__snapshots__/switch.spec.ts.snap @@ -8,10 +8,6 @@ function ____exports.__main(self) repeat local ____switch3 = 0 local ____cond3 = ____switch3 == 1 - if ____cond3 then - __TS__ArrayPush(out, \\"default\\") - break - end do __TS__ArrayPush(out, \\"default\\") end @@ -29,10 +25,6 @@ function ____exports.__main(self) repeat local ____switch3 = 1 local ____cond3 = ____switch3 == 1 - if ____cond3 then - __TS__ArrayPush(out, \\"default\\") - break - end do __TS__ArrayPush(out, \\"default\\") end From a39026bd50ac8e5506031e297e2ee021bab196e3 Mon Sep 17 00:00:00 2001 From: Justin Walsh Date: Tue, 31 Aug 2021 13:00:48 -0700 Subject: [PATCH 24/24] fix: ensure we evaluate empty fallthrough clause --- src/transformation/visitors/switch.ts | 18 ++++++++-- test/unit/switch.spec.ts | 48 +++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/transformation/visitors/switch.ts b/src/transformation/visitors/switch.ts index 59f4a41c9..1a05bae41 100644 --- a/src/transformation/visitors/switch.ts +++ b/src/transformation/visitors/switch.ts @@ -102,11 +102,25 @@ export const transformSwitchStatement: FunctionVisitor = (st condition ?? lua.createBooleanLiteral(false) ) ); + + // Clear condition ot ensure it is not evaluated twice + condition = undefined; isInitialCondition = false; } - // Skip default clause if is the last clause, as it will fallthrough to the final default below - if (i === clauses.length - 1) continue; + // Allow default to fallthrough to final default clause + if (i === clauses.length - 1) { + // Evaluate the final condition that we may be skipping + if (condition) { + statements.push( + lua.createAssignmentStatement( + conditionVariable, + lua.createBinaryExpression(conditionVariable, condition, lua.SyntaxKind.OrOperator) + ) + ); + } + continue; + } } // Transform the clause and append the final break statement if necessary diff --git a/test/unit/switch.spec.ts b/test/unit/switch.spec.ts index 55ac0c5d2..6ed965595 100644 --- a/test/unit/switch.spec.ts +++ b/test/unit/switch.spec.ts @@ -416,6 +416,54 @@ test.each([0, 1, 2, 3, 4])("switch handles side-effects (%p)", inp => { `.expectToMatchJsResult(); }); +test.each([1, 2])("switch handles side-effects with empty fallthrough (%p)", inp => { + util.testFunction` + const out = []; + + let y = 0; + function foo() { + return y++; + } + + let x = 0 as number; + switch (x) { + // empty fallthrough 1 or many times + ${new Array(inp).fill("case foo():").join("\n")} + default: + out.push("default"); + + } + + out.push(y); + return out; + `.expectToMatchJsResult(); +}); + +test.each([1, 2])("switch handles side-effects with empty fallthrough (preceding clause) (%p)", inp => { + util.testFunction` + const out = []; + + let y = 0; + function foo() { + return y++; + } + + let x = 0 as number; + switch (x) { + case 1: + out.push(1); + // empty fallthrough 1 or many times + ${new Array(inp).fill("case foo():").join("\n")} + default: + out.push("default"); + + } + + out.push(y); + return out; + `.expectToMatchJsResult(); +}); + test.each([0, 1, 2, 3, 4])("switch handles async side-effects (%p)", inp => { util.testFunction` (async () => {