From ade3235ca3053f8786579050dc4dffa48d10233f Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Mon, 11 Sep 2023 16:54:05 -0700 Subject: [PATCH] ForInStatement: specialize inference/types Replaces the use of `NextIterableOf` in for-in with a new `NextPropertyOf` instruction. The key distinction is `for-of` invokes an arbitrary iterator, which means a) each iteration may mutate the collection being iterated and b) the returned value may be mutable. However, `for-in` invokes a language-level mechanism to iterate: simply iterating alone _cannot_ modify the collection, and the returned value is known to be a primitive. --- .../src/HIR/BuildHIR.ts | 8 ++-- .../babel-plugin-react-forget/src/HIR/HIR.ts | 5 ++ .../src/HIR/PrintHIR.ts | 4 ++ .../src/HIR/visitors.ts | 8 ++++ .../src/Inference/InferReferenceEffects.ts | 6 +++ .../src/Optimization/DeadCodeElimination.ts | 6 ++- .../ReactiveScopes/CodegenReactiveFunction.ts | 4 ++ .../InferReactiveScopeVariables.ts | 1 + .../ReactiveScopes/PruneNonEscapingScopes.ts | 1 + .../src/TypeInference/InferTypes.ts | 5 ++ .../for-in-statement-type-inference.expect.md | 46 +++++++++++++++++++ .../for-in-statement-type-inference.js | 16 +++++++ 12 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-in-statement-type-inference.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-in-statement-type-inference.js diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts index 68957c287c..c2088c085a 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts @@ -943,7 +943,7 @@ function lowerStatement( initBlock ); - // The init of a ForOf statement is compound over a left (VariableDeclaration | LVal) and + // The init of a ForIn statement is compound over a left (VariableDeclaration | LVal) and // right (Expression), so we synthesize a new InstrValue and assignment (potentially multiple // instructions when we handle other syntax like Patterns) const left = stmt.get("left"); @@ -952,14 +952,14 @@ function lowerStatement( if (left.isVariableDeclaration()) { const declarations = left.get("declarations"); CompilerError.invariant(declarations.length === 1, { - reason: `Expected only one declaration in the init of a ForOfStatement, got ${declarations.length}`, + reason: `Expected only one declaration in the init of a ForInStatement, got ${declarations.length}`, description: null, loc: left.node.loc ?? null, suggestions: null, }); const id = declarations[0].get("id"); const nextIterableOf = lowerValueToTemporary(builder, { - kind: "NextIterableOf", // TODO: change this to reflect for-in semantics (returns immutable keys, does not modify collection) + kind: "NextPropertyOf", loc: leftLoc, value, }); @@ -973,7 +973,7 @@ function lowerStatement( test = lowerValueToTemporary(builder, assign); } else { builder.errors.push({ - reason: `(BuildHIR::lowerStatement) Handle ${left.type} inits in ForOfStatement`, + reason: `(BuildHIR::lowerStatement) Handle ${left.type} inits in ForInStatement`, severity: ErrorSeverity.Todo, loc: left.node.loc ?? null, suggestions: null, diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts index 154f12ad47..3b8dd72205 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts @@ -788,6 +788,11 @@ export type InstructionValue = value: Place; // the collection loc: SourceLocation; } + | { + kind: "NextPropertyOf"; + value: Place; // the collection + loc: SourceLocation; + } // Models a prefix update expression such as --x or ++y // This instructions increments or decrements the // but evaluates to the value of prior to the update. diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts index 83022699c0..4b840b9ea3 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts @@ -542,6 +542,10 @@ export function printInstructionValue(instrValue: ReactiveValue): string { value = `NextIterableOf ${printPlace(instrValue.value)}`; break; } + case "NextPropertyOf": { + value = `NextPropertyOf ${printPlace(instrValue.value)}`; + break; + } case "Debugger": { value = `Debugger`; break; diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/visitors.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/visitors.ts index 3268ed1943..79b2ce3970 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/visitors.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/visitors.ts @@ -193,6 +193,10 @@ export function* eachInstructionValueOperand( yield instrValue.value; break; } + case "NextPropertyOf": { + yield instrValue.value; + break; + } case "PostfixUpdate": case "PrefixUpdate": { yield instrValue.value; @@ -476,6 +480,10 @@ export function mapInstructionOperands( instrValue.value = fn(instrValue.value); break; } + case "NextPropertyOf": { + instrValue.value = fn(instrValue.value); + break; + } case "PostfixUpdate": case "PrefixUpdate": { instrValue.value = fn(instrValue.value); 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 dd9be9244a..8cdabd16de 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts @@ -1020,6 +1020,12 @@ function inferBlock( valueKind = ValueKind.Mutable; break; } + case "NextPropertyOf": { + effectKind = Effect.Read; + lvalueEffect = Effect.Store; + valueKind = ValueKind.Immutable; + break; + } default: { assertExhaustive(instrValue, "Unexpected instruction kind"); } diff --git a/compiler/packages/babel-plugin-react-forget/src/Optimization/DeadCodeElimination.ts b/compiler/packages/babel-plugin-react-forget/src/Optimization/DeadCodeElimination.ts index ac8d7c47f4..8931ca4250 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Optimization/DeadCodeElimination.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Optimization/DeadCodeElimination.ts @@ -217,9 +217,11 @@ function pruneableValue(value: InstructionValue, state: State): boolean { // Potentially safe to prune, since they should just be creating new values return false; } + case "NextPropertyOf": case "NextIterableOf": { - // Technically a NextIterableOf will never be unused because it's always used later by - // another StoreLocal or Destructure instruction, but conceptually we can't prune + // Technically a NextIterableOf/NextPropertyOf will never be unused because it's + // always used later by another StoreLocal or Destructure instruction, but conceptually + // we can't prune return false; } case "LoadContext": diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts index ab5009dcc6..99d1b15dce 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -1212,6 +1212,10 @@ function codegenInstructionValue( value = codegenPlace(cx, instrValue.value); break; } + case "NextPropertyOf": { + value = codegenPlace(cx, instrValue.value); + break; + } case "PostfixUpdate": { value = t.updateExpression( instrValue.operation, diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts index 0d9050dda0..88914bfc94 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -244,6 +244,7 @@ function mayAllocate(env: Environment, instruction: Instruction): boolean { case "TemplateLiteral": case "Primitive": case "NextIterableOf": + case "NextPropertyOf": case "Debugger": { return false; } diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts index 0039fab936..4652d9db10 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts @@ -430,6 +430,7 @@ function computeMemoizationInputs( rvalues: value.children, }; } + case "NextPropertyOf": case "Debugger": case "ComputedDelete": case "PropertyDelete": diff --git a/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts b/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts index de1ae19285..c256787786 100644 --- a/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts +++ b/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts @@ -260,6 +260,11 @@ function* generateInstructionTypes( break; } + case "NextPropertyOf": { + yield equation(left, { kind: "Primitive" }); + break; + } + case "DeclareLocal": case "NewExpression": case "JsxExpression": diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-in-statement-type-inference.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-in-statement-type-inference.expect.md new file mode 100644 index 0000000000..cb52a706b2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-in-statement-type-inference.expect.md @@ -0,0 +1,46 @@ + +## Input + +```javascript +const { identity, mutate } = require("shared-runtime"); + +function Component(props) { + let x; + const object = { ...props.value }; + for (const y in object) { + x = y; + } + mutate(x); // can't modify, x is known primitive! + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: { a: "a", b: "B", c: "C!" } }], +}; + +``` + +## Code + +```javascript +const { identity, mutate } = require("shared-runtime"); + +function Component(props) { + let x; + const object = { ...props.value }; + for (const y in object) { + x = y; + } + + mutate(x); + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: { a: "a", b: "B", c: "C!" } }], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-in-statement-type-inference.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-in-statement-type-inference.js new file mode 100644 index 0000000000..801f159795 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-in-statement-type-inference.js @@ -0,0 +1,16 @@ +const { identity, mutate } = require("shared-runtime"); + +function Component(props) { + let x; + const object = { ...props.value }; + for (const y in object) { + x = y; + } + mutate(x); // can't modify, x is known primitive! + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: { a: "a", b: "B", c: "C!" } }], +};