@samitouri / QOS-React / commits / 1fdbcfe162

[rhir] Add dependencies produced by active (incomplete) scopes

--- > If this operand is used in a scope, has a dynamic value, and was defined before this scope, then its a dependency of the scope. > (from current comments in PropagateScopeDependencies::visitDependency) A reactive scope can take a dependency from a definition produced by an incomplete parent scope. Our tests previously did not cover this, since most object types aliased together and remained mutable throughout a ReactiveScope. e.g. our tests did not have ``` scope @0 (deps=..., declarations=[x, y]) { x = {}; // define a reactive, immutable value that is not aliased to become mutable const immutableVal = ...; scope @1 (deps=immutableVal, declarations=[y]) { y = read(immutableVal) } mutateX(x, ...); } ``` We should not add a dependency if it is produced in exactly the same scope as the one it is used. It is safe (and correct) to depend on values produced by a parent scope. --- Note that we still should check for whether a defining scope is active to determine whether it should be added as a output of that scope ([src](https://github.com/facebook/react-forget/blob/b608ab20d57229b528deeffa19f1ee08a4bad37a/forget/src/ReactiveScopes/PropagateScopeDependencies.ts#L469-L478)). Access of an identifier produced by a parent scope (i.e. adding a variable defined by a scope's parent as its own dependency) does not require adding that identifier to the parent's `declarations`, since that identifier is already valid to access via identifier binding rules.

Mofei Zhang committed Feb 28, 2023 at 16:36 UTC 1fdbcfe162c0719d8156f40f3e71a8fc005f9b7b
3 files changed +37 -27
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+1 -1
@@ -505,7 +505,7 @@ class Context {
505 currentDeclaration !== undefined &&
506 currentDeclaration.id < currentScope.range.start &&
507 (currentDeclaration.scope == null ||
508 - !this.#isScopeActive(currentDeclaration.scope))
508 + currentDeclaration.scope !== currentScope)
509 ) {
510 // Check if there is an existing dependency that describes this operand
511 // We do not try to join/reduce dependencies here due to missing info
compiler/forget/src/__tests__/fixtures/hir/allocating-primitive-as-dep.expect.md
+18 -13
@@ -53,7 +53,7 @@ function AllocatingPrimitiveAsDep(props) {
53 }
54
55 function PrimitiveAsDepNested(props) {
56 - const $ = React.unstable_useMemoCache(8);
56 + const $ = React.unstable_useMemoCache(10);
57 const c_0 = $[0] !== props.b;
58 const c_1 = $[1] !== props.a;
59 let x;
@@ -69,12 +69,15 @@ function PrimitiveAsDepNested(props) {
69 } else {
70 t0 = $[4];
71 }
72 + const t1 = t0 + 1;
73 + const c_5 = $[5] !== t1;
74 let y;
73 - if ($[5] === Symbol.for("react.memo_cache_sentinel")) {
74 - y = foo(t0 + 1);
75 - $[5] = y;
75 + if (c_5) {
76 + y = foo(t1);
77 + $[5] = t1;
78 + $[6] = y;
79 } else {
77 - y = $[5];
80 + y = $[6];
81 }
82 mutate(x, props.a);
83 $[0] = props.b;
@@ -83,16 +86,18 @@ function PrimitiveAsDepNested(props) {
86 } else {
87 x = $[2];
88 }
86 - const c_6 = $[6] !== x;
87 - let t1;
88 - if (c_6) {
89 - t1 = [x, y];
90 - $[6] = x;
91 - $[7] = t1;
89 + const c_7 = $[7] !== x;
90 + const c_8 = $[8] !== y;
91 + let t2;
92 + if (c_7 || c_8) {
93 + t2 = [x, y];
94 + $[7] = x;
95 + $[8] = y;
96 + $[9] = t2;
97 } else {
93 - t1 = $[7];
98 + t2 = $[9];
99 }
95 - return t1;
100 + return t2;
101 }
102
103 ```
compiler/forget/src/__tests__/fixtures/hir/primitive-as-dep.expect.md
+18 -13
@@ -46,19 +46,22 @@ function PrimitiveAsDep(props) {
46 }
47
48 function PrimitiveAsDepNested(props) {
49 - const $ = React.unstable_useMemoCache(6);
49 + const $ = React.unstable_useMemoCache(8);
50 const c_0 = $[0] !== props.b;
51 const c_1 = $[1] !== props.a;
52 let x;
53 if (c_0 || c_1) {
54 x = {};
55 mutate(x);
56 + const t0 = props.b + 1;
57 + const c_3 = $[3] !== t0;
58 let y;
57 - if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
58 - y = foo(props.b + 1);
59 - $[3] = y;
59 + if (c_3) {
60 + y = foo(t0);
61 + $[3] = t0;
62 + $[4] = y;
63 } else {
61 - y = $[3];
64 + y = $[4];
65 }
66 mutate(x, props.a);
67 $[0] = props.b;
@@ -67,16 +70,18 @@ function PrimitiveAsDepNested(props) {
70 } else {
71 x = $[2];
72 }
70 - const c_4 = $[4] !== x;
71 - let t0;
72 - if (c_4) {
73 - t0 = [x, y];
74 - $[4] = x;
75 - $[5] = t0;
73 + const c_5 = $[5] !== x;
74 + const c_6 = $[6] !== y;
75 + let t1;
76 + if (c_5 || c_6) {
77 + t1 = [x, y];
78 + $[5] = x;
79 + $[6] = y;
80 + $[7] = t1;
81 } else {
77 - t0 = $[5];
82 + t1 = $[7];
83 }
79 - return t0;
84 + return t1;
85 }
86
87 ```