[facepalm] Fix bug w missing deps
While reviewing @poteto's PR I noticed that there were some cases of missing dependencies. I tracked it down to a bug I introduced [here](https://github.com/facebook/react-forget/commit/5b827eb85ce0b09a72e620449d1d676071c2e0b9#r100646304). Decl.id is meant to be the id of the instruction that declares the variable. We then test to see if a dependency is later than that. If the Decl.id is incorrectly too high, then we miss some dependencies thinking they aren't defined yet.
Joe Savona committed
Feb 14, 2023 at 16:22 UTC
008cebd633d5f28955a3ddd358c9e177d256ddc4
6 files changed
+33
-22
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+2
-1
@@ -151,6 +151,7 @@ class Context {
151
}
152
153
const decl = this.#declarations.get(maybeDependency.place.identifier.id);
154
+
155
// if decl is undefined here, then this is a free var
156
// (all other decls e.g. `let x;` should be initialized in BuildHIR)
157
@@ -376,7 +377,7 @@ function visitInstruction(context: Context, instr: ReactiveInstruction): void {
377
} else {
378
context.declare(lvalue.place.identifier, {
379
kind: DeclKind.Dynamic,
379
- id: lvalue.place.identifier.mutableRange.start,
380
+ id: instr.id,
381
scope: context.currentScope,
382
});
383
}
compiler/forget/src/__tests__/fixtures/hir/reassignment-conditional.expect.md
+7
-5
@@ -48,14 +48,16 @@ function Component(props) {
48
}
49
50
y.push(props.p2);
51
- const c_3 = $[3] !== y;
51
+ const c_3 = $[3] !== x$0;
52
+ const c_4 = $[4] !== y;
53
let t0;
53
- if (c_3) {
54
+ if (c_3 || c_4) {
55
t0 = <Component x={x$0} y={y}></Component>;
55
- $[3] = y;
56
- $[4] = t0;
56
+ $[3] = x$0;
57
+ $[4] = y;
58
+ $[5] = t0;
59
} else {
58
- t0 = $[4];
60
+ t0 = $[5];
61
}
62
return t0;
63
}
compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.expect.md
+5
-3
@@ -87,8 +87,9 @@ function foo(a, b, c) {
87
}
88
}
89
const c_8 = $[8] !== y;
90
+ const c_9 = $[9] !== x$0;
91
let t0;
91
- if (c_8) {
92
+ if (c_8 || c_9) {
93
t0 = (
94
<div>
95
{y}
@@ -96,9 +97,10 @@ function foo(a, b, c) {
97
</div>
98
);
99
$[8] = y;
99
- $[9] = t0;
100
+ $[9] = x$0;
101
+ $[10] = t0;
102
} else {
101
- t0 = $[9];
103
+ t0 = $[10];
104
}
105
return t0;
106
}
compiler/forget/src/__tests__/fixtures/hir/ssa-leave-case.expect.md
+5
-3
@@ -46,8 +46,9 @@ function Component(props) {
46
y$0 = $[3];
47
}
48
const c_4 = $[4] !== x;
49
+ const c_5 = $[5] !== y$0;
50
let t0;
50
- if (c_4) {
51
+ if (c_4 || c_5) {
52
t0 = (
53
<Component>
54
{x}
@@ -55,9 +56,10 @@ function Component(props) {
56
</Component>
57
);
58
$[4] = x;
58
- $[5] = t0;
59
+ $[5] = y$0;
60
+ $[6] = t0;
61
} else {
60
- t0 = $[5];
62
+ t0 = $[6];
63
}
64
return t0;
65
}
compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md
+7
-5
@@ -83,14 +83,16 @@ function Component(props) {
83
child = $[6];
84
}
85
y$0.push(props.p4);
86
- const c_7 = $[7] !== child;
86
+ const c_7 = $[7] !== y$0;
87
+ const c_8 = $[8] !== child;
88
let t0;
88
- if (c_7) {
89
+ if (c_7 || c_8) {
90
t0 = <Component data={y$0}>{child}</Component>;
90
- $[7] = child;
91
- $[8] = t0;
91
+ $[7] = y$0;
92
+ $[8] = child;
93
+ $[9] = t0;
94
} else {
93
- t0 = $[8];
95
+ t0 = $[9];
96
}
97
return t0;
98
}
compiler/forget/src/__tests__/fixtures/hir/switch.expect.md
+7
-5
@@ -66,14 +66,16 @@ function Component(props) {
66
child = $[6];
67
}
68
y$0.push(props.p4);
69
- const c_7 = $[7] !== child;
69
+ const c_7 = $[7] !== y$0;
70
+ const c_8 = $[8] !== child;
71
let t0;
71
- if (c_7) {
72
+ if (c_7 || c_8) {
73
t0 = <Component data={y$0}>{child}</Component>;
73
- $[7] = child;
74
- $[8] = t0;
74
+ $[7] = y$0;
75
+ $[8] = child;
76
+ $[9] = t0;
77
} else {
76
- t0 = $[8];
78
+ t0 = $[9];
79
}
80
return t0;
81
}