Skip to content

fixes various issues with translating assignment operators... #241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Perryvw merged 3 commits into from
Oct 18, 2018
Merged

fixes various issues with translating assignment operators... #241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4ef49b
fixes for issues with assignments with side-effects, especially expre…
tomblind Oct 14, 2018
50ff3ca
refactoring based on feedback
tomblind Oct 17, 2018
d991754
checking all array values in deep assignment tests
tomblind Oct 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/TSHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,32 @@ export class TSHelper {
}
return true;
}

// Returns true for expressions that may have effects when evaluated
public static isExpressionWithEvaluationEffect(node: ts.Expression): boolean {
return !(ts.isLiteralExpression(node) || ts.isIdentifier(node));
}

// If expression is property/element access with possible effects from being evaluated, returns true along with the
// separated object and index expressions.
public static isAccessExpressionWithEvaluationEffects(node: ts.Expression, checker: ts.TypeChecker):
[boolean, ts.Expression, ts.Expression] {
if (ts.isElementAccessExpression(node)
&& (this.isExpressionWithEvaluationEffect(node.expression)
|| this.isExpressionWithEvaluationEffect(node.argumentExpression))) {
const type = checker.getTypeAtLocation(node.expression);
if (this.isArrayType(type, checker)) {
// Offset arrays by one
const oneLit = ts.createNumericLiteral("1");
const exp = ts.createParen(node.argumentExpression);
const addExp = ts.createBinary(exp, ts.SyntaxKind.PlusToken, oneLit);
return [true, node.expression, addExp];
} else {
return [true, node.expression, node.argumentExpression];
}
} else if (ts.isPropertyAccessExpression(node) && this.isExpressionWithEvaluationEffect(node.expression)) {
return [true, node.expression, ts.createStringLiteral(node.name.text)];
}
return [false, null, null];
}
}
158 changes: 114 additions & 44 deletions src/Transpiler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,11 +826,10 @@ export abstract class LuaTranspiler {
// Check if this is an assignment token, then handle accordingly
const [isAssignment, operator] = tsHelper.isBinaryAssignmentToken(node.operatorToken.kind);
if (isAssignment) {
const binaryExpression = ts.createBinary(node.left, operator, node.right);
binaryExpression.pos = node.operatorToken.pos;
return this.transpileAssignmentExpression(
node.left,
binaryExpression,
operator,
node.right,
tsHelper.isExpressionStatement(node),
false
);
Expand All @@ -840,10 +839,6 @@ export abstract class LuaTranspiler {
const lhs = this.transpileExpression(node.left, true);
const rhs = this.transpileExpression(node.right, true);

if (node.operatorToken.kind === ts.SyntaxKind.EqualsToken && !tsHelper.isExpressionStatement(node)) {
return `(function() ${lhs} = ${rhs}; return ${lhs} end)()`;
}

let result = "";

// Transpile Bitops
Expand Down Expand Up @@ -904,21 +899,7 @@ export abstract class LuaTranspiler {
result = `${lhs}<=${rhs}`;
break;
case ts.SyntaxKind.EqualsToken:
if (tsHelper.hasSetAccessor(node.left, this.checker)) {
return this.transpileSetAccessor(node.left as ts.PropertyAccessExpression, rhs);
}

if (ts.isArrayLiteralExpression(node.left)) {
// Destructing assignment
const vars = node.left.elements.map(e => this.transpileExpression(e)).join(",");
if (tsHelper.isTupleReturnCall(node.right, this.checker)) {
return `${vars} = ${rhs}`;
} else {
return `${vars} = ${this.transpileDestructingAssignmentValue(node.right)}`;
}
}

result = `${lhs} = ${rhs}`;
result = this.transpileAssignment(node, lhs, rhs);
break;
case ts.SyntaxKind.EqualsEqualsToken:
case ts.SyntaxKind.EqualsEqualsEqualsToken:
Expand Down Expand Up @@ -947,6 +928,44 @@ export abstract class LuaTranspiler {
}
}

public transpileAssignment(node: ts.BinaryExpression, lhs: string, rhs: string): string {
if (tsHelper.hasSetAccessor(node.left, this.checker)) {
return this.transpileSetAccessor(node.left as ts.PropertyAccessExpression, rhs);
}

if (ts.isArrayLiteralExpression(node.left)) {
// Destructing assignment
const vars = node.left.elements.map(e => this.transpileExpression(e)).join(",");
const vals = tsHelper.isTupleReturnCall(node.right, this.checker)
? rhs : this.transpileDestructingAssignmentValue(node.right);
if (tsHelper.isExpressionStatement(node)) {
// In JS, the right side of a destructuring assignment is evaluated before the left
const tmps = node.left.elements.map((_, i) => `__TS_tmp${i}`).join(",");
return `do local ${tmps} = ${vals}; ${vars} = ${tmps} end`;
} else {
return `(function(...) ${vars} = ...; return {...} end)(${vals})`;
}
}

if (tsHelper.isExpressionStatement(node)) {
return `${lhs} = ${rhs}`;
}

const [hasEffects, objExp, indexExp] = tsHelper.isAccessExpressionWithEvaluationEffects(
node.left, this.checker);
if (hasEffects) {
// Property/element access expressions need to have individual parts cached
const obj = this.transpileExpression(objExp);
const index = this.transpileExpression(indexExp);
return `(function(o, i, v) o[i] = v; return v end)(${obj}, ${index}, ${rhs})`;
} else if (tsHelper.isExpressionWithEvaluationEffect(node.right)) {
// Cache right-hand express in temp
return `(function() local __TS_tmp = ${rhs}; ${lhs} = __TS_tmp; return __TS_tmp end)()`;
} else {
return `(function() ${lhs} = ${rhs}; return ${rhs} end)()`;
}
}

public transpileUnaryBitOperation(node: ts.PrefixUnaryExpression, operand: string): string {
throw TSTLErrors.UnsupportedForTarget("Bitwise operations", this.options.luaTarget, node);
}
Expand Down Expand Up @@ -979,31 +998,78 @@ export abstract class LuaTranspiler {
`function() return ${val1} end`, `function() return ${val2} end`);
}

public transpileBinaryAssignmentExpression(
assignee: ts.Expression,
lhs: ts.Expression,
operator: ts.BinaryOperator,
rhs: ts.Expression,
pos: number): string {
const opExp = ts.createBinary(lhs, operator, rhs);
opExp.pos = pos;
const assignExp = ts.createAssignment(assignee, opExp);
return this.transpileBinaryExpression(assignExp);
}

public transpileAssignmentExpression(
lhs: ts.Expression,
replacementExpression: ts.BinaryExpression,
operator: ts.BinaryOperator,
rhs: ts.Expression,
isStatement: boolean,
returnValueBefore: boolean): string {
// Assign replacement expression to its left-hand side
const assignment = ts.createAssignment(lhs, replacementExpression);

if (isStatement) {
return this.transpileExpression(assignment);
} else {
let statement: string; // Assignment statement
let result: string; // Result if expression
let wrap = true; // Wrap statement in do...end
const pos = (lhs.parent ? lhs.parent : lhs).pos;
const [hasEffects, objExp, indexExp] = tsHelper.isAccessExpressionWithEvaluationEffects(lhs, this.checker);
if (hasEffects) {
// Complex property/element accesses need to cache object/index expressions to avoid repeating side-effects
const objIdent = ts.createIdentifier("__TS_obj");
const indexIdent = ts.createIdentifier("__TS_index");
const tempIdent = ts.createIdentifier("__TS_tmp");
const accessExp = ts.createElementAccess(objIdent, indexIdent);
rhs = ts.createParen(rhs);
const obj = this.transpileExpression(objExp);
const index = this.transpileExpression(indexExp);
let assignTemp: string;
let assign: string;
if (returnValueBefore) {
const oldValueName = `__originalValue${this.transpileExpression(lhs)}${this.genVarCounter++}`;
const oldValueAssignment = ts.createVariableStatement(
[], [ts.createVariableDeclaration(oldValueName, undefined, lhs)]);

const valueReturn = ts.createReturn(ts.createIdentifier(oldValueName));

return this.transpileExpression(ts.createImmediatelyInvokedArrowFunction(
[oldValueAssignment, ts.createStatement(assignment), valueReturn]));
// Postfix
assignTemp = this.transpileBinaryExpression(ts.createAssignment(tempIdent, accessExp));
assign = this.transpileBinaryAssignmentExpression(accessExp, tempIdent, operator, rhs, pos);
} else {
const valueReturn = ts.createReturn(lhs);
return this.transpileExpression(
ts.createImmediatelyInvokedArrowFunction([ts.createStatement(assignment), valueReturn]));
assignTemp = this.transpileBinaryAssignmentExpression(tempIdent, accessExp, operator, rhs, pos);
assign = this.transpileBinaryExpression(ts.createAssignment(accessExp, tempIdent));
}
statement = `local __TS_obj, __TS_index = ${obj}, ${index}; local ${assignTemp}; ${assign}`;
result = "__TS_tmp";

} else if (!isStatement && returnValueBefore) {
// Postfix expressions need to cache original value in temp
const tempIdent = ts.createIdentifier("__TS_tmp");
const assignTemp = this.transpileBinaryExpression(ts.createAssignment(tempIdent, lhs));
const assign = this.transpileBinaryAssignmentExpression(lhs, tempIdent, operator, rhs, pos);
statement = `local ${assignTemp}; ${assign}`;
result = "__TS_tmp";

} else if (!isStatement && (ts.isPropertyAccessExpression(lhs) || ts.isElementAccessExpression(lhs))) {
// Simple property/element access expressions need to cache in temp to avoid double-evaluation
const tempIdent = ts.createIdentifier("__TS_tmp");
const assignTemp = this.transpileBinaryAssignmentExpression(tempIdent, lhs, operator, rhs, pos);
const assign = this.transpileBinaryExpression(ts.createAssignment(lhs, tempIdent));
statement = `local ${assignTemp}; ${assign}`;
result = "__TS_tmp";

} else {
// Simple statements/expressions
statement = this.transpileBinaryAssignmentExpression(lhs, lhs, operator, rhs, pos);
result = this.transpileExpression(lhs);
wrap = false;
}

if (isStatement) {
return wrap ? `do ${statement}; end` : statement;
} else {
return `(function() ${statement}; return ${result} end)()`;
}
}

Expand All @@ -1013,15 +1079,17 @@ export abstract class LuaTranspiler {
case ts.SyntaxKind.PlusPlusToken: {
return this.transpileAssignmentExpression(
node.operand,
ts.createBinary(node.operand, ts.SyntaxKind.PlusToken, ts.createLiteral(1)),
ts.SyntaxKind.PlusToken,
ts.createLiteral(1),
tsHelper.isExpressionStatement(node),
true
);
}
case ts.SyntaxKind.MinusMinusToken: {
return this.transpileAssignmentExpression(
node.operand,
ts.createBinary(node.operand, ts.SyntaxKind.MinusToken, ts.createLiteral(1)),
ts.SyntaxKind.MinusToken,
ts.createLiteral(1),
tsHelper.isExpressionStatement(node),
true
);
Expand All @@ -1039,15 +1107,17 @@ export abstract class LuaTranspiler {
case ts.SyntaxKind.PlusPlusToken: {
return this.transpileAssignmentExpression(
node.operand,
ts.createBinary(node.operand, ts.SyntaxKind.PlusToken, ts.createLiteral(1)),
ts.SyntaxKind.PlusToken,
ts.createLiteral(1),
tsHelper.isExpressionStatement(node),
false
);
}
case ts.SyntaxKind.MinusMinusToken: {
return this.transpileAssignmentExpression(
node.operand,
ts.createBinary(node.operand, ts.SyntaxKind.MinusToken, ts.createLiteral(1)),
ts.SyntaxKind.MinusToken,
ts.createLiteral(1),
tsHelper.isExpressionStatement(node),
false
);
Expand Down
Loading