@samitouri / QOS-React-2 / commits / a9868cc2b1

Fix memoization of lambdas in @enableOptimizeFunctionExpr feature

This is a fix specifically for the `enableOptimizeFunctionExpression` feature (disabled by default). I tried running all of our fixtures with that flag on everywhere, and this was the only issue. It's actually extracted from another fixture which is more complicated, this is a distilled version. There were two bugs: * DCE was running after LeaveSSA when it needs to run before. Fixing the order means code inside the function expression stops getting removed. * In the feature flag, context variables share Identifier instances with the surrounding code, whereas they are distinct instances without the feature. This meant that mutable ranges from inside the function propagated outside the function, throwing off our inference. The fix was to reset the ranges of context variables after inferring the function expression's effects.

Joe Savona committed Jun 16, 2023 at 14:42 UTC a9868cc2b1c19066e9d93bbc404fdca02a314651
4 files changed +63 -3
compiler/forget/packages/babel-plugin-react-forget/src/Inference/AnalyseFunctions.ts
+3 -1
@@ -17,7 +17,7 @@ import {
17 Place,
18 ReactiveScopeDependency,
19 } from "../HIR";
20 -import { constantPropagation } from "../Optimization";
20 +import { constantPropagation, deadCodeElimination } from "../Optimization";
21 import { inferReactiveScopeVariables } from "../ReactiveScopes";
22 import { eliminateRedundantPhi, enterSSA, leaveSSA } from "../SSA";
23 import { inferTypes } from "../TypeInference";
@@ -113,6 +113,7 @@ function lower(func: HIRFunction): void {
113
114 analyseFunctions(func);
115 inferReferenceEffects(func, { isFunctionExpression: true });
116 + deadCodeElimination(func);
117 inferMutableRanges(func);
118 leaveSSA(func);
119 inferReactiveScopeVariables(func);
@@ -133,6 +134,7 @@ function infer(
134 ) {
135 mutations.set(operand.identifier.name, operand.effect);
136 }
137 + operand.identifier.mutableRange.end = operand.identifier.mutableRange.start;
138 }
139
140 for (const dep of value.dependencies) {
compiler/forget/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts
-2
@@ -31,7 +31,6 @@ import {
31 } from "../HIR/HIR";
32 import { printPlace } from "../HIR/PrintHIR";
33 import { eachPatternOperand } from "../HIR/visitors";
34 -import { deadCodeElimination } from "../Optimization";
34 import { Err, Ok, Result } from "../Utils/Result";
35 import { assertExhaustive } from "../Utils/utils";
36 import { buildReactiveFunction } from "./BuildReactiveFunction";
@@ -962,7 +961,6 @@ function codegenInstructionValue(
961 case "FunctionExpression": {
962 if (cx.env.enableOptimizeFunctionExpressions) {
963 const loweredFunc = instrValue.loweredFunc;
965 - deadCodeElimination(loweredFunc);
964 const reactiveFunction = buildReactiveFunction(loweredFunc);
965 pruneUnusedLabels(reactiveFunction);
966 pruneUnusedLValues(reactiveFunction);
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reassigned-phi-in-returned-function-expression.expect.md new
+48
@@ -0,0 +1,48 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @enableOptimizeFunctionExpressions
6 +function Component(props) {
7 + return () => {
8 + let str;
9 + if (arguments.length) {
10 + str = arguments[0];
11 + } else {
12 + str = props.str;
13 + }
14 + global.log(str);
15 + };
16 +}
17 +
18 +```
19 +
20 +## Code
21 +
22 +```javascript
23 +import { unstable_useMemoCache as useMemoCache } from "react"; // @enableOptimizeFunctionExpressions
24 +function Component(props) {
25 + const $ = useMemoCache(2);
26 + const c_0 = $[0] !== props.str;
27 + let t0;
28 + if (c_0) {
29 + t0 = () => {
30 + let str = undefined;
31 + if (arguments.length) {
32 + str = arguments[0];
33 + } else {
34 + str = props.str;
35 + }
36 +
37 + global.log(str);
38 + };
39 + $[0] = props.str;
40 + $[1] = t0;
41 + } else {
42 + t0 = $[1];
43 + }
44 + return t0;
45 +}
46 +
47 +```
48 +
\ No newline at end of file
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reassigned-phi-in-returned-function-expression.js new
+12
@@ -0,0 +1,12 @@
1 +// @enableOptimizeFunctionExpressions
2 +function Component(props) {
3 + return () => {
4 + let str;
5 + if (arguments.length) {
6 + str = arguments[0];
7 + } else {
8 + str = props.str;
9 + }
10 + global.log(str);
11 + };
12 +}