From d2e5c31fc66aba93bd63616c529a6df723033ea1 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 22 Nov 2022 10:42:36 -0800 Subject: [PATCH] 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. --- .../reassignment-separate-scopes.expect.md | 166 ++++++++++++++++++ .../hir/reassignment-separate-scopes.js | 25 +++ 2 files changed, 191 insertions(+) create mode 100644 compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.js diff --git a/compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.expect.md b/compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.expect.md new file mode 100644 index 0000000000..2378bad585 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.expect.md @@ -0,0 +1,166 @@ + +## Input + +```javascript +function foo(a, b, c) { + let x = []; + if (a) { + x.push(a); + } + let y =
{x}
; + + switch (b) { + case 0: { + x = []; + x.push(b); + break; + } + default: { + x = []; + x.push(c); + } + } + return ( +
+ {y} + {x} +
+ ); +} + +``` + +## HIR + +``` +bb0: + [1] Let mutate x$4_@0[1:5] = Array [] + [2] If (read a$1) then:bb2 else:bb1 +bb2: + predecessor blocks: bb0 + [3] Call mutate x$4_@0.push(read a$1) + [4] Goto bb1 +bb1: + predecessor blocks: bb2 bb0 + [5] Const mutate $6_@1 = "div" + [6] Let mutate y$5_@2 = JSX {freeze x$4_@0} + [7] Const mutate $7_@4[7:14] = 0 + [8] Switch (read b$2) + Case read $7_@4: bb5 + Default: bb4 +bb5: + predecessor blocks: bb1 + [9] Reassign mutate x$4_@4[7:14] = Array [] + [10] Call mutate x$4_@4.push(read b$2) + [11] Goto bb3 +bb4: + predecessor blocks: bb1 + [12] Reassign mutate x$4_@4[7:14] = Array [] + [13] Call mutate x$4_@4.push(read c$3) + [14] Goto bb3 +bb3: + predecessor blocks: bb5 bb4 + [15] Const mutate $8_@5 = "div" + [16] Const mutate $9_@6 = "\n " + [17] Const mutate $10_@7 = "\n " + [18] Const mutate $11_@8 = "\n " + [19] Const mutate $12_@9 = JSX {read $9_@6}{read y$5_@2}{read $10_@7}{freeze x$4_@4}{read $11_@8} + [20] Return read $12_@9 +``` + +### CFG + +```mermaid +flowchart TB + %% Basic Blocks + subgraph bb0 + bb0_instrs[" + [1] Let mutate x$4_@0[1:5] = Array [] + "] + bb0_instrs --> bb0_terminal(["If (read a$1)"]) + end + subgraph bb2 + bb2_instrs[" + [3] Call mutate x$4_@0.push(read a$1) + "] + bb2_instrs --> bb2_terminal(["Goto"]) + end + subgraph bb1 + bb1_instrs[" + [5] Const mutate $6_@1 = 'div' + [6] Let mutate y$5_@2 = JSX {freeze x$4_@0} + [7] Const mutate $7_@4[7:14] = 0 + "] + bb1_instrs --> bb1_terminal(["Switch (read b$2)"]) + end + subgraph bb5 + bb5_instrs[" + [9] Reassign mutate x$4_@4[7:14] = Array [] + [10] Call mutate x$4_@4.push(read b$2) + "] + bb5_instrs --> bb5_terminal(["Goto"]) + end + subgraph bb4 + bb4_instrs[" + [12] Reassign mutate x$4_@4[7:14] = Array [] + [13] Call mutate x$4_@4.push(read c$3) + "] + bb4_instrs --> bb4_terminal(["Goto"]) + end + subgraph bb3 + bb3_instrs[" + [15] Const mutate $8_@5 = 'div' + [16] Const mutate $9_@6 = '\n ' + [17] Const mutate $10_@7 = '\n ' + [18] Const mutate $11_@8 = '\n ' + [19] Const mutate $12_@9 = JSX {read $9_@6}{read y$5_@2}{read $10_@7}{freeze x$4_@4}{read $11_@8} + "] + bb3_instrs --> bb3_terminal(["Return read $12_@9"]) + end + + %% Jumps + bb0_terminal -- "then" --> bb2 + bb0_terminal -- "else" --> bb1 + bb2_terminal --> bb1 + bb1_terminal -- "read $7_@4" --> bb5 + bb1_terminal -- "default" --> bb4 + bb1_terminal -- "fallthrough" --> bb3 + bb5_terminal --> bb3 + bb4_terminal --> bb3 + +``` + +## Code + +```javascript +function foo$0(a$1, b$2, c$3) { + let x$4 = []; + bb1: if (a$1) { + x$4.push(a$1); + } + + let y$5 =
{x$4}
; + + bb3: switch (b$2) { + case 0: { + x$4 = []; + x$4.push(b$2); + break bb3; + } + + default: { + x$4 = []; + x$4.push(c$3); + } + } + + return ( +
+ {y$5} + {x$4} +
+ ); +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.js b/compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.js new file mode 100644 index 0000000000..39afb78096 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.js @@ -0,0 +1,25 @@ +function foo(a, b, c) { + let x = []; + if (a) { + x.push(a); + } + let y =
{x}
; + + switch (b) { + case 0: { + x = []; + x.push(b); + break; + } + default: { + x = []; + x.push(c); + } + } + return ( +
+ {y} + {x} +
+ ); +}