From 7ecee6a09148e3117c487438e4ef575bc6b569bc Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 3 May 2023 17:10:28 -0700 Subject: [PATCH] Support complex computed properties in OptionalMemberExpression Our previous lowering for OptionalMemberExpression reordered the evaluation of properties, such that we had to restrict the allowed properties to those that were safe for reordering. With the new representation we preserve order of evaluation, so we can relax the restriction. This unblocks a few cases in an internal product. --- compiler/forget/src/HIR/BuildHIR.ts | 11 +---- ...ional-computed-member-expression.expect.md | 25 ----------- ...ional-computed-member-expression.expect.md | 43 +++++++++++++++++++ ...=> optional-computed-member-expression.js} | 0 ...mber-expression-call-as-property.expect.md | 41 ++++++++++++++++++ ...onal-member-expression-call-as-property.js | 4 ++ ...optional-member-expr-as-property.expect.md | 41 ++++++++++++++++++ ...n-with-optional-member-expr-as-property.js | 4 ++ 8 files changed, 134 insertions(+), 35 deletions(-) delete mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.optional-computed-member-expression.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/optional-computed-member-expression.expect.md rename compiler/forget/src/__tests__/fixtures/compiler/{error.optional-computed-member-expression.js => optional-computed-member-expression.js} (100%) create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-call-as-property.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-call-as-property.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-with-optional-member-expr-as-property.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-with-optional-member-expr-as-property.js diff --git a/compiler/forget/src/HIR/BuildHIR.ts b/compiler/forget/src/HIR/BuildHIR.ts index 12880fdac3..ea74366b29 100644 --- a/compiler/forget/src/HIR/BuildHIR.ts +++ b/compiler/forget/src/HIR/BuildHIR.ts @@ -2176,16 +2176,7 @@ function lowerMemberExpression( }, }; } - let property: Place; - // See "PropertyLoad" for the difference between optionalMemberExpr() - // and node.optional here - if (expr.isOptionalMemberExpression()) { - // if expr is in an optional chain, evaluation of `property` is - // conditional on whether expr is nullish - property = lowerReorderableExpression(builder, propertyNode); - } else { - property = lowerExpressionToTemporary(builder, propertyNode); - } + const property = lowerExpressionToTemporary(builder, propertyNode); const value: InstructionValue = { kind: "ComputedLoad", object: { ...object }, diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.optional-computed-member-expression.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.optional-computed-member-expression.expect.md deleted file mode 100644 index 403dd693d4..0000000000 --- a/compiler/forget/src/__tests__/fixtures/compiler/error.optional-computed-member-expression.expect.md +++ /dev/null @@ -1,25 +0,0 @@ - -## Input - -```javascript -function Component(props) { - const object = makeObject(props); - return object?.[props.key]; -} - -``` - - -## Error - -``` -[ReactForget] TodoError: (BuildHIR::node.lowerReorderableExpression) Expression type 'MemberExpression' cannot be safely reordered - 1 | function Component(props) { - 2 | const object = makeObject(props); -> 3 | return object?.[props.key]; - | ^^^^^^^^^ - 4 | } - 5 | -``` - - \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/optional-computed-member-expression.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/optional-computed-member-expression.expect.md new file mode 100644 index 0000000000..ffe5efaed8 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/optional-computed-member-expression.expect.md @@ -0,0 +1,43 @@ + +## Input + +```javascript +function Component(props) { + const object = makeObject(props); + return object?.[props.key]; +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function Component(props) { + const $ = useMemoCache(5); + const c_0 = $[0] !== props; + let t0; + if (c_0) { + t0 = makeObject(props); + $[0] = props; + $[1] = t0; + } else { + t0 = $[1]; + } + const object = t0; + const c_2 = $[2] !== object; + const c_3 = $[3] !== props; + let t1; + if (c_2 || c_3) { + t1 = object?.[props.key]; + $[2] = object; + $[3] = props; + $[4] = t1; + } else { + t1 = $[4]; + } + return t1; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.optional-computed-member-expression.js b/compiler/forget/src/__tests__/fixtures/compiler/optional-computed-member-expression.js similarity index 100% rename from compiler/forget/src/__tests__/fixtures/compiler/error.optional-computed-member-expression.js rename to compiler/forget/src/__tests__/fixtures/compiler/optional-computed-member-expression.js diff --git a/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-call-as-property.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-call-as-property.expect.md new file mode 100644 index 0000000000..62cfb69f35 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-call-as-property.expect.md @@ -0,0 +1,41 @@ + +## Input + +```javascript +function Component(props) { + const x = makeObject(); + return x?.[foo(props.value)]; +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function Component(props) { + const $ = useMemoCache(4); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = makeObject(); + $[0] = t0; + } else { + t0 = $[0]; + } + const x = t0; + const c_1 = $[1] !== props; + const c_2 = $[2] !== x; + let t1; + if (c_1 || c_2) { + t1 = x?.[foo(props.value)]; + $[1] = props; + $[2] = x; + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-call-as-property.js b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-call-as-property.js new file mode 100644 index 0000000000..ad51e5ec62 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-call-as-property.js @@ -0,0 +1,4 @@ +function Component(props) { + const x = makeObject(); + return x?.[foo(props.value)]; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-with-optional-member-expr-as-property.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-with-optional-member-expr-as-property.expect.md new file mode 100644 index 0000000000..e393a0d5a2 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-with-optional-member-expr-as-property.expect.md @@ -0,0 +1,41 @@ + +## Input + +```javascript +function Component(props) { + const x = makeObject(); + return x.y?.[props.a?.[props.b?.[props.c]]]; +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function Component(props) { + const $ = useMemoCache(4); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = makeObject(); + $[0] = t0; + } else { + t0 = $[0]; + } + const x = t0; + const c_1 = $[1] !== x.y; + const c_2 = $[2] !== props; + let t1; + if (c_1 || c_2) { + t1 = x.y?.[props.a?.[props.b?.[props.c]]]; + $[1] = x.y; + $[2] = props; + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-with-optional-member-expr-as-property.js b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-with-optional-member-expr-as-property.js new file mode 100644 index 0000000000..d5ced1ec06 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-with-optional-member-expr-as-property.js @@ -0,0 +1,4 @@ +function Component(props) { + const x = makeObject(); + return x.y?.[props.a?.[props.b?.[props.c]]]; +}