From 474c38c573e4a56ff3f8eb31bb22c9c65895118e Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Tue, 28 Feb 2023 16:35:59 -0500 Subject: [PATCH] [rhir][tests] Added tests for primitives as dependencies --- Our current compiler has specific logic for determining what can be a reactive value / reactive dependency. Currently, all of the following affect whether an identifier is a reactive: - **alias analysis** (applicable to objects) - **data + control flow** (whether any other reactive identifiers is used in determining it) - **reactive scopes** (we generalize and say anything produced by a block with reactive dependencies must be non-stable and reactive) - this is not true in the case of const primitives, but an overestimate is safe - whether the **scope that declares this identifier** is ~~currently active~~ the same scope in which it is used (fixed by #1275) (since a scope cannot be dependent on itself) These conditions are complex. We end up inferring most identifiers as `mutable` and `object` types, which have different stability and aliasing properties from primitives. As a result, we're missing some cases in our existing test coverage. Test case output is fixed by #1274 and #1275 --- (This can be separated from the stack below, which implements conditional dependencies. Happy to merge that first and open this as a new stack if that produces a significantly better Git PR history.) --- .../hir/allocating-primitive-as-dep.expect.md | 99 +++++++++++++++++++ .../hir/allocating-primitive-as-dep.js | 16 +++ .../fixtures/hir/primitive-as-dep.expect.md | 83 ++++++++++++++++ .../fixtures/hir/primitive-as-dep.js | 17 ++++ 4 files changed, 215 insertions(+) create mode 100644 compiler/forget/src/__tests__/fixtures/hir/allocating-primitive-as-dep.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/allocating-primitive-as-dep.js create mode 100644 compiler/forget/src/__tests__/fixtures/hir/primitive-as-dep.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/primitive-as-dep.js diff --git a/compiler/forget/src/__tests__/fixtures/hir/allocating-primitive-as-dep.expect.md b/compiler/forget/src/__tests__/fixtures/hir/allocating-primitive-as-dep.expect.md new file mode 100644 index 0000000000..d50063894e --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/allocating-primitive-as-dep.expect.md @@ -0,0 +1,99 @@ + +## Input + +```javascript +// bar(props.b) is an allocating expression that produces a primitive, which means +// that Forget should memoize it. +// Correctness: +// - y depends on either bar(props.b) or bar(props.b) + 1 +function AllocatingPrimitiveAsDep(props) { + let y = foo(bar(props).b + 1); + return y; +} + +function PrimitiveAsDepNested(props) { + let x = {}; + mutate(x); + let y = foo(bar(props.b) + 1); + mutate(x, props.a); + return [x, y]; +} + +``` + +## Code + +```javascript +// bar(props.b) is an allocating expression that produces a primitive, which means +// that Forget should memoize it. +// Correctness: +// - y depends on either bar(props.b) or bar(props.b) + 1 +function AllocatingPrimitiveAsDep(props) { + const $ = React.unstable_useMemoCache(4); + const c_0 = $[0] !== props; + let t0; + if (c_0) { + t0 = bar(props); + $[0] = props; + $[1] = t0; + } else { + t0 = $[1]; + } + const t1 = t0.b + 1; + const c_2 = $[2] !== t1; + let y; + if (c_2) { + y = foo(t1); + $[2] = t1; + $[3] = y; + } else { + y = $[3]; + } + return y; +} + +function PrimitiveAsDepNested(props) { + const $ = React.unstable_useMemoCache(8); + const c_0 = $[0] !== props.b; + const c_1 = $[1] !== props.a; + let x; + if (c_0 || c_1) { + x = {}; + mutate(x); + const c_3 = $[3] !== props.b; + let t0; + if (c_3) { + t0 = bar(props.b); + $[3] = props.b; + $[4] = t0; + } else { + t0 = $[4]; + } + let y; + if ($[5] === Symbol.for("react.memo_cache_sentinel")) { + y = foo(t0 + 1); + $[5] = y; + } else { + y = $[5]; + } + mutate(x, props.a); + $[0] = props.b; + $[1] = props.a; + $[2] = x; + } else { + x = $[2]; + } + const c_6 = $[6] !== x; + let t1; + if (c_6) { + t1 = [x, y]; + $[6] = x; + $[7] = t1; + } else { + t1 = $[7]; + } + return t1; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/allocating-primitive-as-dep.js b/compiler/forget/src/__tests__/fixtures/hir/allocating-primitive-as-dep.js new file mode 100644 index 0000000000..ba0095d7f3 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/allocating-primitive-as-dep.js @@ -0,0 +1,16 @@ +// bar(props.b) is an allocating expression that produces a primitive, which means +// that Forget should memoize it. +// Correctness: +// - y depends on either bar(props.b) or bar(props.b) + 1 +function AllocatingPrimitiveAsDep(props) { + let y = foo(bar(props).b + 1); + return y; +} + +function PrimitiveAsDepNested(props) { + let x = {}; + mutate(x); + let y = foo(bar(props.b) + 1); + mutate(x, props.a); + return [x, y]; +} diff --git a/compiler/forget/src/__tests__/fixtures/hir/primitive-as-dep.expect.md b/compiler/forget/src/__tests__/fixtures/hir/primitive-as-dep.expect.md new file mode 100644 index 0000000000..f06413e71b --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/primitive-as-dep.expect.md @@ -0,0 +1,83 @@ + +## Input + +```javascript +// props.b + 1 is an non-allocating expression, which means Forget can +// emit it trivially and repeatedly (e.g. no need to memoize props.b + 1 +// separately from props.b) +// Correctness: +// y depends on either props.b or props.b + 1 +function PrimitiveAsDep(props) { + let y = foo(props.b + 1); + return y; +} + +function PrimitiveAsDepNested(props) { + let x = {}; + mutate(x); + let y = foo(props.b + 1); + mutate(x, props.a); + return [x, y]; +} + +``` + +## Code + +```javascript +// props.b + 1 is an non-allocating expression, which means Forget can +// emit it trivially and repeatedly (e.g. no need to memoize props.b + 1 +// separately from props.b) +// Correctness: +// y depends on either props.b or props.b + 1 +function PrimitiveAsDep(props) { + const $ = React.unstable_useMemoCache(2); + const t0 = props.b + 1; + const c_0 = $[0] !== t0; + let y; + if (c_0) { + y = foo(t0); + $[0] = t0; + $[1] = y; + } else { + y = $[1]; + } + return y; +} + +function PrimitiveAsDepNested(props) { + const $ = React.unstable_useMemoCache(6); + const c_0 = $[0] !== props.b; + const c_1 = $[1] !== props.a; + let x; + if (c_0 || c_1) { + x = {}; + mutate(x); + let y; + if ($[3] === Symbol.for("react.memo_cache_sentinel")) { + y = foo(props.b + 1); + $[3] = y; + } else { + y = $[3]; + } + mutate(x, props.a); + $[0] = props.b; + $[1] = props.a; + $[2] = x; + } else { + x = $[2]; + } + const c_4 = $[4] !== x; + let t0; + if (c_4) { + t0 = [x, y]; + $[4] = x; + $[5] = t0; + } else { + t0 = $[5]; + } + return t0; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/primitive-as-dep.js b/compiler/forget/src/__tests__/fixtures/hir/primitive-as-dep.js new file mode 100644 index 0000000000..ca8b2e987f --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/primitive-as-dep.js @@ -0,0 +1,17 @@ +// props.b + 1 is an non-allocating expression, which means Forget can +// emit it trivially and repeatedly (e.g. no need to memoize props.b + 1 +// separately from props.b) +// Correctness: +// y depends on either props.b or props.b + 1 +function PrimitiveAsDep(props) { + let y = foo(props.b + 1); + return y; +} + +function PrimitiveAsDepNested(props) { + let x = {}; + mutate(x); + let y = foo(props.b + 1); + mutate(x, props.a); + return [x, y]; +}