Fix last(?) order of evaluation bug
Not to get ahead of myself (sorry i had to), but i think this is the last order of evaluation bug. At least it's the last one we know of[1]. Per the previous PR, the issue is that constant propagation can copy the last value of a sequence expression to where the sequence is used, leaving the original sequence expression out of order after other instructions are moved around. We fix that here by explicitly skipping constant propagation for the last value of a sequence block. [1] There are some places where we _would_ have evaluation order bugs if we allowed arbitrary expressions, but we explicitly limit the expressions we allow in those places. For the curious: switch test case values and destructuring default values.
Joe Savona committed
Jun 4, 2023 at 21:41 UTC
3744930728bfe6a08c382b25834e258bc44bc07f
5 files changed
+12
-5
compiler/forget/src/HIR/BuildHIR.ts
+1
-1
@@ -1171,7 +1171,7 @@ function lowerExpression(
1171
const continuationBlock = builder.reserve(builder.currentBlockKind());
1172
const place = buildTemporaryPlace(builder, exprLoc);
1173
1174
- const sequenceBlock = builder.enter("value", (_) => {
1174
+ const sequenceBlock = builder.enter("sequence", (_) => {
1175
let last: Place | null = null;
1176
for (const item of expr.get("expressions")) {
1177
last = lowerExpressionToTemporary(builder, item);
compiler/forget/src/HIR/HIR.ts
+1
-1
@@ -250,7 +250,7 @@ export type HIR = {
250
* an exception occurs, therefore the block model only represents explicit throw
251
* statements and not implicit exceptions which may occur.
252
*/
253
-export type BlockKind = "block" | "value" | "loop";
253
+export type BlockKind = "block" | "value" | "loop" | "sequence";
254
export type BasicBlock = {
255
kind: BlockKind;
256
id: BlockId;
compiler/forget/src/Optimization/ConstantPropagation.ts
+7
-1
@@ -113,7 +113,13 @@ function applyConstantPropagation(fn: HIRFunction): boolean {
113
}
114
}
115
116
- for (const instr of block.instructions) {
116
+ for (let i = 0; i < block.instructions.length; i++) {
117
+ if (block.kind === "sequence" && i === block.instructions.length - 1) {
118
+ // evaluating the last value of a value block can break order of evaluation,
119
+ // skip these instructions
120
+ continue;
121
+ }
122
+ const instr = block.instructions[i]!;
123
const value = evaluateInstruction(constants, instr);
124
if (value !== null) {
125
constants.set(instr.lvalue.identifier.id, value);
compiler/forget/src/__tests__/fixtures/compiler/computed-call-evaluation-order.expect.md
renamed
+3
-2
@@ -46,8 +46,9 @@ function Component() {
46
if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
47
x = { f: t1 };
48
49
- console.log("B"), "f";
50
- (console.log("A"), x).f((changeF(x), console.log("arg"), 1));
49
+ (console.log("A"), x)[(console.log("B"), "f")](
50
+ (changeF(x), console.log("arg"), 1)
51
+ );
52
$[2] = x;
53
} else {
54
x = $[2];