@samitouri / QOS-React-2 / commits / 5746d5b07f

Temporary workaround for emitting temporaries multiple times

This is a temporary fix for the issue we discovered on our first integration, where destructuring of a function return value is emitting the function call multiple times: ```javascript // Input const [x, setX] = useState(null); // Output const x = useState(null)[0]; const setX = useState(null)[1]; ``` The reason this happens is that we lower `useState(null)` to a temporary, and then generate a ComputedLoad for each of x and setX. Codegen doesn't emit temporaries eagerly - it assumes they are going to be used exactly once and it re-emits the value each time the temporary is used. Hence why the `useState(null)` part gets duplicated in the output. Right now destructuring is the only place i'm aware of where we reuse temporaries this way. And we do want to change codegen to preserve destructuring in the output to correctly handle array patterns. However, that's a more involved change. For now, this PR is a stopgap. During the pass where we promote temporaries used in scopes to named variables, we now check to see if those temporaries are used multiple times and promote them. The above example would then generate something like ```javascript const t0 = useState(null); const x = t0[0]; const setX = t0[1]; ``` This is still incorrect (it assumes t0 is an array), but it's more likely to work in practice. I'll revert this change once we correctly handle destructuring.

Joe Savona committed Feb 9, 2023 at 17:22 UTC 5746d5b07f418c36c7372b863813723c2d25087b
5 files changed +49 -8
compiler/forget/src/HIR/BuildHIR.ts
-2
@@ -1763,8 +1763,6 @@ function buildTemporaryPlace(builder: HIRBuilder, loc: SourceLocation): Place {
1763 return place;
1764 }
1765
1766 -function lowerFooBar(): void {}
1767 -
1766 function lowerAssignment(
1767 builder: HIRBuilder,
1768 loc: SourceLocation,
compiler/forget/src/ReactiveScopes/PromoteUsedTemporaries.ts
+42 -2
@@ -5,10 +5,18 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 -import { ReactiveFunction, ReactiveScopeBlock } from "../HIR/HIR";
8 +import {
9 + Identifier,
10 + InstructionId,
11 + Place,
12 + ReactiveFunction,
13 + ReactiveInstruction,
14 + ReactiveScopeBlock,
15 +} from "../HIR/HIR";
16 import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
17
18 type VisitorState = {
19 + temporaries: Map<Identifier, number>;
20 nextId: number;
21 };
22 class Visitor extends ReactiveFunctionVisitor<VisitorState> {
@@ -31,7 +39,39 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
39 }
40 }
41 }
42 + override visitPlace(
43 + id: InstructionId,
44 + place: Place,
45 + state: VisitorState
46 + ): void {
47 + let count = state.temporaries.get(place.identifier);
48 + if (count !== undefined) {
49 + state.temporaries.set(place.identifier, count + 1);
50 + }
51 + }
52 + override visitInstruction(
53 + instruction: ReactiveInstruction,
54 + state: VisitorState
55 + ): void {
56 + this.traverseInstruction(instruction, state);
57 + if (
58 + instruction.lvalue !== null &&
59 + instruction.lvalue.place.identifier.name === null &&
60 + instruction.value.kind !== "Identifier"
61 + ) {
62 + state.temporaries.set(instruction.lvalue.place.identifier, 0);
63 + }
64 + }
65 }
66 export function promoteUsedTemporaries(fn: ReactiveFunction) {
36 - visitReactiveFunction(fn, new Visitor(), { nextId: 0 });
67 + const state: VisitorState = {
68 + nextId: 0,
69 + temporaries: new Map(),
70 + };
71 + visitReactiveFunction(fn, new Visitor(), state);
72 + for (const [identifier, count] of state.temporaries) {
73 + if (count > 1) {
74 + identifier.name = `t${state.nextId++}`;
75 + }
76 + }
77 }
compiler/forget/src/__tests__/fixtures/hir/assignment-expression-nested-path.expect.md
+2 -1
@@ -21,7 +21,8 @@ function g(props) {
21 if (c_0) {
22 a = { b: { c: props.c } };
23 a.b.c = a.b.c + 1;
24 - a.b.c = a.b.c * 2;
24 + const t0 = a.b;
25 + t0.c = t0.c * 2;
26 $[0] = props.c;
27 $[1] = a;
28 } else {
compiler/forget/src/__tests__/fixtures/hir/assignment-variations-complex-lvalue.expect.md
+2 -1
@@ -20,7 +20,8 @@ function g() {
20 if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
21 x = { y: { z: 1 } };
22 x.y.z = x.y.z + 1;
23 - x.y.z = x.y.z * 2;
23 + const t0 = x.y;
24 + t0.z = t0.z * 2;
25 $[0] = x;
26 } else {
27 x = $[0];
compiler/forget/src/__tests__/fixtures/hir/controlled-input.expect.md
+3 -2
@@ -15,8 +15,9 @@ function component() {
15 ```javascript
16 function component() {
17 const $ = React.unstable_useMemoCache();
18 - const x = useState(0)[0];
19 - const setX = useState(0)[1];
18 + const t1 = useState(0);
19 + const x = t1[0];
20 + const setX = t1[1];
21 const c_0 = $[0] !== setX;
22 let handler;
23 if (c_0) {