From eee5ca2a92c32103b6bc3c1f7e6cdf1c4807ee56 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Fri, 22 Nov 2024 15:59:59 -0500 Subject: [PATCH] [compiler] Prune all unused array destructure items during DCE (#31619) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We didn't originally support holes within array patterns, so DCE was only able to prune unused items from the end of an array pattern. Now that we support holes we can replace any unused item with a hole, and then just prune the items to the last identifier/spread entry. Note: this was motivated by finding useState where either the state or setState go unused — both are strong indications that you're violating the rules in some way. By DCE-ing the unused portions of the useState destructuring we can easily check if you're ignoring either value. closes #31603 This is a redo of that PR not using ghstack --- .../src/Optimization/DeadCodeElimination.ts | 32 +++++++++---------- .../compiler/concise-arrow-expr.expect.md | 2 +- ...alysis-destructured-rest-element.expect.md | 3 +- ...rtent-mutability-readonly-lambda.expect.md | 2 +- ...ted-callback-from-other-callback.expect.md | 2 +- .../preserve-use-memo-transition.expect.md | 2 +- .../compiler/react-namespace.expect.md | 2 +- ...rol-dependency-phi-setState-type.expect.md | 4 +-- ...ession-of-jsxexpressioncontainer.expect.md | 3 +- .../unused-array-middle-element.expect.md | 2 +- ...patch-considered-as-non-reactive.expect.md | 2 +- ...urned-dispatcher-is-non-reactive.expect.md | 2 +- 12 files changed, 27 insertions(+), 31 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/DeadCodeElimination.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/DeadCodeElimination.ts index 0202d3ecf043f..2b752c6dfd28e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/DeadCodeElimination.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/DeadCodeElimination.ts @@ -6,7 +6,6 @@ */ import { - ArrayPattern, BlockId, HIRFunction, Identifier, @@ -184,29 +183,28 @@ function rewriteInstruction(instr: Instruction, state: State): void { switch (instr.value.lvalue.pattern.kind) { case 'ArrayPattern': { /* - * For arrays, we can only eliminate unused items from the end of the array, - * so we iterate from the end and break once we find a used item. Note that - * we already know at least one item is used, from the pruneableValue check. + * For arrays, we can prune items prior to the end by replacing + * them with a hole. Items at the end can simply be dropped. */ - let nextItems: ArrayPattern['items'] | null = null; - const originalItems = instr.value.lvalue.pattern.items; - for (let i = originalItems.length - 1; i >= 0; i--) { - const item = originalItems[i]; + let lastEntryIndex = 0; + const items = instr.value.lvalue.pattern.items; + for (let i = 0; i < items.length; i++) { + const item = items[i]; if (item.kind === 'Identifier') { - if (state.isIdOrNameUsed(item.identifier)) { - nextItems = originalItems.slice(0, i + 1); - break; + if (!state.isIdOrNameUsed(item.identifier)) { + items[i] = {kind: 'Hole'}; + } else { + lastEntryIndex = i; } } else if (item.kind === 'Spread') { - if (state.isIdOrNameUsed(item.place.identifier)) { - nextItems = originalItems.slice(0, i + 1); - break; + if (!state.isIdOrNameUsed(item.place.identifier)) { + items[i] = {kind: 'Hole'}; + } else { + lastEntryIndex = i; } } } - if (nextItems !== null) { - instr.value.lvalue.pattern.items = nextItems; - } + items.length = lastEntryIndex + 1; break; } case 'ObjectPattern': { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/concise-arrow-expr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/concise-arrow-expr.expect.md index 787418f75b3fb..996afa1cb224b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/concise-arrow-expr.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/concise-arrow-expr.expect.md @@ -16,7 +16,7 @@ function component() { import { c as _c } from "react/compiler-runtime"; function component() { const $ = _c(1); - const [x, setX] = useState(0); + const [, setX] = useState(0); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { const handler = (v) => setX(v); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/escape-analysis-destructured-rest-element.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/escape-analysis-destructured-rest-element.expect.md index dfa55302aee84..f38ffba958587 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/escape-analysis-destructured-rest-element.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/escape-analysis-destructured-rest-element.expect.md @@ -35,8 +35,7 @@ function Component(props) { } let d; if ($[2] !== props.c) { - const [c, ...t0] = props.c; - d = t0; + [, ...d] = props.c; $[2] = props.c; $[3] = d; } else { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inadvertent-mutability-readonly-lambda.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inadvertent-mutability-readonly-lambda.expect.md index 72433173b17ca..76dfc5ab1ca5c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inadvertent-mutability-readonly-lambda.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inadvertent-mutability-readonly-lambda.expect.md @@ -24,7 +24,7 @@ function Component(props) { import { c as _c } from "react/compiler-runtime"; function Component(props) { const $ = _c(2); - const [value, setValue] = useState(null); + const [, setValue] = useState(null); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = (e) => setValue((value_0) => value_0 + e.target.value); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-calls-to-hoisted-callback-from-other-callback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-calls-to-hoisted-callback-from-other-callback.expect.md index 710f3ac8ec782..57ca944af6bc9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-calls-to-hoisted-callback-from-other-callback.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/multiple-calls-to-hoisted-callback-from-other-callback.expect.md @@ -39,7 +39,7 @@ import { useState } from "react"; function Component(props) { const $ = _c(1); - const [_state, setState] = useState(); + const [, setState] = useState(); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { const a = () => b(); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/preserve-use-memo-transition.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/preserve-use-memo-transition.expect.md index 941c37bc3f29f..3687198df3a99 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/preserve-use-memo-transition.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/preserve-use-memo-transition.expect.md @@ -28,7 +28,7 @@ import { useCallback, useTransition } from "react"; function useFoo() { const $ = _c(1); - const [t, start] = useTransition(); + const [, start] = useTransition(); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = () => { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/react-namespace.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/react-namespace.expect.md index 0afc5b651bb94..5deb18397d91b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/react-namespace.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/react-namespace.expect.md @@ -32,7 +32,7 @@ function Component(props) { const $ = _c(5); React.useContext(FooContext); const ref = React.useRef(); - const [x, setX] = React.useState(false); + const [, setX] = React.useState(false); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = () => { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-phi-setState-type.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-phi-setState-type.expect.md index 26e1f35d5b94a..c56bb7936c1aa 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-phi-setState-type.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-phi-setState-type.expect.md @@ -61,8 +61,8 @@ import { useState } from "react"; function Component(props) { const $ = _c(5); - const [x, setX] = useState(false); - const [y, setY] = useState(false); + const [, setX] = useState(false); + const [, setY] = useState(false); let setState; if (props.cond) { setState = setX; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-undefined-expression-of-jsxexpressioncontainer.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-undefined-expression-of-jsxexpressioncontainer.expect.md index b04c51abc51de..3e71c05a2efbf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-undefined-expression-of-jsxexpressioncontainer.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-undefined-expression-of-jsxexpressioncontainer.expect.md @@ -52,8 +52,7 @@ function Component(props) { const { buttons } = props; let nonPrimaryButtons; if ($[0] !== buttons) { - const [primaryButton, ...t0] = buttons; - nonPrimaryButtons = t0; + [, ...nonPrimaryButtons] = buttons; $[0] = buttons; $[1] = nonPrimaryButtons; } else { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unused-array-middle-element.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unused-array-middle-element.expect.md index 2cf76a57adf40..4ae1fb6e22c43 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unused-array-middle-element.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/unused-array-middle-element.expect.md @@ -19,7 +19,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript function foo(props) { - const [x, unused, y] = props.a; + const [x, , y] = props.a; return x + y; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useActionState-dispatch-considered-as-non-reactive.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useActionState-dispatch-considered-as-non-reactive.expect.md index 4181651e3d60f..4b6b94ce431c2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useActionState-dispatch-considered-as-non-reactive.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useActionState-dispatch-considered-as-non-reactive.expect.md @@ -29,7 +29,7 @@ import { useActionState } from "react"; function Component() { const $ = _c(1); - const [actionState, dispatchAction] = useActionState(); + const [, dispatchAction] = useActionState(); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { const onSubmitAction = () => { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md index f5adab196b7bd..a500af2fc8f76 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useReducer-returned-dispatcher-is-non-reactive.expect.md @@ -30,7 +30,7 @@ import { useReducer } from "react"; function f() { const $ = _c(1); - const [state, dispatch] = useReducer(); + const [, dispatch] = useReducer(); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { const onClick = () => {