Fix block scoping issues from MergeConsecutiveBlocks
Fixes the issues from the previous PR. It's a simple fix — we don't merge consecutive blocks if the successor block is some terminal's fallthrough.
Joe Savona committed
Jan 23, 2024 at 11:41 UTC
c92ad38375fe0f0c99460583c7195e8ec74eafec
6 files changed
+48
-40
compiler/packages/babel-plugin-react-forget/src/HIR/MergeConsecutiveBlocks.ts
+15
-6
@@ -14,7 +14,7 @@ import {
14
Instruction,
15
} from "./HIR";
16
import { markPredecessors, removeUnreachableFallthroughs } from "./HIRBuilder";
17
-import { mapOptionalFallthroughs } from "./visitors";
17
+import { mapOptionalFallthroughs, terminalFallthrough } from "./visitors";
18
19
/*
20
* Merges sequences of blocks that will always execute consecutively —
@@ -30,7 +30,13 @@ import { mapOptionalFallthroughs } from "./visitors";
30
*/
31
export function mergeConsecutiveBlocks(fn: HIRFunction): void {
32
const merged = new MergedBlocks();
33
+ const fallthroughBlocks = new Set<BlockId>();
34
for (const [, block] of fn.body.blocks) {
35
+ const fallthrough = terminalFallthrough(block.terminal);
36
+ if (fallthrough !== null) {
37
+ fallthroughBlocks.add(fallthrough);
38
+ }
39
+
40
for (const instr of block.instructions) {
41
if (
42
instr.value.kind === "FunctionExpression" ||
@@ -40,11 +46,14 @@ export function mergeConsecutiveBlocks(fn: HIRFunction): void {
46
}
47
}
48
43
- /*
44
- * Can only merge blocks with a single predecessor, can't merge
45
- * value blocks
46
- */
47
- if (block.kind !== "block" || block.preds.size !== 1) {
49
+ if (
50
+ // Can only merge blocks with a single predecessor
51
+ block.preds.size !== 1 ||
52
+ // Value blocks cannot merge
53
+ block.kind !== "block" ||
54
+ // Merging across fallthroughs could move the predecessor out of its block scope
55
+ fallthroughBlocks.has(block.id)
56
+ ) {
57
continue;
58
}
59
const originalPredecessorId = Array.from(block.preds)[0]!;
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/block-scoping-switch-dead-code.expect.md
+8
-4
@@ -28,11 +28,14 @@ export const FIXTURE_ENTRYPOINT = {
28
29
```javascript
30
function useHook(a, b) {
31
- switch (a) {
31
+ bb1: switch (a) {
32
case 1: {
33
if (b == null) {
34
return;
35
}
36
+
37
+ console.log(b);
38
+ break bb1;
39
}
40
case 2: {
41
return;
@@ -41,8 +44,6 @@ function useHook(a, b) {
44
return;
45
}
46
}
44
-
45
- console.log(b);
47
}
48
49
export const FIXTURE_ENTRYPOINT = {
@@ -51,4 +52,7 @@ export const FIXTURE_ENTRYPOINT = {
52
};
53
54
```
54
-
\ No newline at end of file
55
+
56
+### Eval output
57
+(kind: ok)
58
+logs: ['foo']
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md
+16
-23
@@ -35,23 +35,21 @@ import { useMemo, unstable_useMemoCache as useMemoCache } from "react";
35
function Component(props) {
36
const $ = useMemoCache(2);
37
let t22;
38
- bb2: {
39
- let t0;
40
- if ($[0] !== props.value) {
41
- t0 = { value: props.value };
42
- $[0] = props.value;
43
- $[1] = t0;
44
- } else {
45
- t0 = $[1];
38
+ let t0;
39
+ if ($[0] !== props.value) {
40
+ t0 = { value: props.value };
41
+ $[0] = props.value;
42
+ $[1] = t0;
43
+ } else {
44
+ t0 = $[1];
45
+ }
46
+ const handlers = t0;
47
+ bb2: switch (props.test) {
48
+ case true: {
49
+ console.log(handlers.value);
50
+ break bb2;
51
}
47
- const handlers = t0;
48
- switch (props.test) {
49
- case true: {
50
- console.log(handlers.value);
51
- break bb2;
52
- }
53
- default: {
54
- }
52
+ default: {
53
}
54
}
55
@@ -68,10 +66,5 @@ export const FIXTURE_ENTRYPOINT = {
66
```
67
68
### Eval output
71
-(kind: exception) handlers.foo is not a function
72
-logs: ['The above error occurred in the <WrapperTestComponent> component:\n' +
73
- '\n' +
74
- ' at WrapperTestComponent (<project_root>/packages/sprout/dist/runner-evaluator.js:55:26)\n' +
75
- '\n' +
76
- 'Consider adding an error boundary to your tree to customize error handling behavior.\n' +
77
- 'Visit https://reactjs.org/link/error-boundaries to learn more about error boundaries.']
\ No newline at end of file
69
+(kind: ok) {"value":"hello"}
70
+logs: ['hello']
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/early-return-no-declarations-reassignments-dependencies.expect.md
+2
-2
@@ -79,6 +79,8 @@ function Component(props) {
79
x.push(42);
80
t37 = x;
81
break bb8;
82
+ } else {
83
+ console.log("fallthrough");
84
}
85
}
86
$[0] = t37;
@@ -88,8 +90,6 @@ function Component(props) {
90
if (t37 !== Symbol.for("react.early_return_sentinel")) {
91
return t37;
92
}
91
-
92
- console.log("fallthrough");
93
let t0;
94
if ($[1] !== props.a) {
95
t0 = makeArray(props.a);
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-inverted-if.expect.md
+2
-2
@@ -29,9 +29,9 @@ export const FIXTURE_ENTRYPOINT = {
29
function Component(props) {
30
let t16;
31
bb10: {
32
- bb5: {
32
+ bb2: {
33
if (props.cond) {
34
- break bb5;
34
+ break bb2;
35
}
36
37
t16 = props.a;
compiler/packages/sprout/src/SproutTodoFilter.ts
+5
-3
@@ -6,6 +6,11 @@
6
*/
7
8
const skipFilter = new Set([
9
+ /**
10
+ * Observable different in logging between Forget and non-Forget
11
+ */
12
+ "early-return-no-declarations-reassignments-dependencies",
13
+
14
/**
15
* Category A:
16
* Tests with 0 parameters and 0 refs to external values
@@ -517,9 +522,6 @@ const skipFilter = new Set([
522
"bug-invalid-code-when-bailout",
523
"component-syntax-ref-gating.flow",
524
520
- "block-scoping-switch-dead-code",
521
- "block-scoping-switch-variable-scoping",
522
-
525
// 'react-forget-runtime' not yet supported
526
"flag-enable-emit-hook-guards",
527
]);