Skip to content

Fix cast exception when transforming compound length assignment #1398

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 1 commit into from
Feb 12, 2023
Merged

Fix cast exception when transforming compound length assignment #1398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions src/transformation/utils/preceding-statements.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import * as lua from "../../LuaAST";
import { TransformationContext } from "../context";

export interface WithPrecedingStatements<
T extends lua.Statement | lua.Statement[] | lua.Expression | lua.Expression[]
> {
precedingStatements: lua.Statement[];
result: T;
}

export function transformInPrecedingStatementScope<
TReturn extends lua.Statement | lua.Statement[] | lua.Expression | lua.Expression[]
>(context: TransformationContext, transformer: () => TReturn): [lua.Statement[], TReturn] {
Expand Down
91 changes: 80 additions & 11 deletions src/transformation/visitors/binary-expression/compound.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ import * as ts from "typescript";
import * as lua from "../../../LuaAST";
import { cast, assertNever } from "../../../utils";
import { TransformationContext } from "../../context";
import { transformInPrecedingStatementScope } from "../../utils/preceding-statements";
import { transformInPrecedingStatementScope, WithPrecedingStatements } from "../../utils/preceding-statements";
import { transformBinaryOperation } from "./index";
import { transformAssignmentWithRightPrecedingStatements } from "./assignments";
import { isArrayLength } from "./destructuring-assignments";
import { LuaLibFeature, transformLuaLibFunction } from "../../utils/lualib";

function isLuaExpressionWithSideEffect(expression: lua.Expression) {
return !(lua.isLiteral(expression) || lua.isIdentifier(expression));
Expand Down Expand Up @@ -63,14 +65,26 @@ export const isCompoundAssignmentToken = (token: ts.BinaryOperator): token is ts
export const unwrapCompoundAssignmentToken = (token: ts.CompoundAssignmentOperator): CompoundAssignmentToken =>
compoundToAssignmentTokens[token];

export function transformCompoundAssignment(
function transformCompoundAssignment(
context: TransformationContext,
expression: ts.Expression,
lhs: ts.Expression,
rhs: ts.Expression,
operator: CompoundAssignmentToken,
isPostfix: boolean
) {
): WithPrecedingStatements<lua.Expression> {
if (isArrayLength(context, lhs)) {
const { precedingStatements, result: lengthSetterStatement } = transformCompoundLengthSetter(
context,
expression,
lhs,
rhs,
operator
);

return { precedingStatements, result: lengthSetterStatement.expression };
}

const left = cast(context.transformExpression(lhs), lua.isAssignmentLeftHandSideExpression);
const [rightPrecedingStatements, right] = transformInPrecedingStatementScope(context, () =>
context.transformExpression(rhs)
Expand Down Expand Up @@ -101,13 +115,13 @@ export function transformCompoundAssignment(
);
const assignStatement = lua.createAssignmentStatement(accessExpression, operatorExpression);
return {
statements: [objAndIndexDeclaration, ...precedingStatements, tmpDeclaration, assignStatement],
precedingStatements: [objAndIndexDeclaration, ...precedingStatements, tmpDeclaration, assignStatement],
result: tmp,
};
} else {
if (isSetterSkippingCompoundAssignmentOperator(operator)) {
return {
statements: [
precedingStatements: [
objAndIndexDeclaration,
...transformSetterSkippingCompoundAssignment(
accessExpression,
Expand All @@ -133,7 +147,7 @@ export function transformCompoundAssignment(
const tmpDeclaration = lua.createVariableDeclarationStatement(tmp, operatorExpression);
const assignStatement = lua.createAssignmentStatement(accessExpression, tmp);
return {
statements: [objAndIndexDeclaration, ...precedingStatements, tmpDeclaration, assignStatement],
precedingStatements: [objAndIndexDeclaration, ...precedingStatements, tmpDeclaration, assignStatement],
result: tmp,
};
}
Expand All @@ -158,11 +172,19 @@ export function transformCompoundAssignment(
operatorExpression,
rightPrecedingStatements
);
return { statements: [tmpDeclaration, ...precedingStatements, ...assignStatements], result: tmpIdentifier };
return {
precedingStatements: [tmpDeclaration, ...precedingStatements, ...assignStatements],
result: tmpIdentifier,
};
} else {
if (rightPrecedingStatements.length > 0 && isSetterSkippingCompoundAssignmentOperator(operator)) {
return {
statements: transformSetterSkippingCompoundAssignment(left, operator, right, rightPrecedingStatements),
precedingStatements: transformSetterSkippingCompoundAssignment(
left,
operator,
right,
rightPrecedingStatements
),
result: left,
};
}
Expand All @@ -183,7 +205,7 @@ export function transformCompoundAssignment(
operatorExpression,
precedingStatements
);
return { statements, result: left };
return { precedingStatements: statements, result: left };
}
}

Expand All @@ -196,8 +218,15 @@ export function transformCompoundAssignmentExpression(
operator: CompoundAssignmentToken,
isPostfix: boolean
): lua.Expression {
const { statements, result } = transformCompoundAssignment(context, expression, lhs, rhs, operator, isPostfix);
context.addPrecedingStatements(statements);
const { precedingStatements, result } = transformCompoundAssignment(
context,
expression,
lhs,
rhs,
operator,
isPostfix
);
context.addPrecedingStatements(precedingStatements);
return result;
}

Expand All @@ -208,6 +237,18 @@ export function transformCompoundAssignmentStatement(
rhs: ts.Expression,
operator: CompoundAssignmentToken
): lua.Statement[] {
if (isArrayLength(context, lhs)) {
const { precedingStatements, result: lengthSetterStatement } = transformCompoundLengthSetter(
context,
node,
lhs,
rhs,
operator
);

return [...precedingStatements, lengthSetterStatement];
}

const left = cast(context.transformExpression(lhs), lua.isAssignmentLeftHandSideExpression);
let [rightPrecedingStatements, right] = transformInPrecedingStatementScope(context, () =>
context.transformExpression(rhs)
Expand Down Expand Up @@ -319,3 +360,31 @@ function transformSetterSkippingCompoundAssignment(
),
];
}

function transformCompoundLengthSetter(
context: TransformationContext,
node: ts.Node,
lhs: ts.PropertyAccessExpression | ts.ElementAccessExpression,
rhs: ts.Expression,
operator: CompoundAssignmentToken
): WithPrecedingStatements<lua.ExpressionStatement> {
const [rightPrecedingStatements, right] = transformInPrecedingStatementScope(context, () =>
context.transformExpression(rhs)
);
const table = context.transformExpression(lhs.expression);
const lengthExpression = lua.createUnaryExpression(table, lua.SyntaxKind.LengthOperator, lhs);
const [precedingStatements, operatorExpression] = transformBinaryOperation(
context,
lengthExpression,
right,
rightPrecedingStatements,
operator,
node
);

const arrayLengthAssignment = lua.createExpressionStatement(
transformLuaLibFunction(context, LuaLibFeature.ArraySetLength, node, table, operatorExpression)
);

return { precedingStatements, result: arrayLengthAssignment };
}
17 changes: 17 additions & 0 deletions test/unit/builtins/array.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,23 @@ describe("array.length", () => {
`.expectToEqual(new util.ExecutionError(`invalid array length: ${luaSpecialValueString}`));
});

// https://github.com/TypeScriptToLua/TypeScriptToLua/issues/1395
test("in compound assignment (#1395)", () => {
util.testFunction`
const arr = [1,2,3,4];
const returnVal = arr.length -= 2;
return { arr, returnVal };
`.expectToMatchJsResult();
});

test("as standalone compound assignment (#1395)", () => {
util.testFunction`
const arr = [1,2,3,4];
arr.length -= 2;
return arr;
`.expectToMatchJsResult();
});

test("in array destructuring", () => {
util.testFunction`
const array = [0, 1, 2];
Expand Down