From d2a7be0ac07c1849eefca07e6f9cdbfdf56ad722 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 26 Sep 2024 18:29:53 -0400 Subject: [PATCH] Update (base update) [ghstack-poisoned] --- .../src/HIR/CollectHoistablePropertyLoads.ts | 20 +++--- ...-try-catch-maybe-null-dependency.expect.md | 57 --------------- .../bug-try-catch-maybe-null-dependency.ts | 18 ----- .../try-catch-maybe-null-dependency.expect.md | 71 ------------------- .../try-catch-maybe-null-dependency.ts | 19 ----- ...value-modified-in-catch-escaping.expect.md | 63 ++++++++++++++++ ...ch-try-value-modified-in-catch-escaping.js | 20 ++++++ ...atch-try-value-modified-in-catch.expect.md | 69 ++++++++++++++++++ .../try-catch-try-value-modified-in-catch.js | 19 +++++ .../packages/snap/src/SproutTodoFilter.ts | 1 - 10 files changed, 182 insertions(+), 175 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-try-catch-maybe-null-dependency.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-try-catch-maybe-null-dependency.ts delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-maybe-null-dependency.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-maybe-null-dependency.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch-escaping.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch-escaping.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts index ea90ccaf285a7..0b7f1708ba1fd 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -195,12 +195,12 @@ class PropertyPathRegistry { } function addNonNullPropertyPath( - node: PropertyPathNode, + source: Identifier, + sourceNode: PropertyPathNode, instrId: InstructionId, knownImmutableIdentifiers: Set, result: Set, ): void { - const object = node.fullPath.identifier; /** * Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges * are not valid with respect to current instruction id numbering. @@ -212,14 +212,14 @@ function addNonNullPropertyPath( * See comment at top of function for why we track known immutable identifiers. */ const isMutableAtInstr = - object.mutableRange.end > object.mutableRange.start + 1 && - object.scope != null && - inRange({id: instrId}, object.scope.range); + source.mutableRange.end > source.mutableRange.start + 1 && + source.scope != null && + inRange({id: instrId}, source.scope.range); if ( !isMutableAtInstr || - knownImmutableIdentifiers.has(node.fullPath.identifier.id) + knownImmutableIdentifiers.has(sourceNode.fullPath.identifier.id) ) { - result.add(node); + result.add(sourceNode); } } @@ -282,9 +282,9 @@ function collectNonNullsInBlocks( identifier: instr.value.object.identifier, path: [], }; - const propertyNode = registry.getOrCreateProperty(source); addNonNullPropertyPath( - propertyNode, + instr.value.object.identifier, + registry.getOrCreateProperty(source), instr.id, knownImmutableIdentifiers, assumedNonNullObjects, @@ -297,6 +297,7 @@ function collectNonNullsInBlocks( const sourceNode = temporaries.get(source); if (sourceNode != null) { addNonNullPropertyPath( + instr.value.value.identifier, registry.getOrCreateProperty(sourceNode), instr.id, knownImmutableIdentifiers, @@ -311,6 +312,7 @@ function collectNonNullsInBlocks( const sourceNode = temporaries.get(source); if (sourceNode != null) { addNonNullPropertyPath( + instr.value.object.identifier, registry.getOrCreateProperty(sourceNode), instr.id, knownImmutableIdentifiers, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-try-catch-maybe-null-dependency.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-try-catch-maybe-null-dependency.expect.md deleted file mode 100644 index ed2c4d7d3d293..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-try-catch-maybe-null-dependency.expect.md +++ /dev/null @@ -1,57 +0,0 @@ - -## Input - -```javascript -import {identity} from 'shared-runtime'; - -function useFoo(maybeNullObject: {value: {inner: number}} | null) { - const y = []; - try { - y.push(identity(maybeNullObject.value.inner)); - } catch { - y.push('null'); - } - - return y; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [null], - sequentialRenders: [null, {value: 2}, {value: 3}, null], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -import { identity } from "shared-runtime"; - -function useFoo(maybeNullObject) { - const $ = _c(2); - let y; - if ($[0] !== maybeNullObject.value.inner) { - y = []; - try { - y.push(identity(maybeNullObject.value.inner)); - } catch { - y.push("null"); - } - $[0] = maybeNullObject.value.inner; - $[1] = y; - } else { - y = $[1]; - } - return y; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [null], - sequentialRenders: [null, { value: 2 }, { value: 3 }, null], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-try-catch-maybe-null-dependency.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-try-catch-maybe-null-dependency.ts deleted file mode 100644 index de7e2626f57b4..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-try-catch-maybe-null-dependency.ts +++ /dev/null @@ -1,18 +0,0 @@ -import {identity} from 'shared-runtime'; - -function useFoo(maybeNullObject: {value: {inner: number}} | null) { - const y = []; - try { - y.push(identity(maybeNullObject.value.inner)); - } catch { - y.push('null'); - } - - return y; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [null], - sequentialRenders: [null, {value: 2}, {value: 3}, null], -}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-maybe-null-dependency.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-maybe-null-dependency.expect.md deleted file mode 100644 index 8674de709c3b0..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-maybe-null-dependency.expect.md +++ /dev/null @@ -1,71 +0,0 @@ - -## Input - -```javascript -// @enablePropagateDepsInHIR -import {identity} from 'shared-runtime'; - -function useFoo(maybeNullObject: {value: {inner: number}} | null) { - const y = []; - try { - y.push(identity(maybeNullObject.value.inner)); - } catch { - y.push('null'); - } - - return y; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [null], - sequentialRenders: [null, {value: 2}, {value: 3}, null], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR -import { identity } from "shared-runtime"; - -function useFoo(maybeNullObject) { - const $ = _c(4); - let y; - if ($[0] !== maybeNullObject) { - y = []; - try { - let t0; - if ($[2] !== maybeNullObject.value.inner) { - t0 = identity(maybeNullObject.value.inner); - $[2] = maybeNullObject.value.inner; - $[3] = t0; - } else { - t0 = $[3]; - } - y.push(t0); - } catch { - y.push("null"); - } - $[0] = maybeNullObject; - $[1] = y; - } else { - y = $[1]; - } - return y; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [null], - sequentialRenders: [null, { value: 2 }, { value: 3 }, null], -}; - -``` - -### Eval output -(kind: ok) ["null"] -[null] -[null] -["null"] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-maybe-null-dependency.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-maybe-null-dependency.ts deleted file mode 100644 index 58bf5943d5b1b..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-maybe-null-dependency.ts +++ /dev/null @@ -1,19 +0,0 @@ -// @enablePropagateDepsInHIR -import {identity} from 'shared-runtime'; - -function useFoo(maybeNullObject: {value: {inner: number}} | null) { - const y = []; - try { - y.push(identity(maybeNullObject.value.inner)); - } catch { - y.push('null'); - } - - return y; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [null], - sequentialRenders: [null, {value: 2}, {value: 3}, null], -}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch-escaping.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch-escaping.expect.md new file mode 100644 index 0000000000000..914001f3737bd --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch-escaping.expect.md @@ -0,0 +1,63 @@ + +## Input + +```javascript +// @enablePropagateDepsInHIR +const {throwInput} = require('shared-runtime'); + +function Component(props) { + let x; + try { + const y = []; + y.push(props.y); + throwInput(y); + } catch (e) { + e.push(props.e); + x = e; + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{y: 'foo', e: 'bar'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR +const { throwInput } = require("shared-runtime"); + +function Component(props) { + const $ = _c(2); + let x; + if ($[0] !== props) { + try { + const y = []; + y.push(props.y); + throwInput(y); + } catch (t0) { + const e = t0; + e.push(props.e); + x = e; + } + $[0] = props; + $[1] = x; + } else { + x = $[1]; + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ y: "foo", e: "bar" }], +}; + +``` + +### Eval output +(kind: ok) ["foo","bar"] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch-escaping.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch-escaping.js new file mode 100644 index 0000000000000..5a0864118ba8f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch-escaping.js @@ -0,0 +1,20 @@ +// @enablePropagateDepsInHIR +const {throwInput} = require('shared-runtime'); + +function Component(props) { + let x; + try { + const y = []; + y.push(props.y); + throwInput(y); + } catch (e) { + e.push(props.e); + x = e; + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{y: 'foo', e: 'bar'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch.expect.md new file mode 100644 index 0000000000000..30ecdf6d59e9d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch.expect.md @@ -0,0 +1,69 @@ + +## Input + +```javascript +// @enablePropagateDepsInHIR +const {throwInput} = require('shared-runtime'); + +function Component(props) { + try { + const y = []; + y.push(props.y); + throwInput(y); + } catch (e) { + e.push(props.e); + return e; + } + return null; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{y: 'foo', e: 'bar'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR +const { throwInput } = require("shared-runtime"); + +function Component(props) { + const $ = _c(2); + let t0; + if ($[0] !== props) { + t0 = Symbol.for("react.early_return_sentinel"); + bb0: { + try { + const y = []; + y.push(props.y); + throwInput(y); + } catch (t1) { + const e = t1; + e.push(props.e); + t0 = e; + break bb0; + } + } + $[0] = props; + $[1] = t0; + } else { + t0 = $[1]; + } + if (t0 !== Symbol.for("react.early_return_sentinel")) { + return t0; + } + return null; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ y: "foo", e: "bar" }], +}; + +``` + +### Eval output +(kind: ok) ["foo","bar"] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch.js new file mode 100644 index 0000000000000..97d650453c175 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/try-catch-try-value-modified-in-catch.js @@ -0,0 +1,19 @@ +// @enablePropagateDepsInHIR +const {throwInput} = require('shared-runtime'); + +function Component(props) { + try { + const y = []; + y.push(props.y); + throwInput(y); + } catch (e) { + e.push(props.e); + return e; + } + return null; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{y: 'foo', e: 'bar'}], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index c40392884d579..0ae22a643b326 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -478,7 +478,6 @@ const skipFilter = new Set([ 'fbt/bug-fbt-plural-multiple-function-calls', 'fbt/bug-fbt-plural-multiple-mixed-call-tag', 'bug-invalid-hoisting-functionexpr', - 'bug-try-catch-maybe-null-dependency', 'reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond', 'original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block', 'original-reactive-scopes-fork/bug-hoisted-declaration-with-scope',