Skip to content

Commit 0867793

Browse files
thejustinwalshthejustinwalsh
committed
fix: non duplicating case body switch
1 parent 504fd1a commit 0867793

4 files changed

Lines changed: 121 additions & 52 deletions

File tree

‎src/transformation/visitors/break-continue.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { findScope, ScopeType } from "../utils/scope";
88
export const transformBreakStatement: FunctionVisitor<ts.BreakStatement> = (breakStatement, context) => {
99
const breakableScope = findScope(context, ScopeType.Loop | ScopeType.Switch);
1010
if (breakableScope?.type === ScopeType.Switch) {
11+
// Break is handled by the switch statement (see transformSwitchStatement)
1112
return lua.createBreakStatement(breakStatement);
1213
} else {
1314
return lua.createBreakStatement(breakStatement);

‎src/transformation/visitors/switch.ts‎

Lines changed: 68 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,14 @@ import * as lua from "../../LuaAST";
33
import { FunctionVisitor } from "../context";
44
import { performHoisting, popScope, pushScope, ScopeType } from "../utils/scope";
55

6-
const containsBreakStatement = (statements: ts.Node[]): boolean => {
6+
const containsBreakOrReturn = (statements: ts.Node[]): boolean => {
77
for (const s of statements) {
8-
if (
9-
ts.isIfStatement(s) ||
10-
ts.isSwitchStatement(s) ||
11-
ts.isWhileStatement(s) ||
12-
ts.isDoStatement(s) ||
13-
ts.isForStatement(s) ||
14-
ts.isForInStatement(s) ||
15-
ts.isForOfStatement(s)
16-
) {
17-
// Ignore: Break statements are valid as children of these
18-
// statements without breaking the clause
19-
} else if (ts.isBreakStatement(s)) {
8+
if (ts.isBreakStatement(s) || ts.isReturnStatement(s)) {
209
return true;
21-
} else if (containsBreakStatement(s.getChildren())) {
10+
} else if (!ts.isBlock(s)) {
11+
// Can only ensure a break scoped in a block is deterministic
12+
continue;
13+
} else if (containsBreakOrReturn(s.getChildren())) {
2214
return true;
2315
}
2416
}
@@ -33,51 +25,77 @@ export const transformSwitchStatement: FunctionVisitor<ts.SwitchStatement> = (st
3325
const switchName = `____switch${scope.id}`;
3426
const switchVariable = lua.createIdentifier(switchName);
3527

36-
// Collect the fallthrough bodies for each case as defined by the switch.
37-
const caseBody: lua.Statement[][] = [];
38-
for (let i = 0; i < statement.caseBlock.clauses.length; i++) {
39-
const end = statement.caseBlock.clauses
40-
.slice(i)
41-
.findIndex(clause => containsBreakStatement([...clause.statements]));
42-
caseBody[i] = statement.caseBlock.clauses
43-
.slice(i, end >= 0 ? end + i + 1 : undefined)
44-
.reduce<lua.Statement[]>(
45-
(statements, clause) => [...statements, ...context.transformStatements(clause.statements)],
46-
[]
47-
);
48-
}
28+
// Collect the case clause expressions and accounting for deterministic fallthrough
29+
let allExpressions: lua.BinaryExpression;
30+
statement.caseBlock.clauses.forEach(clause => {
31+
if (!ts.isDefaultClause(clause)) {
32+
allExpressions = allExpressions
33+
? lua.createBinaryExpression(
34+
allExpressions,
35+
lua.createBinaryExpression(
36+
switchVariable,
37+
context.transformExpression(clause.expression),
38+
lua.SyntaxKind.EqualityOperator
39+
),
40+
lua.SyntaxKind.OrOperator
41+
)
42+
: lua.createBinaryExpression(
43+
switchVariable,
44+
context.transformExpression(clause.expression),
45+
lua.SyntaxKind.EqualityOperator
46+
);
47+
}
48+
});
4949

5050
let statements: lua.Statement[] = [];
5151

5252
// Default will either be the only statement, or the else in the if chain
5353
const defaultIndex = statement.caseBlock.clauses.findIndex(c => ts.isDefaultClause(c));
54-
const defaultBody = defaultIndex >= 0 ? caseBody[defaultIndex] : undefined;
54+
const defaultBody = defaultIndex >= 0 ? statement.caseBlock.clauses[defaultIndex].statements : undefined;
5555
if (defaultBody && statement.caseBlock.clauses.length === 1) {
56-
statements.push(lua.createDoStatement(defaultBody));
56+
statements.push(lua.createDoStatement(context.transformStatements(defaultBody)));
5757
} else {
58-
let concatenatedIf: lua.IfStatement | undefined = undefined;
59-
let previousCondition: lua.IfStatement | lua.Block | undefined = defaultBody
60-
? lua.createBlock(defaultBody)
61-
: undefined;
62-
63-
// Starting from the back, concatenating ifs into one big if/elseif/[else] statement
64-
for (let i = statement.caseBlock.clauses.length - 1; i >= 0; i--) {
65-
const clause = statement.caseBlock.clauses[i];
58+
let previousClause: ts.CaseOrDefaultClause;
59+
let condition: lua.Expression;
60+
statement.caseBlock.clauses.forEach(clause => {
61+
if (!condition || (previousClause && containsBreakOrReturn([...previousClause.statements]))) {
62+
if (ts.isDefaultClause(clause)) {
63+
condition = lua.createUnaryExpression(allExpressions, lua.SyntaxKind.NotOperator);
64+
} else {
65+
condition = lua.createBinaryExpression(
66+
switchVariable,
67+
context.transformExpression(clause.expression),
68+
lua.SyntaxKind.EqualityOperator
69+
);
70+
}
71+
} else {
72+
if (ts.isDefaultClause(clause)) {
73+
condition = lua.createBinaryExpression(
74+
condition,
75+
lua.createUnaryExpression(allExpressions, lua.SyntaxKind.NotOperator),
76+
lua.SyntaxKind.OrOperator
77+
);
78+
} else {
79+
condition = lua.createBinaryExpression(
80+
condition,
81+
lua.createBinaryExpression(
82+
switchVariable,
83+
context.transformExpression(clause.expression),
84+
lua.SyntaxKind.EqualityOperator
85+
),
86+
lua.SyntaxKind.OrOperator
87+
);
88+
}
89+
}
6690

67-
// Skip default clause to keep index aligned, handle in else block
68-
if (ts.isDefaultClause(clause)) continue;
91+
if (condition && clause.statements.length) {
92+
statements.push(
93+
lua.createIfStatement(condition, lua.createBlock(context.transformStatements(clause.statements)))
94+
);
95+
}
6996

70-
// If the clause condition holds, go to the correct label
71-
const condition = lua.createBinaryExpression(
72-
switchVariable,
73-
context.transformExpression(clause.expression),
74-
lua.SyntaxKind.EqualityOperator
75-
);
76-
77-
concatenatedIf = lua.createIfStatement(condition, lua.createBlock(caseBody[i]), previousCondition);
78-
previousCondition = concatenatedIf;
79-
}
80-
if (concatenatedIf) statements.push(concatenatedIf);
97+
previousClause = clause;
98+
});
8199
}
82100

83101
statements = performHoisting(context, statements);

‎test/unit/__snapshots__/switch.spec.ts.snap‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,31 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`switch collapses empty case and minimizes conditions 1`] = `
4+
"require(\\"lualib_bundle\\");
5+
local ____exports = {}
6+
function ____exports.__main(self)
7+
local out = {}
8+
repeat
9+
local ____switch3 = 5
10+
if ((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2) then
11+
__TS__ArrayPush(out, \\"0,1,2\\")
12+
end
13+
if (((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2)) or (____switch3 == 3) then
14+
__TS__ArrayPush(out, \\"3\\")
15+
break
16+
end
17+
if not (((((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2)) or (____switch3 == 3)) or (____switch3 == 4)) then
18+
__TS__ArrayPush(out, \\"default\\")
19+
end
20+
if (not (((((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2)) or (____switch3 == 3)) or (____switch3 == 4))) or (____switch3 == 4) then
21+
__TS__ArrayPush(out, \\"4\\")
22+
end
23+
until true
24+
return out
25+
end
26+
return ____exports"
27+
`;
28+
329
exports[`switch uses elseif 1`] = `
430
"local ____exports = {}
531
function ____exports.__main(self)
@@ -11,12 +37,14 @@ function ____exports.__main(self)
1137
result = 200
1238
break
1339
end
14-
elseif ____switch3 == 1 then
40+
end
41+
if (____switch3 == 0) or (____switch3 == 1) then
1542
do
1643
result = 100
1744
break
1845
end
19-
elseif ____switch3 == 2 then
46+
end
47+
if ((____switch3 == 0) or (____switch3 == 1)) or (____switch3 == 2) then
2048
do
2149
result = 1
2250
break

‎test/unit/switch.spec.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,3 +377,25 @@ test("switch does not pollute parent scope", () => {
377377
return y;
378378
`.expectToMatchJsResult();
379379
});
380+
381+
test("switch collapses empty case and minimizes conditions", () => {
382+
util.testFunction`
383+
const out = [];
384+
switch (5 as number) {
385+
case 0:
386+
case 1:
387+
case 2:
388+
out.push("0,1,2");
389+
case 3:
390+
out.push("3");
391+
break;
392+
default:
393+
out.push("default");
394+
case 4:
395+
out.push("4");
396+
}
397+
return out;
398+
`
399+
.expectLuaToMatchSnapshot()
400+
.expectToMatchJsResult();
401+
});

0 commit comments

Comments
 (0)