Propagate scope declarations to parent scopes
Fix for bug demonstrated in #1506. When we add variables as output of their defining scope, we need to propagate this information upwards to all parent scopes which are not current active.
Joe Savona committed
Apr 18, 2023 at 21:27 UTC
a0777212df7009cc26e47ffd46444276d0be923d
8 files changed
+216
-116
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+28
-26
@@ -23,6 +23,7 @@ import {
23
eachInstructionValueOperand,
24
eachPatternOperand,
25
} from "../HIR/visitors";
26
+import { empty, Stack } from "../Utils/Stack";
27
import { assertExhaustive } from "../Utils/utils";
28
import {
29
ReactiveScopeDependencyTree,
@@ -40,13 +41,13 @@ export function propagateScopeDependencies(fn: ReactiveFunction): void {
41
if (fn.id !== null) {
42
context.declare(fn.id, {
43
id: makeInstructionId(0),
43
- scope: null,
44
+ scope: empty(),
45
});
46
}
47
for (const param of fn.params) {
48
context.declare(param.identifier, {
49
id: makeInstructionId(0),
49
- scope: null,
50
+ scope: empty(),
51
});
52
}
53
visit(context, fn.body);
@@ -55,11 +56,9 @@ export function propagateScopeDependencies(fn: ReactiveFunction): void {
56
type DeclMap = Map<IdentifierId, Decl>;
57
type Decl = {
58
id: InstructionId;
58
- scope: ReactiveScope | null;
59
+ scope: Stack<ReactiveScope>;
60
};
61
61
-type Scopes = Array<ReactiveScope>;
62
-
62
class Context {
63
#declarations: DeclMap = new Map();
64
#reassignments: Map<Identifier, Decl> = new Map();
@@ -80,7 +79,7 @@ class Context {
79
// - accessed by all cfg branches (added through promoteDeps)
80
#depsInCurrentConditional: ReactiveScopeDependencyTree =
81
new ReactiveScopeDependencyTree();
83
- #scopes: Scopes = [];
82
+ #scopes: Stack<ReactiveScope> = empty();
83
84
enter(scope: ReactiveScope, fn: () => void): Set<ReactiveScopeDependency> {
85
// Save context of previous scope
@@ -94,12 +93,12 @@ class Context {
93
const scopedDependencies = new ReactiveScopeDependencyTree();
94
this.#inConditionalWithinScope = false;
95
this.#dependencies = scopedDependencies;
97
- this.#scopes.push(scope);
96
+ this.#scopes = this.#scopes.push(scope);
97
98
fn();
99
100
// Restore context of previous scope
102
- this.#scopes.pop();
101
+ this.#scopes = this.#scopes.pop();
102
this.#dependencies = previousDependencies;
103
this.#inConditionalWithinScope = prevInConditional;
104
@@ -246,22 +245,25 @@ class Context {
245
const currentDeclaration =
246
this.#reassignments.get(identifier) ??
247
this.#declarations.get(identifier.id);
249
- const currentScope = this.currentScope;
248
+ const currentScope = this.#scopes !== null ? this.#scopes.value : null;
249
return (
250
currentScope != null &&
251
currentDeclaration !== undefined &&
252
currentDeclaration.id < currentScope.range.start &&
253
(currentDeclaration.scope == null ||
255
- currentDeclaration.scope !== currentScope)
254
+ currentDeclaration.scope.value !== currentScope)
255
);
256
}
257
258
#isScopeActive(scope: ReactiveScope): boolean {
260
- return this.#scopes.indexOf(scope) !== -1;
259
+ if (this.#scopes === null) {
260
+ return false;
261
+ }
262
+ return this.#scopes.contains(scope);
263
}
264
263
- get currentScope(): ReactiveScope | null {
264
- return this.#scopes.at(-1) ?? null;
265
+ get currentScope(): Stack<ReactiveScope> {
266
+ return this.#scopes;
267
}
268
269
visitOperand(place: Place): void {
@@ -300,15 +302,15 @@ class Context {
302
const originalDeclaration = this.#declarations.get(
303
maybeDependency.identifier.id
304
);
303
- if (
304
- originalDeclaration !== undefined &&
305
- originalDeclaration.scope !== null &&
306
- !this.#isScopeActive(originalDeclaration.scope)
307
- ) {
308
- originalDeclaration.scope.declarations.set(
309
- maybeDependency.identifier.id,
310
- maybeDependency.identifier
311
- );
305
+ if (originalDeclaration !== undefined) {
306
+ originalDeclaration.scope.each((scope) => {
307
+ if (!this.#isScopeActive(scope)) {
308
+ scope.declarations.set(
309
+ maybeDependency.identifier.id,
310
+ maybeDependency.identifier
311
+ );
312
+ }
313
+ });
314
}
315
316
if (this.#checkValidDependencyId(maybeDependency.identifier)) {
@@ -326,15 +328,15 @@ class Context {
328
visitReassignment(place: Place): void {
329
const declaration = this.#declarations.get(place.identifier.id);
330
if (
329
- this.currentScope != null &&
331
+ this.currentScope.value != null &&
332
place.identifier.scope != null &&
333
declaration !== undefined &&
332
- declaration.scope !== place.identifier.scope &&
333
- !Array.from(this.currentScope.reassignments).some(
334
+ declaration.scope.value !== place.identifier.scope &&
335
+ !Array.from(this.currentScope.value.reassignments).some(
336
(ident) => ident.id === place.identifier.id
337
)
338
) {
337
- this.currentScope.reassignments.add(place.identifier);
339
+ this.currentScope.value.reassignments.add(place.identifier);
340
}
341
}
342
}
compiler/forget/src/Utils/Stack.ts
new
+79
@@ -0,0 +1,79 @@
1
+/**
2
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ */
7
+
8
+export interface Stack<T> {
9
+ push(value: T): Stack<T>;
10
+
11
+ pop(): Stack<T>;
12
+
13
+ contains(value: T): boolean;
14
+
15
+ each(fn: (value: T) => void): void;
16
+
17
+ get value(): T | null;
18
+}
19
+
20
+export function create<T>(value: T): Stack<T> {
21
+ return new Node(value);
22
+}
23
+
24
+export function empty<T>(): Stack<T> {
25
+ return EMPTY as any;
26
+}
27
+
28
+class Node<T> implements Stack<T> {
29
+ #value: T;
30
+ #next: Stack<T>;
31
+
32
+ constructor(value: T, next: Stack<T> = EMPTY as any) {
33
+ this.#value = value;
34
+ this.#next = next;
35
+ }
36
+
37
+ push(value: T): Node<T> {
38
+ return new Node(value, this);
39
+ }
40
+
41
+ pop(): Stack<T> {
42
+ return this.#next;
43
+ }
44
+
45
+ contains(value: T): boolean {
46
+ return (
47
+ value === this.#value ||
48
+ (this.#next !== null && this.#next.contains(value))
49
+ );
50
+ }
51
+ each(fn: (value: T) => void): void {
52
+ fn(this.#value);
53
+ this.#next.each(fn);
54
+ }
55
+
56
+ get value(): T {
57
+ return this.#value;
58
+ }
59
+}
60
+
61
+class Empty<T> implements Stack<T> {
62
+ push(value: T): Stack<T> {
63
+ return new Node(value, this);
64
+ }
65
+ pop(): Stack<T> {
66
+ return this;
67
+ }
68
+ contains(_value: T): boolean {
69
+ return false;
70
+ }
71
+ each(_fn: (value: T) => void): void {
72
+ return;
73
+ }
74
+ get value(): T | null {
75
+ return null;
76
+ }
77
+}
78
+
79
+const EMPTY: Stack<void> = new Empty();
compiler/forget/src/__tests__/fixtures/compiler/dependencies.expect.md
+27
-16
@@ -25,25 +25,36 @@ function foo(x, y, z) {
25
```javascript
26
import * as React from "react";
27
function foo(x, y, z) {
28
- const $ = React.unstable_useMemoCache(3);
29
- const items = [z];
30
- items.push(x);
31
- const c_0 = $[0] !== x;
32
- const c_1 = $[1] !== y;
28
+ const $ = React.unstable_useMemoCache(7);
29
+ const c_0 = $[0] !== z;
30
+ const c_1 = $[1] !== x;
31
+ const c_2 = $[2] !== y;
32
let items2;
34
- if (c_0 || c_1) {
35
- items2 = [];
36
- if (x) {
37
- items2.push(y);
33
+ if (c_0 || c_1 || c_2) {
34
+ const items = [z];
35
+ items.push(x);
36
+ const c_4 = $[4] !== x;
37
+ const c_5 = $[5] !== y;
38
+ if (c_4 || c_5) {
39
+ items2 = [];
40
+ if (x) {
41
+ items2.push(y);
42
+ }
43
+ $[4] = x;
44
+ $[5] = y;
45
+ $[6] = items2;
46
+ } else {
47
+ items2 = $[6];
48
+ }
49
+ if (y) {
50
+ items.push(x);
51
}
39
- $[0] = x;
40
- $[1] = y;
41
- $[2] = items2;
52
+ $[0] = z;
53
+ $[1] = x;
54
+ $[2] = y;
55
+ $[3] = items2;
56
} else {
43
- items2 = $[2];
44
- }
45
- if (y) {
46
- items.push(x);
57
+ items2 = $[3];
58
}
59
return items2;
60
}
compiler/forget/src/__tests__/fixtures/compiler/inner-memo-value-not-promoted-to-outer-scope-dynamic.expect.md
renamed
+42
-38
@@ -24,7 +24,7 @@ function Component(props) {
24
```javascript
25
import * as React from "react";
26
function Component(props) {
27
- const $ = React.unstable_useMemoCache(21);
27
+ const $ = React.unstable_useMemoCache(23);
28
const item = useFragment(FRAGMENT, props.item);
29
useFreeze(item);
30
const c_0 = $[0] !== item;
@@ -32,6 +32,7 @@ function Component(props) {
32
let t2;
33
let t3;
34
let t4;
35
+ let t0;
36
let t5;
37
let t6;
38
let t7;
@@ -42,12 +43,11 @@ function Component(props) {
43
t7 = "\n ";
44
t3 = View;
45
t4 = "\n ";
45
- let t0;
46
- if ($[8] === Symbol.for("react.memo_cache_sentinel")) {
46
+ if ($[9] === Symbol.for("react.memo_cache_sentinel")) {
47
t0 = <span>Text</span>;
48
- $[8] = t0;
48
+ $[9] = t0;
49
} else {
50
- t0 = $[8];
50
+ t0 = $[9];
51
}
52
t5 = "\n ";
53
t1 = "span";
@@ -57,35 +57,38 @@ function Component(props) {
57
$[2] = t2;
58
$[3] = t3;
59
$[4] = t4;
60
- $[5] = t5;
61
- $[6] = t6;
62
- $[7] = t7;
60
+ $[5] = t0;
61
+ $[6] = t5;
62
+ $[7] = t6;
63
+ $[8] = t7;
64
} else {
65
t1 = $[1];
66
t2 = $[2];
67
t3 = $[3];
68
t4 = $[4];
68
- t5 = $[5];
69
- t6 = $[6];
70
- t7 = $[7];
69
+ t0 = $[5];
70
+ t5 = $[6];
71
+ t6 = $[7];
72
+ t7 = $[8];
73
}
72
- const c_9 = $[9] !== t1;
73
- const c_10 = $[10] !== t2;
74
+ const c_10 = $[10] !== t1;
75
+ const c_11 = $[11] !== t2;
76
let t8;
75
- if (c_9 || c_10) {
77
+ if (c_10 || c_11) {
78
t8 = <t1>{t2}</t1>;
77
- $[9] = t1;
78
- $[10] = t2;
79
- $[11] = t8;
79
+ $[10] = t1;
80
+ $[11] = t2;
81
+ $[12] = t8;
82
} else {
81
- t8 = $[11];
83
+ t8 = $[12];
84
}
83
- const c_12 = $[12] !== t3;
84
- const c_13 = $[13] !== t4;
85
- const c_14 = $[14] !== t5;
86
- const c_15 = $[15] !== t8;
85
+ const c_13 = $[13] !== t3;
86
+ const c_14 = $[14] !== t4;
87
+ const c_15 = $[15] !== t0;
88
+ const c_16 = $[16] !== t5;
89
+ const c_17 = $[17] !== t8;
90
let t9;
88
- if (c_12 || c_13 || c_14 || c_15) {
91
+ if (c_13 || c_14 || c_15 || c_16 || c_17) {
92
t9 = (
93
<t3>
94
{t4}
@@ -94,31 +97,32 @@ function Component(props) {
97
{t8}
98
</t3>
99
);
97
- $[12] = t3;
98
- $[13] = t4;
99
- $[14] = t5;
100
- $[15] = t8;
101
- $[16] = t9;
100
+ $[13] = t3;
101
+ $[14] = t4;
102
+ $[15] = t0;
103
+ $[16] = t5;
104
+ $[17] = t8;
105
+ $[18] = t9;
106
} else {
103
- t9 = $[16];
107
+ t9 = $[18];
108
}
105
- const c_17 = $[17] !== t6;
106
- const c_18 = $[18] !== t7;
107
- const c_19 = $[19] !== t9;
109
+ const c_19 = $[19] !== t6;
110
+ const c_20 = $[20] !== t7;
111
+ const c_21 = $[21] !== t9;
112
let t10;
109
- if (c_17 || c_18 || c_19) {
113
+ if (c_19 || c_20 || c_21) {
114
t10 = (
115
<t6>
116
{t7}
117
{t9}
118
</t6>
119
);
116
- $[17] = t6;
117
- $[18] = t7;
118
- $[19] = t9;
119
- $[20] = t10;
120
+ $[19] = t6;
121
+ $[20] = t7;
122
+ $[21] = t9;
123
+ $[22] = t10;
124
} else {
121
- t10 = $[20];
125
+ t10 = $[22];
126
}
127
return t10;
128
}
compiler/forget/src/__tests__/fixtures/compiler/inner-memo-value-not-promoted-to-outer-scope-dynamic.js
renamed
compiler/forget/src/__tests__/fixtures/compiler/inner-memo-value-not-promoted-to-outer-scope-static.expect.md
renamed
+22
-20
@@ -21,11 +21,12 @@ function Component(props) {
21
```javascript
22
import * as React from "react";
23
function Component(props) {
24
- const $ = React.unstable_useMemoCache(11);
24
+ const $ = React.unstable_useMemoCache(12);
25
let t1;
26
let t2;
27
let t3;
28
let t4;
29
+ let t0;
30
let t5;
31
let t6;
32
let t7;
@@ -36,12 +37,11 @@ function Component(props) {
37
t7 = "\n ";
38
t3 = View;
39
t4 = "\n ";
39
- let t0;
40
- if ($[7] === Symbol.for("react.memo_cache_sentinel")) {
40
+ if ($[8] === Symbol.for("react.memo_cache_sentinel")) {
41
t0 = <span>Text</span>;
42
- $[7] = t0;
42
+ $[8] = t0;
43
} else {
44
- t0 = $[7];
44
+ t0 = $[8];
45
}
46
t5 = "\n ";
47
t1 = "span";
@@ -50,27 +50,29 @@ function Component(props) {
50
$[1] = t2;
51
$[2] = t3;
52
$[3] = t4;
53
- $[4] = t5;
54
- $[5] = t6;
55
- $[6] = t7;
53
+ $[4] = t0;
54
+ $[5] = t5;
55
+ $[6] = t6;
56
+ $[7] = t7;
57
} else {
58
t1 = $[0];
59
t2 = $[1];
60
t3 = $[2];
61
t4 = $[3];
61
- t5 = $[4];
62
- t6 = $[5];
63
- t7 = $[6];
62
+ t0 = $[4];
63
+ t5 = $[5];
64
+ t6 = $[6];
65
+ t7 = $[7];
66
}
67
let t8;
66
- if ($[8] === Symbol.for("react.memo_cache_sentinel")) {
68
+ if ($[9] === Symbol.for("react.memo_cache_sentinel")) {
69
t8 = <t1>{t2}</t1>;
68
- $[8] = t8;
70
+ $[9] = t8;
71
} else {
70
- t8 = $[8];
72
+ t8 = $[9];
73
}
74
let t9;
73
- if ($[9] === Symbol.for("react.memo_cache_sentinel")) {
75
+ if ($[10] === Symbol.for("react.memo_cache_sentinel")) {
76
t9 = (
77
<t3>
78
{t4}
@@ -79,21 +81,21 @@ function Component(props) {
81
{t8}
82
</t3>
83
);
82
- $[9] = t9;
84
+ $[10] = t9;
85
} else {
84
- t9 = $[9];
86
+ t9 = $[10];
87
}
88
let t10;
87
- if ($[10] === Symbol.for("react.memo_cache_sentinel")) {
89
+ if ($[11] === Symbol.for("react.memo_cache_sentinel")) {
90
t10 = (
91
<t6>
92
{t7}
93
{t9}
94
</t6>
95
);
94
- $[10] = t10;
96
+ $[11] = t10;
97
} else {
96
- t10 = $[10];
98
+ t10 = $[11];
99
}
100
return t10;
101
}
compiler/forget/src/__tests__/fixtures/compiler/inner-memo-value-not-promoted-to-outer-scope-static.js
renamed
compiler/forget/src/__tests__/fixtures/compiler/reduce-reactive-deps-join-uncond-scopes-cond-deps.expect.md
+18
-16
@@ -52,40 +52,42 @@ import * as React from "react"; // This tests an optimization, NOT a correctness
52
// }
53
54
function TestJoinCondDepsInUncondScopes(props) {
55
- const $ = React.unstable_useMemoCache(7);
55
+ const $ = React.unstable_useMemoCache(8);
56
const c_0 = $[0] !== props.a.b;
57
+ let x;
58
let y;
59
if (c_0) {
60
y = {};
60
- const c_2 = $[2] !== props;
61
- let x;
62
- if (c_2) {
61
+ const c_3 = $[3] !== props;
62
+ if (c_3) {
63
x = {};
64
if (foo) {
65
mutate1(x, props.a.b);
66
}
67
- $[2] = props;
68
- $[3] = x;
67
+ $[3] = props;
68
+ $[4] = x;
69
} else {
70
- x = $[3];
70
+ x = $[4];
71
}
72
73
mutate2(y, props.a.b);
74
$[0] = props.a.b;
75
- $[1] = y;
75
+ $[1] = x;
76
+ $[2] = y;
77
} else {
77
- y = $[1];
78
+ x = $[1];
79
+ y = $[2];
80
}
79
- const c_4 = $[4] !== x;
80
- const c_5 = $[5] !== y;
81
+ const c_5 = $[5] !== x;
82
+ const c_6 = $[6] !== y;
83
let t0;
82
- if (c_4 || c_5) {
84
+ if (c_5 || c_6) {
85
t0 = [x, y];
84
- $[4] = x;
85
- $[5] = y;
86
- $[6] = t0;
86
+ $[5] = x;
87
+ $[6] = y;
88
+ $[7] = t0;
89
} else {
88
- t0 = $[6];
90
+ t0 = $[7];
91
}
92
return t0;
93
}