@samitouri / QOS-React-2 / commits / d2e5c31fc6

Test case showing reassignment block scoping problem

This demonstrates a situation we don't handle well today. The basic structure is that you have some variable defined at the top level, then some control flow like if/switch where _all_ branches reassign the variable, then some code after that references the resulting phi node: ```javascript let x1; // ... mutate/read x1 if (cond) { x2 = {}; } else { x3 = {}; } x4 = phi(x2, x3); ``` We currently group x3, x3, and x4 into a scope together, but note that...there's no `let` declaration for any of those! This means that it looks like the scope for x2 and x3 start in the consequent/alternate, but the true scope spans from before-after the if. I'm inclined to say that LeaveSSA should run _before_ scope analysis, and produce something like the following in this case: ```javascript let x1; // ...mutate/read x1 let x2; // new variable declaration for the new version of x if (cond) { x2 = {}; } else { x2 = {}; } x2; ``` This then allows us to construct a correct range for x2, which starts in the other block.

Joe Savona committed Nov 22, 2022 at 10:42 UTC d2e5c31fc66aba93bd63616c529a6df723033ea1
2 files changed +191
compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.expect.md new
+166
@@ -0,0 +1,166 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function foo(a, b, c) {
6 + let x = [];
7 + if (a) {
8 + x.push(a);
9 + }
10 + let y = <div>{x}</div>;
11 +
12 + switch (b) {
13 + case 0: {
14 + x = [];
15 + x.push(b);
16 + break;
17 + }
18 + default: {
19 + x = [];
20 + x.push(c);
21 + }
22 + }
23 + return (
24 + <div>
25 + {y}
26 + {x}
27 + </div>
28 + );
29 +}
30 +
31 +```
32 +
33 +## HIR
34 +
35 +```
36 +bb0:
37 + [1] Let mutate x$4_@0[1:5] = Array []
38 + [2] If (read a$1) then:bb2 else:bb1
39 +bb2:
40 + predecessor blocks: bb0
41 + [3] Call mutate x$4_@0.push(read a$1)
42 + [4] Goto bb1
43 +bb1:
44 + predecessor blocks: bb2 bb0
45 + [5] Const mutate $6_@1 = "div"
46 + [6] Let mutate y$5_@2 = JSX <read $6_@1>{freeze x$4_@0}</read $6_@1>
47 + [7] Const mutate $7_@4[7:14] = 0
48 + [8] Switch (read b$2)
49 + Case read $7_@4: bb5
50 + Default: bb4
51 +bb5:
52 + predecessor blocks: bb1
53 + [9] Reassign mutate x$4_@4[7:14] = Array []
54 + [10] Call mutate x$4_@4.push(read b$2)
55 + [11] Goto bb3
56 +bb4:
57 + predecessor blocks: bb1
58 + [12] Reassign mutate x$4_@4[7:14] = Array []
59 + [13] Call mutate x$4_@4.push(read c$3)
60 + [14] Goto bb3
61 +bb3:
62 + predecessor blocks: bb5 bb4
63 + [15] Const mutate $8_@5 = "div"
64 + [16] Const mutate $9_@6 = "\n "
65 + [17] Const mutate $10_@7 = "\n "
66 + [18] Const mutate $11_@8 = "\n "
67 + [19] Const mutate $12_@9 = JSX <read $8_@5>{read $9_@6}{read y$5_@2}{read $10_@7}{freeze x$4_@4}{read $11_@8}</read $8_@5>
68 + [20] Return read $12_@9
69 +```
70 +
71 +### CFG
72 +
73 +```mermaid
74 +flowchart TB
75 + %% Basic Blocks
76 + subgraph bb0
77 + bb0_instrs["
78 + [1] Let mutate x$4_@0[1:5] = Array []
79 + "]
80 + bb0_instrs --> bb0_terminal(["If (read a$1)"])
81 + end
82 + subgraph bb2
83 + bb2_instrs["
84 + [3] Call mutate x$4_@0.push(read a$1)
85 + "]
86 + bb2_instrs --> bb2_terminal(["Goto"])
87 + end
88 + subgraph bb1
89 + bb1_instrs["
90 + [5] Const mutate $6_@1 = 'div'
91 + [6] Let mutate y$5_@2 = JSX <read $6_@1>{freeze x$4_@0}</read $6_@1>
92 + [7] Const mutate $7_@4[7:14] = 0
93 + "]
94 + bb1_instrs --> bb1_terminal(["Switch (read b$2)"])
95 + end
96 + subgraph bb5
97 + bb5_instrs["
98 + [9] Reassign mutate x$4_@4[7:14] = Array []
99 + [10] Call mutate x$4_@4.push(read b$2)
100 + "]
101 + bb5_instrs --> bb5_terminal(["Goto"])
102 + end
103 + subgraph bb4
104 + bb4_instrs["
105 + [12] Reassign mutate x$4_@4[7:14] = Array []
106 + [13] Call mutate x$4_@4.push(read c$3)
107 + "]
108 + bb4_instrs --> bb4_terminal(["Goto"])
109 + end
110 + subgraph bb3
111 + bb3_instrs["
112 + [15] Const mutate $8_@5 = 'div'
113 + [16] Const mutate $9_@6 = '\n '
114 + [17] Const mutate $10_@7 = '\n '
115 + [18] Const mutate $11_@8 = '\n '
116 + [19] Const mutate $12_@9 = JSX <read $8_@5>{read $9_@6}{read y$5_@2}{read $10_@7}{freeze x$4_@4}{read $11_@8}</read $8_@5>
117 + "]
118 + bb3_instrs --> bb3_terminal(["Return read $12_@9"])
119 + end
120 +
121 + %% Jumps
122 + bb0_terminal -- "then" --> bb2
123 + bb0_terminal -- "else" --> bb1
124 + bb2_terminal --> bb1
125 + bb1_terminal -- "read $7_@4" --> bb5
126 + bb1_terminal -- "default" --> bb4
127 + bb1_terminal -- "fallthrough" --> bb3
128 + bb5_terminal --> bb3
129 + bb4_terminal --> bb3
130 +
131 +```
132 +
133 +## Code
134 +
135 +```javascript
136 +function foo$0(a$1, b$2, c$3) {
137 + let x$4 = [];
138 + bb1: if (a$1) {
139 + x$4.push(a$1);
140 + }
141 +
142 + let y$5 = <div>{x$4}</div>;
143 +
144 + bb3: switch (b$2) {
145 + case 0: {
146 + x$4 = [];
147 + x$4.push(b$2);
148 + break bb3;
149 + }
150 +
151 + default: {
152 + x$4 = [];
153 + x$4.push(c$3);
154 + }
155 + }
156 +
157 + return (
158 + <div>
159 + {y$5}
160 + {x$4}
161 + </div>
162 + );
163 +}
164 +
165 +```
166 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.js new
+25
@@ -0,0 +1,25 @@
1 +function foo(a, b, c) {
2 + let x = [];
3 + if (a) {
4 + x.push(a);
5 + }
6 + let y = <div>{x}</div>;
7 +
8 + switch (b) {
9 + case 0: {
10 + x = [];
11 + x.push(b);
12 + break;
13 + }
14 + default: {
15 + x = [];
16 + x.push(c);
17 + }
18 + }
19 + return (
20 + <div>
21 + {y}
22 + {x}
23 + </div>
24 + );
25 +}