From e2836eec5be07acf5a0cf7af3a230f2526d26031 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 30 Oct 2023 12:56:50 -0400 Subject: [PATCH] [bugfix] do not hoist computed memberpaths in lambdas --- .../src/HIR/BuildHIR.ts | 51 +++++++++++------- ...-lambda-array-access-member-expr.expect.md | 30 ----------- ...rray-access-member-expr-captured.expect.md | 52 +++++++++++++++++++ ...ambda-array-access-member-expr-captured.ts | 16 ++++++ ...a-array-access-member-expr-param.expect.md | 50 ++++++++++++++++++ ... lambda-array-access-member-expr-param.ts} | 2 +- .../packages/sprout/src/runner-evaluator.ts | 4 +- 7 files changed, 154 insertions(+), 51 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bug-lambda-array-access-member-expr.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-captured.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-captured.ts create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-param.expect.md rename compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/{error.bug-lambda-array-access-member-expr.ts => lambda-array-access-member-expr-param.ts} (87%) diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts index de4ea10e33..0404b02d03 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts @@ -3530,6 +3530,14 @@ function lowerAssignment( } } +function isValidDependency(path: NodePath): boolean { + const parent: NodePath = path.parentPath; + return ( + !path.node.computed && + !(parent.isCallExpression() && parent.get("callee") === path) + ); +} + function captureScopes({ from, to }: { from: Scope; to: Scope }): Set { let scopes: Set = new Set(); while (from) { @@ -3579,7 +3587,7 @@ function gatherCapturedDeps( | NodePath ): void { // Base context variable to depend on - let baseIdentifier: NodePath; + let baseIdentifier: NodePath | NodePath; // Base expression to depend on, which (for now) may contain non side-effectful // member expressions let dependency: @@ -3604,31 +3612,36 @@ function gatherCapturedDeps( dependency = current; } else if (path.isMemberExpression()) { // Calculate baseIdentifier - let current: NodePath = path; - while (current.isMemberExpression()) { - current = current.get("object"); + let currentId: NodePath = path; + while (currentId.isMemberExpression()) { + currentId = currentId.get("object"); } - if (!current.isIdentifier()) { + if (!currentId.isIdentifier()) { return; } - baseIdentifier = current; + baseIdentifier = currentId; // Get the expression to depend on, which may involve PropertyLoads // for member expressions - current = - path.parent.type === "CallExpression" && - path.parent.callee === path.node - ? path.get("object") - : path; - while (current.isMemberExpression() && current.node.computed) { - // computed nodes may contain side-effectful subexpressions - current = current.get("object"); + let currentDep: + | NodePath + | NodePath + | NodePath = baseIdentifier; + + while (true) { + const nextDep: null | NodePath = currentDep.parentPath; + if ( + nextDep && + nextDep.isMemberExpression() && + isValidDependency(nextDep) + ) { + currentDep = nextDep; + } else { + break; + } } - invariant( - current.isMemberExpression() || current.isIdentifier(), - "Internal invariant broken in BuildHIR, unexpected type for capturedDep" - ); - dependency = current; + + dependency = currentDep; } else { baseIdentifier = path; dependency = path; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bug-lambda-array-access-member-expr.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bug-lambda-array-access-member-expr.expect.md deleted file mode 100644 index b817aa24fd..0000000000 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bug-lambda-array-access-member-expr.expect.md +++ /dev/null @@ -1,30 +0,0 @@ - -## Input - -```javascript -import { invoke } from "shared-runtime"; - -function Foo() { - const x = [{ value: 0 }, { value: 1 }, { value: 2 }]; - const foo = (param: number) => { - return x[param].value; - }; - - return invoke(foo, 1); -} - -export const FIXTURE_ENTRYPONT = { - fn: Foo, - params: [{}], -}; - -``` - - -## Error - -``` -[ReactForget] Todo: EnterSSA: Expected identifier to be defined before being used. Identifier param$10 is undefined (5:5) -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-captured.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-captured.expect.md new file mode 100644 index 0000000000..dab5e5259b --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-captured.expect.md @@ -0,0 +1,52 @@ + +## Input + +```javascript +import { CONST_NUMBER0, invoke } from "shared-runtime"; + +function Foo() { + const x = [{ value: 0 }, { value: 1 }, { value: 2 }]; + const param = CONST_NUMBER0; + const foo = () => { + return x[param].value; + }; + + return invoke(foo); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{}], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +import { CONST_NUMBER0, invoke } from "shared-runtime"; + +function Foo() { + const $ = useMemoCache(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const x = [{ value: 0 }, { value: 1 }, { value: 2 }]; + + const foo = () => x[CONST_NUMBER0].value; + + t0 = invoke(foo); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{}], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-captured.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-captured.ts new file mode 100644 index 0000000000..0bb74fab1f --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-captured.ts @@ -0,0 +1,16 @@ +import { CONST_NUMBER0, invoke } from "shared-runtime"; + +function Foo() { + const x = [{ value: 0 }, { value: 1 }, { value: 2 }]; + const param = CONST_NUMBER0; + const foo = () => { + return x[param].value; + }; + + return invoke(foo); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-param.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-param.expect.md new file mode 100644 index 0000000000..d8ff7c8932 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-param.expect.md @@ -0,0 +1,50 @@ + +## Input + +```javascript +import { invoke } from "shared-runtime"; + +function Foo() { + const x = [{ value: 0 }, { value: 1 }, { value: 2 }]; + const foo = (param: number) => { + return x[param].value; + }; + + return invoke(foo, 1); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{}], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +import { invoke } from "shared-runtime"; + +function Foo() { + const $ = useMemoCache(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const x = [{ value: 0 }, { value: 1 }, { value: 2 }]; + const foo = (param) => x[param].value; + + t0 = invoke(foo, 1); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{}], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bug-lambda-array-access-member-expr.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-param.ts similarity index 87% rename from compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bug-lambda-array-access-member-expr.ts rename to compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-param.ts index 0fc1dfa11e..bf28403983 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bug-lambda-array-access-member-expr.ts +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-array-access-member-expr-param.ts @@ -9,7 +9,7 @@ function Foo() { return invoke(foo, 1); } -export const FIXTURE_ENTRYPONT = { +export const FIXTURE_ENTRYPOINT = { fn: Foo, params: [{}], }; diff --git a/compiler/packages/sprout/src/runner-evaluator.ts b/compiler/packages/sprout/src/runner-evaluator.ts index cd40d54665..6ebf1c1f53 100644 --- a/compiler/packages/sprout/src/runner-evaluator.ts +++ b/compiler/packages/sprout/src/runner-evaluator.ts @@ -99,7 +99,9 @@ export function doEval(source: string): EvaluatorResult { ) { return { kind: "UnexpectedError", - value: 'FIXTURE_ENTRYPOINT not exported!', + value: 'FIXTURE_ENTRYPOINT not exported! Found {' + + Object.keys(exports).filter(e => e !== 'FIXTURE_ENTRYPOINT').toString() + + '}', }; } const validationError = validateEntrypoint(exports.FIXTURE_ENTRYPOINT);