Skip to content

Commit 3237c20

Browse files
tomblindPerryvw
tomblind
authored and
Perryvw
committed
fixes various issues with translating assignment operators... (#241)
* fixes for issues with assignments with side-effects, especially expressions * refactoring based on feedback * checking all array values in deep assignment tests
1 parent e279922 commit 3237c20

File tree

7 files changed

+714
-51
lines changed

7 files changed

+714
-51
lines changed

‎src/TSHelper.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,4 +208,32 @@ export class TSHelper {
208208
}
209209
return true;
210210
}
211+
212+
// Returns true for expressions that may have effects when evaluated
213+
public static isExpressionWithEvaluationEffect(node: ts.Expression): boolean {
214+
return !(ts.isLiteralExpression(node) || ts.isIdentifier(node));
215+
}
216+
217+
// If expression is property/element access with possible effects from being evaluated, returns true along with the
218+
// separated object and index expressions.
219+
public static isAccessExpressionWithEvaluationEffects(node: ts.Expression, checker: ts.TypeChecker):
220+
[boolean, ts.Expression, ts.Expression] {
221+
if (ts.isElementAccessExpression(node)
222+
&& (this.isExpressionWithEvaluationEffect(node.expression)
223+
|| this.isExpressionWithEvaluationEffect(node.argumentExpression))) {
224+
const type = checker.getTypeAtLocation(node.expression);
225+
if (this.isArrayType(type, checker)) {
226+
// Offset arrays by one
227+
const oneLit = ts.createNumericLiteral("1");
228+
const exp = ts.createParen(node.argumentExpression);
229+
const addExp = ts.createBinary(exp, ts.SyntaxKind.PlusToken, oneLit);
230+
return [true, node.expression, addExp];
231+
} else {
232+
return [true, node.expression, node.argumentExpression];
233+
}
234+
} else if (ts.isPropertyAccessExpression(node) && this.isExpressionWithEvaluationEffect(node.expression)) {
235+
return [true, node.expression, ts.createStringLiteral(node.name.text)];
236+
}
237+
return [false, null, null];
238+
}
211239
}

‎src/Transpiler.ts‎

Lines changed: 114 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -827,11 +827,10 @@ export abstract class LuaTranspiler {
827827
// Check if this is an assignment token, then handle accordingly
828828
const [isAssignment, operator] = tsHelper.isBinaryAssignmentToken(node.operatorToken.kind);
829829
if (isAssignment) {
830-
const binaryExpression = ts.createBinary(node.left, operator, node.right);
831-
binaryExpression.pos = node.operatorToken.pos;
832830
return this.transpileAssignmentExpression(
833831
node.left,
834-
binaryExpression,
832+
operator,
833+
node.right,
835834
tsHelper.isExpressionStatement(node),
836835
false
837836
);
@@ -841,10 +840,6 @@ export abstract class LuaTranspiler {
841840
const lhs = this.transpileExpression(node.left, true);
842841
const rhs = this.transpileExpression(node.right, true);
843842

844-
if (node.operatorToken.kind === ts.SyntaxKind.EqualsToken && !tsHelper.isExpressionStatement(node)) {
845-
return `(function() ${lhs} = ${rhs}; return ${lhs} end)()`;
846-
}
847-
848843
let result = "";
849844

850845
// Transpile Bitops
@@ -905,21 +900,7 @@ export abstract class LuaTranspiler {
905900
result = `${lhs}<=${rhs}`;
906901
break;
907902
case ts.SyntaxKind.EqualsToken:
908-
if (tsHelper.hasSetAccessor(node.left, this.checker)) {
909-
return this.transpileSetAccessor(node.left as ts.PropertyAccessExpression, rhs);
910-
}
911-
912-
if (ts.isArrayLiteralExpression(node.left)) {
913-
// Destructing assignment
914-
const vars = node.left.elements.map(e => this.transpileExpression(e)).join(",");
915-
if (tsHelper.isTupleReturnCall(node.right, this.checker)) {
916-
return `${vars} = ${rhs}`;
917-
} else {
918-
return `${vars} = ${this.transpileDestructingAssignmentValue(node.right)}`;
919-
}
920-
}
921-
922-
result = `${lhs} = ${rhs}`;
903+
result = this.transpileAssignment(node, lhs, rhs);
923904
break;
924905
case ts.SyntaxKind.EqualsEqualsToken:
925906
case ts.SyntaxKind.EqualsEqualsEqualsToken:
@@ -948,6 +929,44 @@ export abstract class LuaTranspiler {
948929
}
949930
}
950931

932+
public transpileAssignment(node: ts.BinaryExpression, lhs: string, rhs: string): string {
933+
if (tsHelper.hasSetAccessor(node.left, this.checker)) {
934+
return this.transpileSetAccessor(node.left as ts.PropertyAccessExpression, rhs);
935+
}
936+
937+
if (ts.isArrayLiteralExpression(node.left)) {
938+
// Destructing assignment
939+
const vars = node.left.elements.map(e => this.transpileExpression(e)).join(",");
940+
const vals = tsHelper.isTupleReturnCall(node.right, this.checker)
941+
? rhs : this.transpileDestructingAssignmentValue(node.right);
942+
if (tsHelper.isExpressionStatement(node)) {
943+
// In JS, the right side of a destructuring assignment is evaluated before the left
944+
const tmps = node.left.elements.map((_, i) => `__TS_tmp${i}`).join(",");
945+
return `do local ${tmps} = ${vals}; ${vars} = ${tmps} end`;
946+
} else {
947+
return `(function(...) ${vars} = ...; return {...} end)(${vals})`;
948+
}
949+
}
950+
951+
if (tsHelper.isExpressionStatement(node)) {
952+
return `${lhs} = ${rhs}`;
953+
}
954+
955+
const [hasEffects, objExp, indexExp] = tsHelper.isAccessExpressionWithEvaluationEffects(
956+
node.left, this.checker);
957+
if (hasEffects) {
958+
// Property/element access expressions need to have individual parts cached
959+
const obj = this.transpileExpression(objExp);
960+
const index = this.transpileExpression(indexExp);
961+
return `(function(o, i, v) o[i] = v; return v end)(${obj}, ${index}, ${rhs})`;
962+
} else if (tsHelper.isExpressionWithEvaluationEffect(node.right)) {
963+
// Cache right-hand express in temp
964+
return `(function() local __TS_tmp = ${rhs}; ${lhs} = __TS_tmp; return __TS_tmp end)()`;
965+
} else {
966+
return `(function() ${lhs} = ${rhs}; return ${rhs} end)()`;
967+
}
968+
}
969+
951970
public transpileUnaryBitOperation(node: ts.PrefixUnaryExpression, operand: string): string {
952971
throw TSTLErrors.UnsupportedForTarget("Bitwise operations", this.options.luaTarget, node);
953972
}
@@ -980,31 +999,78 @@ export abstract class LuaTranspiler {
980999
`function() return ${val1} end`, `function() return ${val2} end`);
9811000
}
9821001

1002+
public transpileBinaryAssignmentExpression(
1003+
assignee: ts.Expression,
1004+
lhs: ts.Expression,
1005+
operator: ts.BinaryOperator,
1006+
rhs: ts.Expression,
1007+
pos: number): string {
1008+
const opExp = ts.createBinary(lhs, operator, rhs);
1009+
opExp.pos = pos;
1010+
const assignExp = ts.createAssignment(assignee, opExp);
1011+
return this.transpileBinaryExpression(assignExp);
1012+
}
1013+
9831014
public transpileAssignmentExpression(
9841015
lhs: ts.Expression,
985-
replacementExpression: ts.BinaryExpression,
1016+
operator: ts.BinaryOperator,
1017+
rhs: ts.Expression,
9861018
isStatement: boolean,
9871019
returnValueBefore: boolean): string {
988-
// Assign replacement expression to its left-hand side
989-
const assignment = ts.createAssignment(lhs, replacementExpression);
990-
991-
if (isStatement) {
992-
return this.transpileExpression(assignment);
993-
} else {
1020+
let statement: string; // Assignment statement
1021+
let result: string; // Result if expression
1022+
let wrap = true; // Wrap statement in do...end
1023+
const pos = (lhs.parent ? lhs.parent : lhs).pos;
1024+
const [hasEffects, objExp, indexExp] = tsHelper.isAccessExpressionWithEvaluationEffects(lhs, this.checker);
1025+
if (hasEffects) {
1026+
// Complex property/element accesses need to cache object/index expressions to avoid repeating side-effects
1027+
const objIdent = ts.createIdentifier("__TS_obj");
1028+
const indexIdent = ts.createIdentifier("__TS_index");
1029+
const tempIdent = ts.createIdentifier("__TS_tmp");
1030+
const accessExp = ts.createElementAccess(objIdent, indexIdent);
1031+
rhs = ts.createParen(rhs);
1032+
const obj = this.transpileExpression(objExp);
1033+
const index = this.transpileExpression(indexExp);
1034+
let assignTemp: string;
1035+
let assign: string;
9941036
if (returnValueBefore) {
995-
const oldValueName = `__originalValue${this.transpileExpression(lhs)}${this.genVarCounter++}`;
996-
const oldValueAssignment = ts.createVariableStatement(
997-
[], [ts.createVariableDeclaration(oldValueName, undefined, lhs)]);
998-
999-
const valueReturn = ts.createReturn(ts.createIdentifier(oldValueName));
1000-
1001-
return this.transpileExpression(ts.createImmediatelyInvokedArrowFunction(
1002-
[oldValueAssignment, ts.createStatement(assignment), valueReturn]));
1037+
// Postfix
1038+
assignTemp = this.transpileBinaryExpression(ts.createAssignment(tempIdent, accessExp));
1039+
assign = this.transpileBinaryAssignmentExpression(accessExp, tempIdent, operator, rhs, pos);
10031040
} else {
1004-
const valueReturn = ts.createReturn(lhs);
1005-
return this.transpileExpression(
1006-
ts.createImmediatelyInvokedArrowFunction([ts.createStatement(assignment), valueReturn]));
1041+
assignTemp = this.transpileBinaryAssignmentExpression(tempIdent, accessExp, operator, rhs, pos);
1042+
assign = this.transpileBinaryExpression(ts.createAssignment(accessExp, tempIdent));
10071043
}
1044+
statement = `local __TS_obj, __TS_index = ${obj}, ${index}; local ${assignTemp}; ${assign}`;
1045+
result = "__TS_tmp";
1046+
1047+
} else if (!isStatement && returnValueBefore) {
1048+
// Postfix expressions need to cache original value in temp
1049+
const tempIdent = ts.createIdentifier("__TS_tmp");
1050+
const assignTemp = this.transpileBinaryExpression(ts.createAssignment(tempIdent, lhs));
1051+
const assign = this.transpileBinaryAssignmentExpression(lhs, tempIdent, operator, rhs, pos);
1052+
statement = `local ${assignTemp}; ${assign}`;
1053+
result = "__TS_tmp";
1054+
1055+
} else if (!isStatement && (ts.isPropertyAccessExpression(lhs) || ts.isElementAccessExpression(lhs))) {
1056+
// Simple property/element access expressions need to cache in temp to avoid double-evaluation
1057+
const tempIdent = ts.createIdentifier("__TS_tmp");
1058+
const assignTemp = this.transpileBinaryAssignmentExpression(tempIdent, lhs, operator, rhs, pos);
1059+
const assign = this.transpileBinaryExpression(ts.createAssignment(lhs, tempIdent));
1060+
statement = `local ${assignTemp}; ${assign}`;
1061+
result = "__TS_tmp";
1062+
1063+
} else {
1064+
// Simple statements/expressions
1065+
statement = this.transpileBinaryAssignmentExpression(lhs, lhs, operator, rhs, pos);
1066+
result = this.transpileExpression(lhs);
1067+
wrap = false;
1068+
}
1069+
1070+
if (isStatement) {
1071+
return wrap ? `do ${statement}; end` : statement;
1072+
} else {
1073+
return `(function() ${statement}; return ${result} end)()`;
10081074
}
10091075
}
10101076

@@ -1014,15 +1080,17 @@ export abstract class LuaTranspiler {
10141080
case ts.SyntaxKind.PlusPlusToken: {
10151081
return this.transpileAssignmentExpression(
10161082
node.operand,
1017-
ts.createBinary(node.operand, ts.SyntaxKind.PlusToken, ts.createLiteral(1)),
1083+
ts.SyntaxKind.PlusToken,
1084+
ts.createLiteral(1),
10181085
tsHelper.isExpressionStatement(node),
10191086
true
10201087
);
10211088
}
10221089
case ts.SyntaxKind.MinusMinusToken: {
10231090
return this.transpileAssignmentExpression(
10241091
node.operand,
1025-
ts.createBinary(node.operand, ts.SyntaxKind.MinusToken, ts.createLiteral(1)),
1092+
ts.SyntaxKind.MinusToken,
1093+
ts.createLiteral(1),
10261094
tsHelper.isExpressionStatement(node),
10271095
true
10281096
);
@@ -1040,15 +1108,17 @@ export abstract class LuaTranspiler {
10401108
case ts.SyntaxKind.PlusPlusToken: {
10411109
return this.transpileAssignmentExpression(
10421110
node.operand,
1043-
ts.createBinary(node.operand, ts.SyntaxKind.PlusToken, ts.createLiteral(1)),
1111+
ts.SyntaxKind.PlusToken,
1112+
ts.createLiteral(1),
10441113
tsHelper.isExpressionStatement(node),
10451114
false
10461115
);
10471116
}
10481117
case ts.SyntaxKind.MinusMinusToken: {
10491118
return this.transpileAssignmentExpression(
10501119
node.operand,
1051-
ts.createBinary(node.operand, ts.SyntaxKind.MinusToken, ts.createLiteral(1)),
1120+
ts.SyntaxKind.MinusToken,
1121+
ts.createLiteral(1),
10521122
tsHelper.isExpressionStatement(node),
10531123
false
10541124
);

0 commit comments

Comments
 (0)