X Tutup
Skip to content

Commit 96c3f8d

Browse files
ark120202Perryvw
ark120202
authored and
Perryvw
committed
Share scope between all switch cases (#775)
* Share scope between all switch cases * Readd `do` blocks around cases and enable dead code elemination * Use `pushScope` return value (from #783) * Special-case switch scope hoisting * Add test from PR feedback
1 parent ed86bbd commit 96c3f8d

File tree

5 files changed

+88
-55
lines changed

5 files changed

+88
-55
lines changed

‎src/LuaPrinter.ts‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -273,16 +273,24 @@ export class LuaPrinter {
273273

274274
protected printStatementArray(statements: lua.Statement[]): SourceChunk[] {
275275
const statementNodes: SourceNode[] = [];
276-
statements = this.removeDeadAndEmptyStatements(statements);
277-
statements.forEach((s, i) => {
278-
const node = this.printStatement(s);
276+
for (const [index, statement] of statements.entries()) {
277+
if (this.isStatementEmpty(statement)) continue;
279278

280-
if (i > 0 && this.statementMayRequireSemiColon(statements[i - 1]) && this.nodeStartsWithParenthesis(node)) {
281-
statementNodes[i - 1].add(";");
279+
const node = this.printStatement(statement);
280+
281+
if (
282+
index > 0 &&
283+
this.statementMayRequireSemiColon(statements[index - 1]) &&
284+
this.nodeStartsWithParenthesis(node)
285+
) {
286+
statementNodes[index - 1].add(";");
282287
}
283288

284289
statementNodes.push(node);
285-
});
290+
291+
if (lua.isReturnStatement(statement)) break;
292+
}
293+
286294
return statementNodes.length > 0 ? [...this.joinChunks("\n", statementNodes), "\n"] : [];
287295
}
288296

@@ -778,19 +786,6 @@ export class LuaPrinter {
778786
return new SourceNode(null, null, this.sourceFile, LuaPrinter.operatorMap[kind]);
779787
}
780788

781-
protected removeDeadAndEmptyStatements(statements: lua.Statement[]): lua.Statement[] {
782-
const aliveStatements = [];
783-
for (const statement of statements) {
784-
if (!this.isStatementEmpty(statement)) {
785-
aliveStatements.push(statement);
786-
}
787-
if (lua.isReturnStatement(statement)) {
788-
break;
789-
}
790-
}
791-
return aliveStatements;
792-
}
793-
794789
protected isStatementEmpty(statement: lua.Statement): boolean {
795790
return lua.isDoStatement(statement) && (!statement.statements || statement.statements.length === 0);
796791
}

‎src/transformation/utils/lua-ast.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as ts from "typescript";
2+
import * as assert from "assert";
23
import { LuaTarget } from "../../CompilerOptions";
34
import * as lua from "../../LuaAST";
45
import { TransformationContext } from "../context";
@@ -99,6 +100,8 @@ export function createHoistableVariableDeclarationStatement(
99100
const declaration = lua.createVariableDeclarationStatement(identifier, initializer, tsOriginal);
100101
if (!context.options.noHoisting && identifier.symbolId) {
101102
const scope = peekScope(context);
103+
assert(scope.type !== ScopeType.Switch);
104+
102105
if (!scope.variableDeclarations) {
103106
scope.variableDeclarations = [];
104107
}
@@ -143,14 +146,16 @@ export function createLocalOrExportedOrGlobalDeclaration(
143146
const insideFunction = findScope(context, ScopeType.Function) !== undefined;
144147

145148
if (context.isModule || getCurrentNamespace(context) || insideFunction || isVariableDeclaration) {
146-
// local
149+
const scope = peekScope(context);
150+
147151
const isPossibleWrappedFunction =
148152
!isFunctionDeclaration &&
149153
tsOriginal &&
150154
ts.isVariableDeclaration(tsOriginal) &&
151155
tsOriginal.initializer &&
152156
isFunctionType(context, context.checker.getTypeAtLocation(tsOriginal.initializer));
153-
if (isPossibleWrappedFunction) {
157+
158+
if (isPossibleWrappedFunction || scope.type === ScopeType.Switch) {
154159
// Split declaration and assignment for wrapped function types to allow recursion
155160
declaration = lua.createVariableDeclarationStatement(lhs, undefined, tsOriginal);
156161
assignment = lua.createAssignmentStatement(lhs, rhs, tsOriginal);
@@ -160,13 +165,15 @@ export function createLocalOrExportedOrGlobalDeclaration(
160165

161166
if (!context.options.noHoisting) {
162167
// Remember local variable declarations for hoisting later
163-
const scope = peekScope(context);
164-
165168
if (!scope.variableDeclarations) {
166169
scope.variableDeclarations = [];
167170
}
168171

169172
scope.variableDeclarations.push(declaration);
173+
174+
if (scope.type === ScopeType.Switch) {
175+
declaration = undefined;
176+
}
170177
}
171178
} else if (rhs) {
172179
// global

‎src/transformation/utils/scope.ts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,14 @@ export function findScope(context: TransformationContext, scopeTypes: ScopeType)
7777
}
7878

7979
const scopeIdCounters = new WeakMap<TransformationContext, number>();
80-
export function pushScope(context: TransformationContext, scopeType: ScopeType): void {
80+
export function pushScope(context: TransformationContext, scopeType: ScopeType): Scope {
8181
const nextScopeId = (scopeIdCounters.get(context) ?? 0) + 1;
8282
scopeIdCounters.set(context, nextScopeId);
8383

8484
const scopeStack = getScopeStack(context);
85-
scopeStack.push({ type: scopeType, id: nextScopeId });
85+
const scope: Scope = { type: scopeType, id: nextScopeId };
86+
scopeStack.push(scope);
87+
return scope;
8688
}
8789

8890
export function popScope(context: TransformationContext): Scope {
@@ -161,20 +163,20 @@ function hoistVariableDeclarations(
161163
for (const declaration of scope.variableDeclarations) {
162164
const symbols = declaration.left.map(i => i.symbolId).filter(isNonNull);
163165
if (symbols.some(s => shouldHoistSymbol(context, s, scope))) {
164-
let assignment: lua.AssignmentStatement | undefined;
165-
if (declaration.right) {
166-
assignment = lua.createAssignmentStatement(declaration.left, declaration.right);
167-
lua.setNodePosition(assignment, declaration); // Preserve position info for sourcemap
168-
}
169-
170166
const index = result.indexOf(declaration);
171167
assert(index > -1);
172-
if (assignment) {
168+
169+
if (declaration.right) {
170+
const assignment = lua.createAssignmentStatement(declaration.left, declaration.right);
171+
lua.setNodePosition(assignment, declaration); // Preserve position info for sourcemap
173172
result.splice(index, 1, assignment);
174173
} else {
175174
result.splice(index, 1);
176175
}
177176

177+
hoistedLocals.push(...declaration.left);
178+
} else if (scope.type === ScopeType.Switch) {
179+
assert(!declaration.right);
178180
hoistedLocals.push(...declaration.left);
179181
}
180182
}

‎src/transformation/visitors/switch.ts‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,19 @@ import { LuaTarget } from "../../CompilerOptions";
33
import * as lua from "../../LuaAST";
44
import { FunctionVisitor } from "../context";
55
import { UnsupportedForTarget } from "../utils/errors";
6-
import { peekScope, performHoisting, popScope, pushScope, ScopeType } from "../utils/scope";
6+
import { performHoisting, popScope, pushScope, ScopeType } from "../utils/scope";
77

88
export const transformSwitchStatement: FunctionVisitor<ts.SwitchStatement> = (statement, context) => {
99
if (context.luaTarget === LuaTarget.Lua51) {
1010
throw UnsupportedForTarget("Switch statements", LuaTarget.Lua51, statement);
1111
}
1212

13-
pushScope(context, ScopeType.Switch);
14-
13+
const scope = pushScope(context, ScopeType.Switch);
1514
// Give the switch a unique name to prevent nested switches from acting up.
16-
const scope = peekScope(context);
1715
const switchName = `____switch${scope.id}`;
18-
19-
const expression = context.transformExpression(statement.expression);
2016
const switchVariable = lua.createIdentifier(switchName);
21-
const switchVariableDeclaration = lua.createVariableDeclarationStatement(switchVariable, expression);
2217

23-
let statements: lua.Statement[] = [switchVariableDeclaration];
18+
let statements: lua.Statement[] = [];
2419

2520
const caseClauses = statement.caseBlock.clauses.filter(ts.isCaseClause);
2621
for (const [index, clause] of caseClauses.entries()) {
@@ -37,25 +32,21 @@ export const transformSwitchStatement: FunctionVisitor<ts.SwitchStatement> = (st
3732
}
3833

3934
const hasDefaultCase = statement.caseBlock.clauses.some(ts.isDefaultClause);
40-
if (hasDefaultCase) {
41-
statements.push(lua.createGotoStatement(`${switchName}_case_default`));
42-
} else {
43-
statements.push(lua.createGotoStatement(`${switchName}_end`));
44-
}
35+
statements.push(lua.createGotoStatement(`${switchName}_${hasDefaultCase ? "case_default" : "end"}`));
4536

4637
for (const [index, clause] of statement.caseBlock.clauses.entries()) {
47-
const label = ts.isCaseClause(clause)
48-
? lua.createLabelStatement(`${switchName}_case_${index}`)
49-
: lua.createLabelStatement(`${switchName}_case_default`);
50-
51-
const body = lua.createDoStatement(context.transformStatements(clause.statements));
52-
statements.push(label, body);
38+
const labelName = `${switchName}_case_${ts.isCaseClause(clause) ? index : "default"}`;
39+
statements.push(lua.createLabelStatement(labelName));
40+
statements.push(lua.createDoStatement(context.transformStatements(clause.statements)));
5341
}
5442

5543
statements.push(lua.createLabelStatement(`${switchName}_end`));
5644

5745
statements = performHoisting(context, statements);
5846
popScope(context);
5947

48+
const expression = context.transformExpression(statement.expression);
49+
statements.unshift(lua.createVariableDeclarationStatement(switchVariable, expression));
50+
6051
return statements;
6152
};

‎test/unit/conditionals.spec.ts‎

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,46 @@ test.each([0, 1, 2, 3])("nestedSwitch (%p)", inp => {
157157
`.expectToMatchJsResult();
158158
});
159159

160-
test.each([0, 1, 2])("switchLocalScope (%p)", inp => {
160+
test("switch cases scope", () => {
161+
util.testFunction`
162+
switch (0 as number) {
163+
case 0:
164+
let foo: number | undefined = 1;
165+
case 1:
166+
foo = 2;
167+
case 2:
168+
return foo;
169+
}
170+
`.expectToMatchJsResult();
171+
});
172+
173+
test("variable in nested scope does not interfere with case scope", () => {
174+
util.testFunction`
175+
let foo: number = 0;
176+
switch (foo) {
177+
case 0: {
178+
let foo = 1;
179+
}
180+
181+
case 1:
182+
return foo;
183+
}
184+
`.expectToMatchJsResult();
185+
});
186+
187+
test.only("switch using variable re-declared in cases", () => {
188+
util.testFunction`
189+
let foo: number = 0;
190+
switch (foo) {
191+
case 0:
192+
let foo = true;
193+
case 1:
194+
return foo;
195+
}
196+
`.expectToMatchJsResult();
197+
});
198+
199+
test.each([0, 1, 2])("switch with block statement scope (%p)", inp => {
161200
util.testFunction`
162201
let result: number = -1;
163202
@@ -183,8 +222,6 @@ test.each([0, 1, 2])("switchLocalScope (%p)", inp => {
183222

184223
test.each([0, 1, 2, 3])("switchReturn (%p)", inp => {
185224
util.testFunction`
186-
const result: number = -1;
187-
188225
switch (<number>${inp}) {
189226
case 0:
190227
return 0;
@@ -195,7 +232,8 @@ test.each([0, 1, 2, 3])("switchReturn (%p)", inp => {
195232
return 2;
196233
break;
197234
}
198-
return result;
235+
236+
return -1;
199237
`.expectToMatchJsResult();
200238
});
201239

0 commit comments

Comments
 (0)
X Tutup