From 3aaf8be25cc4db8307eb3f6ce65a0bc4101d43e8 Mon Sep 17 00:00:00 2001 From: Sathya Gunasekaran Date: Wed, 4 Oct 2023 12:11:35 +0530 Subject: [PATCH] [hir] Use a stable identity for undefined value InferReferenceEffects uses object identity to merge states, which breaks when we create a new object to model `undefined`. Two value objects representing `undefined` are not equal due to referential equality. Instead, let's use a singleton to represent `undefined` value. --- .../src/Inference/InferReferenceEffects.ts | 13 ++++-- .../for-loop-let-undefined-decl.expect.md | 46 +++++++++++++++++++ .../compiler/for-loop-let-undefined-decl.js | 16 +++++++ .../compiler/loop-unused-let.expect.md | 21 +++++++++ .../fixtures/compiler/loop-unused-let.js | 5 ++ .../packages/sprout/src/SproutTodoFilter.ts | 1 + 6 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-loop-let-undefined-decl.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-loop-let-undefined-decl.js create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/loop-unused-let.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/loop-unused-let.js diff --git a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts index 57d2b0808d..0a3953093d 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts @@ -12,6 +12,7 @@ import { BlockId, CallExpression, Effect, + GeneratedSource, HIRFunction, IdentifierId, InstructionKind, @@ -39,6 +40,12 @@ import { } from "../HIR/visitors"; import { assertExhaustive } from "../Utils/utils"; +const UndefinedValue: InstructionValue = { + kind: "Primitive", + loc: GeneratedSource, + value: undefined, +}; + /** * For every usage of a value in the given function, infers the effect or action * taken at that reference. Each reference is inferred as exactly one of: @@ -933,11 +940,7 @@ function inferBlock( continue; } case "DeclareLocal": { - const value: InstructionValue = { - kind: "Primitive", - loc: instrValue.loc, - value: undefined, - }; + const value = UndefinedValue; state.initialize( value, // Catch params may be aliased to mutable values diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-loop-let-undefined-decl.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-loop-let-undefined-decl.expect.md new file mode 100644 index 0000000000..1e722713ad --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-loop-let-undefined-decl.expect.md @@ -0,0 +1,46 @@ + +## Input + +```javascript +function useFoo() { + for (let i = 0; i <= 5; i++) { + let color; + if (isSelected) { + color = isCurrent ? "#FFCC22" : "#FF5050"; + } else { + color = isCurrent ? "#CCFF03" : "#CCCCCC"; + } + console.log(color); + } +} + +export const FIXTURE_ENTRYPOINT = { + params: [], + fn: useFoo, +}; + +``` + +## Code + +```javascript +function useFoo() { + for (let i = 0; i <= 5; i++) { + let color = undefined; + if (isSelected) { + color = isCurrent ? "#FFCC22" : "#FF5050"; + } else { + color = isCurrent ? "#CCFF03" : "#CCCCCC"; + } + + console.log(color); + } +} + +export const FIXTURE_ENTRYPOINT = { + params: [], + fn: useFoo, +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-loop-let-undefined-decl.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-loop-let-undefined-decl.js new file mode 100644 index 0000000000..d97b3968c7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-loop-let-undefined-decl.js @@ -0,0 +1,16 @@ +function useFoo() { + for (let i = 0; i <= 5; i++) { + let color; + if (isSelected) { + color = isCurrent ? "#FFCC22" : "#FF5050"; + } else { + color = isCurrent ? "#CCFF03" : "#CCCCCC"; + } + console.log(color); + } +} + +export const FIXTURE_ENTRYPOINT = { + params: [], + fn: useFoo, +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/loop-unused-let.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/loop-unused-let.expect.md new file mode 100644 index 0000000000..3ac76347cf --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/loop-unused-let.expect.md @@ -0,0 +1,21 @@ + +## Input + +```javascript +function useFoo() { + while (1) { + let foo; + } +} + +``` + +## Code + +```javascript +function useFoo() { + while (1) {} +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/loop-unused-let.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/loop-unused-let.js new file mode 100644 index 0000000000..c2c8048010 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/loop-unused-let.js @@ -0,0 +1,5 @@ +function useFoo() { + while (1) { + let foo; + } +} diff --git a/compiler/packages/sprout/src/SproutTodoFilter.ts b/compiler/packages/sprout/src/SproutTodoFilter.ts index 65f14c5a92..e1a5f510d9 100644 --- a/compiler/packages/sprout/src/SproutTodoFilter.ts +++ b/compiler/packages/sprout/src/SproutTodoFilter.ts @@ -461,6 +461,7 @@ const skipFilter = new Set([ "fbtparam-with-jsx-fragment-value", "fbt-preserve-jsxtext", "useContext-mutable-value", + "loop-unused-let", ]); export default skipFilter;