Skip to content

Commit 2592ff2

Browse files
RealColdFryRealColdFry
committed
fix >> (signed right shift) producing wrong results on Lua 5.3+
1 parent ef946a3 commit 2592ff2

4 files changed

Lines changed: 109 additions & 67 deletions

File tree

‎src/transformation/utils/diagnostics.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,6 @@ export const unsupportedAccessorInObjectLiteral = createErrorDiagnosticFactory(
8181
"Accessors in object literal are not supported."
8282
);
8383

84-
export const unsupportedRightShiftOperator = createErrorDiagnosticFactory(
85-
"Right shift operator is not supported for target Lua 5.3. Use `>>>` instead."
86-
);
87-
8884
const getLuaTargetName = (version: LuaTarget) => (version === LuaTarget.LuaJIT ? "LuaJIT" : `Lua ${version}`);
8985
export const unsupportedForTarget = createErrorDiagnosticFactory(
9086
(functionality: string, version: LuaTarget) =>

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

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { LuaTarget } from "../../../CompilerOptions";
33
import * as lua from "../../../LuaAST";
44
import { assertNever } from "../../../utils";
55
import { TransformationContext } from "../../context";
6-
import { unsupportedForTarget, unsupportedRightShiftOperator } from "../../utils/diagnostics";
6+
import { unsupportedForTarget } from "../../utils/diagnostics";
77

88
export type BitOperator = ts.ShiftOperator | ts.BitwiseOperator;
99
export const isBitOperator = (operator: ts.BinaryOperator): operator is BitOperator =>
@@ -33,11 +33,7 @@ function transformBinaryBitLibOperation(
3333
);
3434
}
3535

36-
function transformBitOperatorToLuaOperator(
37-
context: TransformationContext,
38-
node: ts.Node,
39-
operator: BitOperator
40-
): lua.BinaryOperator {
36+
function transformBitOperatorToLuaOperator(operator: BitOperator): lua.BinaryOperator {
4137
switch (operator) {
4238
case ts.SyntaxKind.BarToken:
4339
return lua.SyntaxKind.BitwiseOrOperator;
@@ -48,8 +44,6 @@ function transformBitOperatorToLuaOperator(
4844
case ts.SyntaxKind.LessThanLessThanToken:
4945
return lua.SyntaxKind.BitwiseLeftShiftOperator;
5046
case ts.SyntaxKind.GreaterThanGreaterThanToken:
51-
context.diagnostics.push(unsupportedRightShiftOperator(node));
52-
return lua.SyntaxKind.BitwiseRightShiftOperator;
5347
case ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken:
5448
return lua.SyntaxKind.BitwiseRightShiftOperator;
5549
}
@@ -75,8 +69,7 @@ export function transformBinaryBitOperation(
7569
case LuaTarget.Lua52:
7670
return transformBinaryBitLibOperation(node, left, right, operator, "bit32");
7771
default:
78-
// Lua 5.3+ `>>` is arithmetic (sign-extending), but TS `>>>` is logical (zero-fill).
79-
// Emit `(left & 0xFFFFFFFF) >> right` to convert to unsigned 32-bit first.
72+
// TS `>>>` is logical on int32; Lua 5.3+ `>>` is logical on 64-bit. Mask to 32 bits first.
8073
if (operator === ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken) {
8174
const mask = lua.createBinaryExpression(
8275
left,
@@ -91,7 +84,41 @@ export function transformBinaryBitOperation(
9184
node
9285
);
9386
}
94-
const luaOperator = transformBitOperatorToLuaOperator(context, node, operator);
87+
// TS `>>` is arithmetic on int32; Lua 5.3+ has no native equivalent. Sign-extend the
88+
// low 32 bits to a 64-bit signed value, then floor-divide by 2^right.
89+
if (operator === ts.SyntaxKind.GreaterThanGreaterThanToken) {
90+
const masked = lua.createBinaryExpression(
91+
left,
92+
lua.createNumericLiteral(0xffffffff, node),
93+
lua.SyntaxKind.BitwiseAndOperator,
94+
node
95+
);
96+
const xored = lua.createBinaryExpression(
97+
lua.createParenthesizedExpression(masked, node),
98+
lua.createNumericLiteral(0x80000000, node),
99+
lua.SyntaxKind.BitwiseExclusiveOrOperator,
100+
node
101+
);
102+
const signed = lua.createBinaryExpression(
103+
lua.createParenthesizedExpression(xored, node),
104+
lua.createNumericLiteral(0x80000000, node),
105+
lua.SyntaxKind.SubtractionOperator,
106+
node
107+
);
108+
const divisor = lua.createBinaryExpression(
109+
lua.createNumericLiteral(1, node),
110+
right,
111+
lua.SyntaxKind.BitwiseLeftShiftOperator,
112+
node
113+
);
114+
return lua.createBinaryExpression(
115+
lua.createParenthesizedExpression(signed, node),
116+
lua.createParenthesizedExpression(divisor, node),
117+
lua.SyntaxKind.FloorDivisionOperator,
118+
node
119+
);
120+
}
121+
const luaOperator = transformBitOperatorToLuaOperator(operator);
95122
return lua.createBinaryExpression(left, right, luaOperator, node);
96123
}
97124
}

‎test/unit/__snapshots__/expressions.spec.ts.snap‎

Lines changed: 39 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,13 @@ ____exports.__result = a << b
372372
return ____exports"
373373
`;
374374
375+
exports[`Bitop [5.3] ("a>>=b") 1`] = `
376+
"local ____exports = {}
377+
a = (((a & 4294967295) ~ 2147483648) - 2147483648) // (1 << b)
378+
____exports.__result = a
379+
return ____exports"
380+
`;
381+
375382
exports[`Bitop [5.3] ("a>>>=b") 1`] = `
376383
"local ____exports = {}
377384
a = (a & 4294967295) >> b
@@ -385,6 +392,12 @@ ____exports.__result = (a & 4294967295) >> b
385392
return ____exports"
386393
`;
387394
395+
exports[`Bitop [5.3] ("a>>b") 1`] = `
396+
"local ____exports = {}
397+
____exports.__result = (((a & 4294967295) ~ 2147483648) - 2147483648) // (1 << b)
398+
return ____exports"
399+
`;
400+
388401
exports[`Bitop [5.3] ("a^=b") 1`] = `
389402
"local ____exports = {}
390403
a = a ~ b
@@ -443,6 +456,13 @@ ____exports.__result = a << b
443456
return ____exports"
444457
`;
445458
459+
exports[`Bitop [5.4] ("a>>=b") 1`] = `
460+
"local ____exports = {}
461+
a = (((a & 4294967295) ~ 2147483648) - 2147483648) // (1 << b)
462+
____exports.__result = a
463+
return ____exports"
464+
`;
465+
446466
exports[`Bitop [5.4] ("a>>>=b") 1`] = `
447467
"local ____exports = {}
448468
a = (a & 4294967295) >> b
@@ -456,6 +476,12 @@ ____exports.__result = (a & 4294967295) >> b
456476
return ____exports"
457477
`;
458478
479+
exports[`Bitop [5.4] ("a>>b") 1`] = `
480+
"local ____exports = {}
481+
____exports.__result = (((a & 4294967295) ~ 2147483648) - 2147483648) // (1 << b)
482+
return ____exports"
483+
`;
484+
459485
exports[`Bitop [5.4] ("a^=b") 1`] = `
460486
"local ____exports = {}
461487
a = a ~ b
@@ -514,6 +540,13 @@ ____exports.__result = a << b
514540
return ____exports"
515541
`;
516542
543+
exports[`Bitop [5.5] ("a>>=b") 1`] = `
544+
"local ____exports = {}
545+
a = (((a & 4294967295) ~ 2147483648) - 2147483648) // (1 << b)
546+
____exports.__result = a
547+
return ____exports"
548+
`;
549+
517550
exports[`Bitop [5.5] ("a>>>=b") 1`] = `
518551
"local ____exports = {}
519552
a = (a & 4294967295) >> b
@@ -527,6 +560,12 @@ ____exports.__result = (a & 4294967295) >> b
527560
return ____exports"
528561
`;
529562
563+
exports[`Bitop [5.5] ("a>>b") 1`] = `
564+
"local ____exports = {}
565+
____exports.__result = (((a & 4294967295) ~ 2147483648) - 2147483648) // (1 << b)
566+
return ____exports"
567+
`;
568+
530569
exports[`Bitop [5.5] ("a^=b") 1`] = `
531570
"local ____exports = {}
532571
a = a ~ b
@@ -748,37 +787,3 @@ exports[`Undefined Expression 1`] = `
748787
____exports.__result = nil
749788
return ____exports"
750789
`;
751-
752-
exports[`Unsupported bitop 5.3 ("a>>=b"): code 1`] = `
753-
"local ____exports = {}
754-
a = a >> b
755-
____exports.__result = a
756-
return ____exports"
757-
`;
758-
759-
exports[`Unsupported bitop 5.3 ("a>>=b"): diagnostics 1`] = `"main.ts(1,25): error TSTL: Right shift operator is not supported for target Lua 5.3. Use \`>>>\` instead."`;
760-
761-
exports[`Unsupported bitop 5.3 ("a>>b"): code 1`] = `
762-
"local ____exports = {}
763-
____exports.__result = a >> b
764-
return ____exports"
765-
`;
766-
767-
exports[`Unsupported bitop 5.3 ("a>>b"): diagnostics 1`] = `"main.ts(1,25): error TSTL: Right shift operator is not supported for target Lua 5.3. Use \`>>>\` instead."`;
768-
769-
exports[`Unsupported bitop 5.4 ("a>>=b"): code 1`] = `
770-
"local ____exports = {}
771-
a = a >> b
772-
____exports.__result = a
773-
return ____exports"
774-
`;
775-
776-
exports[`Unsupported bitop 5.4 ("a>>=b"): diagnostics 1`] = `"main.ts(1,25): error TSTL: Right shift operator is not supported for target Lua 5.3. Use \`>>>\` instead."`;
777-
778-
exports[`Unsupported bitop 5.4 ("a>>b"): code 1`] = `
779-
"local ____exports = {}
780-
____exports.__result = a >> b
781-
return ____exports"
782-
`;
783-
784-
exports[`Unsupported bitop 5.4 ("a>>b"): diagnostics 1`] = `"main.ts(1,25): error TSTL: Right shift operator is not supported for target Lua 5.3. Use \`>>>\` instead."`;

‎test/unit/expressions.spec.ts‎

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as tstl from "../../src";
2-
import { unsupportedForTarget, unsupportedRightShiftOperator } from "../../src/transformation/utils/diagnostics";
2+
import { unsupportedForTarget } from "../../src/transformation/utils/diagnostics";
33
import * as util from "../util";
44

55
test.each([
@@ -49,9 +49,22 @@ test.each(["a+=b", "a-=b", "a*=b", "a/=b", "a%=b", "a**=b"])("Binary expressions
4949
`.expectToMatchJsResult();
5050
});
5151

52-
const supportedInAll = ["~a", "a&b", "a&=b", "a|b", "a|=b", "a^b", "a^=b", "a<<b", "a<<=b", "a>>>b", "a>>>=b"];
53-
const unsupportedIn53And54 = ["a>>b", "a>>=b"];
54-
const allBinaryOperators = [...supportedInAll, ...unsupportedIn53And54];
52+
const supportedInAll = [
53+
"~a",
54+
"a&b",
55+
"a&=b",
56+
"a|b",
57+
"a|=b",
58+
"a^b",
59+
"a^=b",
60+
"a<<b",
61+
"a<<=b",
62+
"a>>b",
63+
"a>>=b",
64+
"a>>>b",
65+
"a>>>=b",
66+
];
67+
const allBinaryOperators = supportedInAll;
5568
test.each(allBinaryOperators)("Bitop [5.0] (%p)", input => {
5669
// Bit operations not supported in 5.0, expect an exception
5770
util.testExpression(input)
@@ -103,20 +116,6 @@ test.each(supportedInAll)("Bitop [5.5] (%p)", input => {
103116
.expectLuaToMatchSnapshot();
104117
});
105118

106-
test.each(unsupportedIn53And54)("Unsupported bitop 5.3 (%p)", input => {
107-
util.testExpression(input)
108-
.setOptions({ luaTarget: tstl.LuaTarget.Lua53, luaLibImport: tstl.LuaLibImportKind.None })
109-
.disableSemanticCheck()
110-
.expectDiagnosticsToMatchSnapshot([unsupportedRightShiftOperator.code]);
111-
});
112-
113-
test.each(unsupportedIn53And54)("Unsupported bitop 5.4 (%p)", input => {
114-
util.testExpression(input)
115-
.setOptions({ luaTarget: tstl.LuaTarget.Lua54, luaLibImport: tstl.LuaLibImportKind.None })
116-
.disableSemanticCheck()
117-
.expectDiagnosticsToMatchSnapshot([unsupportedRightShiftOperator.code]);
118-
});
119-
120119
// Execution tests: verify >>> produces correct results matching JS semantics
121120
for (const expression of ["-5 >>> 0", "-1 >>> 0", "1 >>> 0", "-1 >>> 16", "255 >>> 4"]) {
122121
util.testEachVersion(`Unsigned right shift execution (${expression})`, () => util.testExpression(expression), {
@@ -132,6 +131,21 @@ for (const expression of ["-5 >>> 0", "-1 >>> 0", "1 >>> 0", "-1 >>> 16", "255 >
132131
});
133132
}
134133

134+
// Execution tests: verify >> produces correct results matching JS semantics
135+
for (const expression of ["-8 >> 1", "-1 >> 0", "-1 >> 16", "0x7FFFFFFF >> 0", "255 >> 4", "5 >> 1"]) {
136+
util.testEachVersion(`Signed right shift execution (${expression})`, () => util.testExpression(expression), {
137+
[tstl.LuaTarget.Universal]: false,
138+
[tstl.LuaTarget.Lua50]: false, // No bit library in WASM runtime
139+
[tstl.LuaTarget.Lua51]: false, // No bit library in WASM runtime
140+
[tstl.LuaTarget.Lua52]: false, // bit32.arshift returns uint32, not int32
141+
[tstl.LuaTarget.Lua53]: builder => builder.expectToMatchJsResult(),
142+
[tstl.LuaTarget.Lua54]: builder => builder.expectToMatchJsResult(),
143+
[tstl.LuaTarget.Lua55]: builder => builder.expectToMatchJsResult(),
144+
[tstl.LuaTarget.LuaJIT]: false, // Can't execute LuaJIT in tests
145+
[tstl.LuaTarget.Luau]: false,
146+
});
147+
}
148+
135149
for (const code of ["let a = -5; a >>>= 0; return a;", "let a = -1; a >>>= 16; return a;"]) {
136150
util.testEachVersion(`Unsigned right shift assignment execution (${code})`, () => util.testFunction(code), {
137151
[tstl.LuaTarget.Universal]: false,

0 commit comments

Comments
 (0)