Skip to content

Commit 1e56676

Browse files
RealColdFryRealColdFry
committed
Revert "fix >> (signed right shift) producing wrong results on Lua 5.3+"
This reverts commit 2592ff2.
1 parent 54a9d60 commit 1e56676

4 files changed

Lines changed: 70 additions & 150 deletions

File tree

‎src/transformation/utils/diagnostics.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ 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+
8488
const getLuaTargetName = (version: LuaTarget) => (version === LuaTarget.LuaJIT ? "LuaJIT" : `Lua ${version}`);
8589
export const unsupportedForTarget = createErrorDiagnosticFactory(
8690
(functionality: string, version: LuaTarget) =>

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

Lines changed: 14 additions & 50 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 } from "../../utils/diagnostics";
6+
import { unsupportedForTarget, unsupportedRightShiftOperator } from "../../utils/diagnostics";
77

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

36-
type NonShiftRightBitOperator = Exclude<
37-
BitOperator,
38-
ts.SyntaxKind.GreaterThanGreaterThanToken | ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken
39-
>;
40-
41-
function transformBitOperatorToLuaOperator(operator: NonShiftRightBitOperator): lua.BinaryOperator {
36+
function transformBitOperatorToLuaOperator(
37+
context: TransformationContext,
38+
node: ts.Node,
39+
operator: BitOperator
40+
): lua.BinaryOperator {
4241
switch (operator) {
4342
case ts.SyntaxKind.BarToken:
4443
return lua.SyntaxKind.BitwiseOrOperator;
@@ -48,6 +47,11 @@ function transformBitOperatorToLuaOperator(operator: NonShiftRightBitOperator):
4847
return lua.SyntaxKind.BitwiseAndOperator;
4948
case ts.SyntaxKind.LessThanLessThanToken:
5049
return lua.SyntaxKind.BitwiseLeftShiftOperator;
50+
case ts.SyntaxKind.GreaterThanGreaterThanToken:
51+
context.diagnostics.push(unsupportedRightShiftOperator(node));
52+
return lua.SyntaxKind.BitwiseRightShiftOperator;
53+
case ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken:
54+
return lua.SyntaxKind.BitwiseRightShiftOperator;
5155
}
5256
}
5357

@@ -71,7 +75,8 @@ export function transformBinaryBitOperation(
7175
case LuaTarget.Lua52:
7276
return transformBinaryBitLibOperation(node, left, right, operator, "bit32");
7377
default:
74-
// TS `>>>` is logical on int32; Lua 5.3+ `>>` is logical on 64-bit. Mask to 32 bits first.
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.
7580
if (operator === ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken) {
7681
const mask = lua.createBinaryExpression(
7782
left,
@@ -86,48 +91,7 @@ export function transformBinaryBitOperation(
8691
node
8792
);
8893
}
89-
// TS `>>` is arithmetic on int32; Lua 5.3+ has no native equivalent. Sign-extend the
90-
// low 32 bits to a 64-bit signed value, then floor-divide by 2^(right & 31). Masking
91-
// the shift amount matches JS, which only uses the low 5 bits of the right operand.
92-
if (operator === ts.SyntaxKind.GreaterThanGreaterThanToken) {
93-
const masked = lua.createBinaryExpression(
94-
left,
95-
lua.createNumericLiteral(0xffffffff, node),
96-
lua.SyntaxKind.BitwiseAndOperator,
97-
node
98-
);
99-
const xored = lua.createBinaryExpression(
100-
lua.createParenthesizedExpression(masked, node),
101-
lua.createNumericLiteral(0x80000000, node),
102-
lua.SyntaxKind.BitwiseExclusiveOrOperator,
103-
node
104-
);
105-
const signed = lua.createBinaryExpression(
106-
lua.createParenthesizedExpression(xored, node),
107-
lua.createNumericLiteral(0x80000000, node),
108-
lua.SyntaxKind.SubtractionOperator,
109-
node
110-
);
111-
const shiftAmount = lua.createBinaryExpression(
112-
right,
113-
lua.createNumericLiteral(31, node),
114-
lua.SyntaxKind.BitwiseAndOperator,
115-
node
116-
);
117-
const divisor = lua.createBinaryExpression(
118-
lua.createNumericLiteral(1, node),
119-
lua.createParenthesizedExpression(shiftAmount, node),
120-
lua.SyntaxKind.BitwiseLeftShiftOperator,
121-
node
122-
);
123-
return lua.createBinaryExpression(
124-
lua.createParenthesizedExpression(signed, node),
125-
lua.createParenthesizedExpression(divisor, node),
126-
lua.SyntaxKind.FloorDivisionOperator,
127-
node
128-
);
129-
}
130-
const luaOperator = transformBitOperatorToLuaOperator(operator);
94+
const luaOperator = transformBitOperatorToLuaOperator(context, node, operator);
13195
return lua.createBinaryExpression(left, right, luaOperator, node);
13296
}
13397
}

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

Lines changed: 34 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -372,13 +372,6 @@ ____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 & 31))
378-
____exports.__result = a
379-
return ____exports"
380-
`;
381-
382375
exports[`Bitop [5.3] ("a>>>=b") 1`] = `
383376
"local ____exports = {}
384377
a = (a & 4294967295) >> b
@@ -392,12 +385,6 @@ ____exports.__result = (a & 4294967295) >> b
392385
return ____exports"
393386
`;
394387
395-
exports[`Bitop [5.3] ("a>>b") 1`] = `
396-
"local ____exports = {}
397-
____exports.__result = (((a & 4294967295) ~ 2147483648) - 2147483648) // (1 << (b & 31))
398-
return ____exports"
399-
`;
400-
401388
exports[`Bitop [5.3] ("a^=b") 1`] = `
402389
"local ____exports = {}
403390
a = a ~ b
@@ -456,13 +443,6 @@ ____exports.__result = a << b
456443
return ____exports"
457444
`;
458445
459-
exports[`Bitop [5.4] ("a>>=b") 1`] = `
460-
"local ____exports = {}
461-
a = (((a & 4294967295) ~ 2147483648) - 2147483648) // (1 << (b & 31))
462-
____exports.__result = a
463-
return ____exports"
464-
`;
465-
466446
exports[`Bitop [5.4] ("a>>>=b") 1`] = `
467447
"local ____exports = {}
468448
a = (a & 4294967295) >> b
@@ -476,12 +456,6 @@ ____exports.__result = (a & 4294967295) >> b
476456
return ____exports"
477457
`;
478458
479-
exports[`Bitop [5.4] ("a>>b") 1`] = `
480-
"local ____exports = {}
481-
____exports.__result = (((a & 4294967295) ~ 2147483648) - 2147483648) // (1 << (b & 31))
482-
return ____exports"
483-
`;
484-
485459
exports[`Bitop [5.4] ("a^=b") 1`] = `
486460
"local ____exports = {}
487461
a = a ~ b
@@ -540,13 +514,6 @@ ____exports.__result = a << b
540514
return ____exports"
541515
`;
542516
543-
exports[`Bitop [5.5] ("a>>=b") 1`] = `
544-
"local ____exports = {}
545-
a = (((a & 4294967295) ~ 2147483648) - 2147483648) // (1 << (b & 31))
546-
____exports.__result = a
547-
return ____exports"
548-
`;
549-
550517
exports[`Bitop [5.5] ("a>>>=b") 1`] = `
551518
"local ____exports = {}
552519
a = (a & 4294967295) >> b
@@ -560,12 +527,6 @@ ____exports.__result = (a & 4294967295) >> b
560527
return ____exports"
561528
`;
562529
563-
exports[`Bitop [5.5] ("a>>b") 1`] = `
564-
"local ____exports = {}
565-
____exports.__result = (((a & 4294967295) ~ 2147483648) - 2147483648) // (1 << (b & 31))
566-
return ____exports"
567-
`;
568-
569530
exports[`Bitop [5.5] ("a^=b") 1`] = `
570531
"local ____exports = {}
571532
a = a ~ b
@@ -787,3 +748,37 @@ exports[`Undefined Expression 1`] = `
787748
____exports.__result = nil
788749
return ____exports"
789750
`;
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: 18 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as tstl from "../../src";
2-
import { unsupportedForTarget } from "../../src/transformation/utils/diagnostics";
2+
import { unsupportedForTarget, unsupportedRightShiftOperator } from "../../src/transformation/utils/diagnostics";
33
import * as util from "../util";
44

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

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;
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];
6855
test.each(allBinaryOperators)("Bitop [5.0] (%p)", input => {
6956
// Bit operations not supported in 5.0, expect an exception
7057
util.testExpression(input)
@@ -116,6 +103,20 @@ test.each(supportedInAll)("Bitop [5.5] (%p)", input => {
116103
.expectLuaToMatchSnapshot();
117104
});
118105

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+
119120
// Execution tests: verify >>> produces correct results matching JS semantics
120121
for (const expression of ["-5 >>> 0", "-1 >>> 0", "1 >>> 0", "-1 >>> 16", "255 >>> 4"]) {
121122
util.testEachVersion(`Unsigned right shift execution (${expression})`, () => util.testExpression(expression), {
@@ -131,36 +132,6 @@ for (const expression of ["-5 >>> 0", "-1 >>> 0", "1 >>> 0", "-1 >>> 16", "255 >
131132
});
132133
}
133134

134-
// Execution tests: verify >> produces correct results matching JS semantics
135-
for (const expression of [
136-
"-8 >> 1",
137-
"-1 >> 0",
138-
"-1 >> 16",
139-
"0x7FFFFFFF >> 0",
140-
"255 >> 4",
141-
"5 >> 1",
142-
"-1 >> 31",
143-
"0x7FFFFFFF >> 31",
144-
"0x80000000 >> 31",
145-
"1 >> 31",
146-
"-8 >> 32",
147-
"1 >> 32",
148-
"-8 >> 33",
149-
"-1 >> 63",
150-
]) {
151-
util.testEachVersion(`Signed right shift execution (${expression})`, () => util.testExpression(expression), {
152-
[tstl.LuaTarget.Universal]: false,
153-
[tstl.LuaTarget.Lua50]: false, // No bit library in WASM runtime
154-
[tstl.LuaTarget.Lua51]: false, // No bit library in WASM runtime
155-
[tstl.LuaTarget.Lua52]: false, // bit32.arshift returns uint32, not int32
156-
[tstl.LuaTarget.Lua53]: builder => builder.expectToMatchJsResult(),
157-
[tstl.LuaTarget.Lua54]: builder => builder.expectToMatchJsResult(),
158-
[tstl.LuaTarget.Lua55]: builder => builder.expectToMatchJsResult(),
159-
[tstl.LuaTarget.LuaJIT]: false, // Can't execute LuaJIT in tests
160-
[tstl.LuaTarget.Luau]: false,
161-
});
162-
}
163-
164135
for (const code of ["let a = -5; a >>>= 0; return a;", "let a = -1; a >>>= 16; return a;"]) {
165136
util.testEachVersion(`Unsigned right shift assignment execution (${code})`, () => util.testFunction(code), {
166137
[tstl.LuaTarget.Universal]: false,
@@ -175,20 +146,6 @@ for (const code of ["let a = -5; a >>>= 0; return a;", "let a = -1; a >>>= 16; r
175146
});
176147
}
177148

178-
for (const code of ["let a = -8; a >>= 1; return a;", "let a = -1; a >>= 16; return a;"]) {
179-
util.testEachVersion(`Signed right shift assignment execution (${code})`, () => util.testFunction(code), {
180-
[tstl.LuaTarget.Universal]: false,
181-
[tstl.LuaTarget.Lua50]: false, // No bit library in WASM runtime
182-
[tstl.LuaTarget.Lua51]: false, // No bit library in WASM runtime
183-
[tstl.LuaTarget.Lua52]: false, // bit32.arshift returns uint32, not int32
184-
[tstl.LuaTarget.Lua53]: builder => builder.expectToMatchJsResult(),
185-
[tstl.LuaTarget.Lua54]: builder => builder.expectToMatchJsResult(),
186-
[tstl.LuaTarget.Lua55]: builder => builder.expectToMatchJsResult(),
187-
[tstl.LuaTarget.LuaJIT]: false, // Can't execute LuaJIT in tests
188-
[tstl.LuaTarget.Luau]: false,
189-
});
190-
}
191-
192149
test.each(["1+1", "-1+1", "1*30+4", "1*(3+4)", "1*(3+4*2)", "10-(4+5)"])(
193150
"Binary expressions ordering parentheses (%p)",
194151
input => {

0 commit comments

Comments
 (0)