From e047db8d037f63884a41575ea733c27fe8bd1f14 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 21 Mar 2024 17:44:13 -0400 Subject: [PATCH] [repro] Repro for control flow bug in PropagateScopeDependency --- Thanks to @josephsavona for finding this bug. This is another example of why we really want hir-everywhere. Forget output currently nullthrows because we believe `obj.a` is run unconditionally in source (missing the break/returns out of this scope) --- ...uce-reactive-deps-break-in-scope.expect.md | 63 +++++++++++++++ ...bug-reduce-reactive-deps-break-in-scope.ts | 19 +++++ ...ce-reactive-deps-return-in-scope.expect.md | 69 +++++++++++++++++ ...ug-reduce-reactive-deps-return-in-scope.ts | 17 +++++ ...eactive-cond-deps-break-in-scope.expect.md | 70 +++++++++++++++++ ...educe-reactive-cond-deps-break-in-scope.ts | 21 +++++ ...active-cond-deps-return-in-scope.expect.md | 76 +++++++++++++++++++ ...duce-reactive-cond-deps-return-in-scope.ts | 19 +++++ .../packages/snap/src/SproutTodoFilter.ts | 7 +- 9 files changed, 357 insertions(+), 4 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-break-in-scope.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-break-in-scope.ts create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-return-in-scope.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-return-in-scope.ts create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-break-in-scope.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-break-in-scope.ts create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-return-in-scope.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-return-in-scope.ts diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-break-in-scope.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-break-in-scope.expect.md new file mode 100644 index 0000000000..9629dd5cf2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-break-in-scope.expect.md @@ -0,0 +1,63 @@ + +## Input + +```javascript +function useFoo({ obj, objIsNull }) { + const x = []; + b0: { + if (objIsNull) { + break b0; + } + x.push(obj.a); + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function useFoo(t0) { + const $ = useMemoCache(3); + const { obj, objIsNull } = t0; + let x; + if ($[0] !== objIsNull || $[1] !== obj.a) { + x = []; + bb1: { + if (objIsNull) { + break bb1; + } + + x.push(obj.a); + } + $[0] = objIsNull; + $[1] = obj.a; + $[2] = x; + } else { + x = $[2]; + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-break-in-scope.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-break-in-scope.ts new file mode 100644 index 0000000000..8db289e9a7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-break-in-scope.ts @@ -0,0 +1,19 @@ +function useFoo({ obj, objIsNull }) { + const x = []; + b0: { + if (objIsNull) { + break b0; + } + x.push(obj.a); + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-return-in-scope.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-return-in-scope.expect.md new file mode 100644 index 0000000000..5340758854 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-return-in-scope.expect.md @@ -0,0 +1,69 @@ + +## Input + +```javascript +function useFoo({ obj, objIsNull }) { + const x = []; + if (objIsNull) { + return; + } + x.push(obj.b); + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function useFoo(t0) { + const $ = useMemoCache(4); + const { obj, objIsNull } = t0; + let x; + let t1; + if ($[0] !== objIsNull || $[1] !== obj.b) { + t1 = Symbol.for("react.early_return_sentinel"); + bb7: { + x = []; + if (objIsNull) { + t1 = undefined; + break bb7; + } + + x.push(obj.b); + } + $[0] = objIsNull; + $[1] = obj.b; + $[2] = x; + $[3] = t1; + } else { + x = $[2]; + t1 = $[3]; + } + if (t1 !== Symbol.for("react.early_return_sentinel")) { + return t1; + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-return-in-scope.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-return-in-scope.ts new file mode 100644 index 0000000000..25a4b75eb7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/bug-reduce-reactive-deps-return-in-scope.ts @@ -0,0 +1,17 @@ +function useFoo({ obj, objIsNull }) { + const x = []; + if (objIsNull) { + return; + } + x.push(obj.b); + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-break-in-scope.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-break-in-scope.expect.md new file mode 100644 index 0000000000..2ee79a5d5c --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-break-in-scope.expect.md @@ -0,0 +1,70 @@ + +## Input + +```javascript +function useFoo({ obj, objIsNull }) { + const x = []; + b0: { + if (objIsNull) { + break b0; + } else { + x.push(obj.a); + } + x.push(obj.b); + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function useFoo(t0) { + const $ = useMemoCache(3); + const { obj, objIsNull } = t0; + let x; + if ($[0] !== objIsNull || $[1] !== obj) { + x = []; + bb1: { + if (objIsNull) { + break bb1; + } else { + x.push(obj.a); + } + + x.push(obj.b); + } + $[0] = objIsNull; + $[1] = obj; + $[2] = x; + } else { + x = $[2]; + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; + +``` + +### Eval output +(kind: ok) [] +[2,null] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-break-in-scope.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-break-in-scope.ts new file mode 100644 index 0000000000..2a5cc03bf9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-break-in-scope.ts @@ -0,0 +1,21 @@ +function useFoo({ obj, objIsNull }) { + const x = []; + b0: { + if (objIsNull) { + break b0; + } else { + x.push(obj.a); + } + x.push(obj.b); + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-return-in-scope.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-return-in-scope.expect.md new file mode 100644 index 0000000000..a15917c97c --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-return-in-scope.expect.md @@ -0,0 +1,76 @@ + +## Input + +```javascript +function useFoo({ obj, objIsNull }) { + const x = []; + if (objIsNull) { + return; + } else { + x.push(obj.a); + } + x.push(obj.b); + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function useFoo(t0) { + const $ = useMemoCache(4); + const { obj, objIsNull } = t0; + let x; + let t1; + if ($[0] !== objIsNull || $[1] !== obj) { + t1 = Symbol.for("react.early_return_sentinel"); + bb8: { + x = []; + if (objIsNull) { + t1 = undefined; + break bb8; + } else { + x.push(obj.a); + } + + x.push(obj.b); + } + $[0] = objIsNull; + $[1] = obj; + $[2] = x; + $[3] = t1; + } else { + x = $[2]; + t1 = $[3]; + } + if (t1 !== Symbol.for("react.early_return_sentinel")) { + return t1; + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; + +``` + +### Eval output +(kind: ok) +[2,null] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-return-in-scope.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-return-in-scope.ts new file mode 100644 index 0000000000..992c8310a2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-return-in-scope.ts @@ -0,0 +1,19 @@ +function useFoo({ obj, objIsNull }) { + const x = []; + if (objIsNull) { + return; + } else { + x.push(obj.a); + } + x.push(obj.b); + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ obj: null, objIsNull: true }], + sequentialRenders: [ + { obj: null, objIsNull: true }, + { obj: { a: 2 }, objIsNull: false }, + ], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index af1b1e0a43..fef339c363 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -531,10 +531,9 @@ const skipFilter = new Set([ "rules-of-hooks/rules-of-hooks-93dc5d5e538a", "rules-of-hooks/rules-of-hooks-69521d94fa03", - // bug - "bug-jsx-memberexpr-tag-in-lambda", - "bug-invalid-code-when-bailout", - "component-syntax-ref-gating.flow", + // bugs + "bug-reduce-reactive-deps-return-in-scope", + "bug-reduce-reactive-deps-break-in-scope", // 'react-forget-runtime' not yet supported "flag-enable-emit-hook-guards",