Skip to content

Commit 66bb36e

Browse files
GlassBricksGlassBricks
authored
Reduce use of temp variables (#1291)
* Reduce use of temp variables * Remove obsolete check * Add test cases for destructuring with no temps
1 parent 3b75b04 commit 66bb36e

File tree

6 files changed

+84
-52
lines changed

6 files changed

+84
-52
lines changed

‎src/transformation/visitors/binary-expression/assignments.ts‎

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,6 @@ function transformDestructuredAssignmentExpression(
107107
context: TransformationContext,
108108
expression: ts.DestructuringAssignment
109109
) {
110-
const rootIdentifier = context.createTempNameForNode(expression.right);
111-
112110
let [rightPrecedingStatements, right] = transformInPrecedingStatementScope(context, () =>
113111
context.transformExpression(expression.right)
114112
);
@@ -117,12 +115,15 @@ function transformDestructuredAssignmentExpression(
117115
right = wrapInTable(right);
118116
}
119117

120-
const statements = [
121-
lua.createVariableDeclarationStatement(rootIdentifier, right),
122-
...transformDestructuringAssignment(context, expression, rootIdentifier, rightPrecedingStatements.length > 0),
123-
];
118+
const rightExpr = moveToPrecedingTemp(context, right, expression.right);
119+
const statements = transformDestructuringAssignment(
120+
context,
121+
expression,
122+
rightExpr,
123+
rightPrecedingStatements.length > 0
124+
);
124125

125-
return { statements, result: rootIdentifier };
126+
return { statements, result: rightExpr };
126127
}
127128

128129
export function transformAssignmentExpression(
@@ -152,7 +153,6 @@ export function transformAssignmentExpression(
152153
}
153154

154155
if (ts.isPropertyAccessExpression(expression.left) || ts.isElementAccessExpression(expression.left)) {
155-
const tempVar = context.createTempNameForNode(expression.right);
156156
const [precedingStatements, right] = transformInPrecedingStatementScope(context, () =>
157157
context.transformExpression(expression.right)
158158
);
@@ -163,12 +163,10 @@ export function transformAssignmentExpression(
163163
precedingStatements.length > 0
164164
);
165165

166-
context.addPrecedingStatements([
167-
...precedingStatements,
168-
lua.createVariableDeclarationStatement(tempVar, right, expression.right),
169-
lua.createAssignmentStatement(left, lua.cloneIdentifier(tempVar), expression.left),
170-
]);
171-
return lua.cloneIdentifier(tempVar);
166+
context.addPrecedingStatements(precedingStatements);
167+
const rightExpr = moveToPrecedingTemp(context, right, expression.right);
168+
context.addPrecedingStatements(lua.createAssignmentStatement(left, rightExpr, expression.left));
169+
return rightExpr;
172170
} else {
173171
// Simple assignment
174172
// ${left} = ${right}; return ${left}
@@ -232,24 +230,8 @@ export function transformAssignmentStatement(
232230
return [lua.createAssignmentStatement(left, right, expression)];
233231
}
234232

235-
let [rightPrecedingStatements, right] = transformInPrecedingStatementScope(context, () =>
236-
context.transformExpression(expression.right)
237-
);
238-
context.addPrecedingStatements(rightPrecedingStatements);
239-
if (isMultiReturnCall(context, expression.right)) {
240-
right = wrapInTable(right);
241-
}
242-
243-
const rootIdentifier = context.createTempNameForNode(expression.left);
244-
return [
245-
lua.createVariableDeclarationStatement(rootIdentifier, right),
246-
...transformDestructuringAssignment(
247-
context,
248-
expression,
249-
rootIdentifier,
250-
rightPrecedingStatements.length > 0
251-
),
252-
];
233+
const { statements } = transformDestructuredAssignmentExpression(context, expression);
234+
return statements;
253235
} else {
254236
const [precedingStatements, right] = transformInPrecedingStatementScope(context, () =>
255237
context.transformExpression(expression.right)

‎src/transformation/visitors/expression-list.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ export function shouldMoveToTemp(context: TransformationContext, expression: lua
1111
return (
1212
!lua.isLiteral(expression) &&
1313
!(lua.isIdentifier(expression) && expression.symbolId === tempSymbolId) && // Treat generated temps as consts
14-
!(tsOriginal && (isConstIdentifier(context, tsOriginal) || isOptionalContinuation(tsOriginal)))
14+
!(
15+
tsOriginal &&
16+
(isConstIdentifier(context, tsOriginal) ||
17+
isOptionalContinuation(tsOriginal) ||
18+
tsOriginal.kind === ts.SyntaxKind.ThisKeyword)
19+
)
1520
);
1621
}
1722

‎src/transformation/visitors/variable-declaration.ts‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { createCallableTable, isFunctionTypeWithProperties } from "./function";
1212
import { transformIdentifier } from "./identifier";
1313
import { isMultiReturnCall } from "./language-extensions/multi";
1414
import { transformPropertyName } from "./literal";
15+
import { moveToPrecedingTemp } from "./expression-list";
1516

1617
export function transformArrayBindingElement(
1718
context: TransformationContext,
@@ -38,7 +39,7 @@ export function transformArrayBindingElement(
3839
export function transformBindingPattern(
3940
context: TransformationContext,
4041
pattern: ts.BindingPattern,
41-
table: lua.Identifier,
42+
table: lua.Expression,
4243
propertyAccessStack: ts.PropertyName[] = []
4344
): lua.Statement[] {
4445
const result: lua.Statement[] = [];
@@ -165,21 +166,20 @@ export function transformBindingVariableDeclaration(
165166
ts.isBindingElement(e) && (!ts.isIdentifier(e.name) || e.dotDotDotToken);
166167

167168
if (ts.isObjectBindingPattern(bindingPattern) || bindingPattern.elements.some(isComplexBindingElement)) {
168-
let table: lua.Identifier;
169-
if (initializer !== undefined && ts.isIdentifier(initializer)) {
170-
table = transformIdentifier(context, initializer);
171-
} else {
169+
let table: lua.Expression;
170+
if (initializer) {
172171
// Contain the expression in a temporary variable
173-
if (initializer) {
174-
table = context.createTempNameForNode(initializer);
175-
let expression = context.transformExpression(initializer);
176-
if (isMultiReturnCall(context, initializer)) {
177-
expression = wrapInTable(expression);
178-
}
179-
statements.push(lua.createVariableDeclarationStatement(table, expression));
180-
} else {
181-
table = lua.createAnonymousIdentifier();
172+
let expression = context.transformExpression(initializer);
173+
if (isMultiReturnCall(context, initializer)) {
174+
expression = wrapInTable(expression);
182175
}
176+
const [moveStatements, movedExpr] = transformInPrecedingStatementScope(context, () =>
177+
moveToPrecedingTemp(context, expression, initializer)
178+
);
179+
statements.push(...moveStatements);
180+
table = movedExpr;
181+
} else {
182+
table = lua.createAnonymousIdentifier();
183183
}
184184
statements.push(...transformBindingPattern(context, bindingPattern, table));
185185
return statements;

‎test/translation/__snapshots__/transformation.spec.ts.snap‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,8 @@ value1 = obj.value1
269269
value2 = obj.value2
270270
obj2 = {value3 = 1, value4 = 2}
271271
value3 = obj2.value3
272-
value4 = obj2.value4
272+
local ____obj2_0 = obj2
273+
value4 = ____obj2_0.value4
273274
function fun1(self)
274275
end
275276
fun2 = function()

‎test/unit/destructuring.spec.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,28 @@ test.each(testCases)("in variable declaration (%p)", ({ binding, value }) => {
7474
`.expectToMatchJsResult();
7575
});
7676

77+
test.each(testCases)("in variable declaration from const variable (%p)", ({ binding, value }) => {
78+
util.testFunction`
79+
let ${allBindings};
80+
{
81+
const v: any = ${value};
82+
const ${binding} = v;
83+
return { ${allBindings} };
84+
}
85+
`.expectToMatchJsResult();
86+
});
87+
88+
test.each(testCases)("in variable declaration from this (%p)", ({ binding, value }) => {
89+
util.testFunction`
90+
let ${allBindings};
91+
function test(this: any) {
92+
const ${binding} = this;
93+
return { ${allBindings} };
94+
}
95+
return test.call(${value});
96+
`.expectToMatchJsResult();
97+
});
98+
7799
test.each(testCases)("in exported variable declaration (%p)", ({ binding, value }) => {
78100
util.testModule`
79101
export const ${binding} = ${value};
@@ -101,6 +123,28 @@ test.each(assignmentTestCases)("in assignment expression (%p)", ({ binding, valu
101123
`.expectToMatchJsResult();
102124
});
103125

126+
test.each(assignmentTestCases)("in assignment expression from const variable (%p)", ({ binding, value }) => {
127+
util.testFunction`
128+
let ${allBindings};
129+
const obj = { prop: false };
130+
const v: any = ${value};
131+
const expressionResult = (${binding} = v);
132+
return { ${allBindings}, expressionResult };
133+
`.expectToMatchJsResult();
134+
});
135+
136+
test.each(assignmentTestCases)("in assignment expression from this (%p)", ({ binding, value }) => {
137+
util.testFunction`
138+
let ${allBindings};
139+
const obj = { prop: false };
140+
function test(this: any) {
141+
const expressionResult = (${binding} = this);
142+
return { ${allBindings}, obj, expressionResult };
143+
}
144+
return test.call(${value});
145+
`.expectToMatchJsResult();
146+
});
147+
104148
test.each(["[]", "{}"])("empty binding pattern", bindingPattern => {
105149
util.testFunction`
106150
let i = 1;

‎test/unit/language-extensions/__snapshots__/multi.spec.ts.snap‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ end"
6161

6262
exports[`invalid $multi call (const [a = 0] = $multi()): diagnostics 1`] = `"main.ts(2,25): error TSTL: The $multi function must be called in a return statement."`;
6363

64-
exports[`invalid $multi call (const {} = $multi();): code 1`] = `"local _____24multi_result_0 = {{____(_G)}}"`;
64+
exports[`invalid $multi call (const {} = $multi();): code 1`] = `"local ____temp_0 = {{____(_G)}}"`;
6565

6666
exports[`invalid $multi call (const {} = $multi();): diagnostics 1`] = `"main.ts(2,20): error TSTL: The $multi function must be called in a return statement."`;
6767

@@ -219,10 +219,10 @@ local function multi(self, ...)
219219
return ...
220220
end
221221
local a
222-
local _____24multi_result_0 = {____(nil, 1)}
223-
a = _____24multi_result_0[1]
222+
local ____temp_0 = {____(nil, 1)}
223+
a = ____temp_0[1]
224224
____exports.a = a
225-
if _____24multi_result_0 then
225+
if ____temp_0 then
226226
a = a + 1
227227
____exports.a = a
228228
end

0 commit comments

Comments
 (0)