diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index e0f5f899e8cbf..b1ef0bd431d14 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -36,7 +36,7 @@ import { getHookKind, makeIdentifierName, } from "../HIR/HIR"; -import { printPlace } from "../HIR/PrintHIR"; +import { printIdentifier, printPlace } from "../HIR/PrintHIR"; import { eachPatternOperand } from "../HIR/visitors"; import { Err, Ok, Result } from "../Utils/Result"; import { GuardKind } from "../Utils/RuntimeDiagnosticConstants"; @@ -524,8 +524,8 @@ function codegenReactiveScope( } CompilerError.invariant(identifier.name != null, { - reason: `Expected identifier '@${identifier.id}' to be named`, - description: null, + reason: `Expected scope declaration identifier to be named`, + description: `Declaration \`${printIdentifier(identifier)}\` is unnamed in scope @${scope.id}`, loc: null, suggestions: null, }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts index a3f5768336ec6..235d7a565b89d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts @@ -12,21 +12,20 @@ import { IdentifierId, InstructionId, Place, + PrunedReactiveScopeBlock, ReactiveFunction, ReactiveScopeBlock, ReactiveValue, + ScopeId, promoteTemporary, promoteTemporaryJsxTag, } from "../HIR/HIR"; import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors"; -type VisitorState = { - tags: JsxExpressionTags; -}; -class Visitor extends ReactiveFunctionVisitor { - override visitScope(block: ReactiveScopeBlock, state: VisitorState): void { - this.traverseScope(block, state); - for (const dep of block.scope.dependencies) { +class Visitor extends ReactiveFunctionVisitor { + override visitScope(scopeBlock: ReactiveScopeBlock, state: State): void { + this.traverseScope(scopeBlock, state); + for (const dep of scopeBlock.scope.dependencies) { const { identifier } = dep; if (identifier.name == null) { promoteIdentifier(identifier, state); @@ -39,14 +38,29 @@ class Visitor extends ReactiveFunctionVisitor { * Many of our current test fixtures do not return a value, so * it is better for now to promote (and memoize) every output. */ - for (const [, declaration] of block.scope.declarations) { + for (const [, declaration] of scopeBlock.scope.declarations) { if (declaration.identifier.name == null) { promoteIdentifier(declaration.identifier, state); } } } - override visitParam(place: Place, state: VisitorState): void { + override visitPrunedScope( + scopeBlock: PrunedReactiveScopeBlock, + state: State + ): void { + this.traversePrunedScope(scopeBlock, state); + for (const [, declaration] of scopeBlock.scope.declarations) { + if ( + declaration.identifier.name == null && + state.pruned.get(declaration.identifier.id)?.usedOutsideScope === true + ) { + promoteIdentifier(declaration.identifier, state); + } + } + } + + override visitParam(place: Place, state: State): void { if (place.identifier.name === null) { promoteIdentifier(place.identifier, state); } @@ -55,7 +69,7 @@ class Visitor extends ReactiveFunctionVisitor { override visitValue( id: InstructionId, value: ReactiveValue, - state: VisitorState + state: State ): void { this.traverseValue(id, value, state); if (value.kind === "FunctionExpression" || value.kind === "ObjectMethod") { @@ -67,7 +81,7 @@ class Visitor extends ReactiveFunctionVisitor { _id: InstructionId, _dependencies: Array, fn: ReactiveFunction, - state: VisitorState + state: State ): void { for (const operand of fn.params) { const place = operand.kind === "Identifier" ? operand : operand.place; @@ -80,25 +94,65 @@ class Visitor extends ReactiveFunctionVisitor { } type JsxExpressionTags = Set; -class CollectJsxTagsVisitor extends ReactiveFunctionVisitor { +type State = { + tags: JsxExpressionTags; + pruned: Map< + IdentifierId, + { activeScopes: Array; usedOutsideScope: boolean } + >; // true if referenced within another scope, false if only accessed outside of scopes +}; + +class CollectPromotableTemporaries extends ReactiveFunctionVisitor { + activeScopes: Array = []; + + override visitPlace(_id: InstructionId, place: Place, state: State): void { + if ( + this.activeScopes.length !== 0 && + state.pruned.has(place.identifier.id) + ) { + const prunedPlace = state.pruned.get(place.identifier.id)!; + if (prunedPlace.activeScopes.indexOf(this.activeScopes.at(-1)!) === -1) { + prunedPlace.usedOutsideScope = true; + } + } + } + override visitValue( id: InstructionId, value: ReactiveValue, - state: JsxExpressionTags + state: State ): void { this.traverseValue(id, value, state); if (value.kind === "JsxExpression" && value.tag.kind === "Identifier") { - state.add(value.tag.identifier.id); + state.tags.add(value.tag.identifier.id); } } + + override visitPrunedScope( + scopeBlock: PrunedReactiveScopeBlock, + state: State + ): void { + for (const [id] of scopeBlock.scope.declarations) { + state.pruned.set(id, { + activeScopes: [...this.activeScopes], + usedOutsideScope: false, + }); + } + } + + override visitScope(scopeBlock: ReactiveScopeBlock, state: State): void { + this.activeScopes.push(scopeBlock.scope.id); + this.traverseScope(scopeBlock, state); + this.activeScopes.pop(); + } } export function promoteUsedTemporaries(fn: ReactiveFunction): void { - const tags: JsxExpressionTags = new Set(); - visitReactiveFunction(fn, new CollectJsxTagsVisitor(), tags); - const state: VisitorState = { - tags, + const state: State = { + tags: new Set(), + pruned: new Map(), }; + visitReactiveFunction(fn, new CollectPromotableTemporaries(), state); for (const operand of fn.params) { const place = operand.kind === "Identifier" ? operand : operand.place; if (place.identifier.name === null) { @@ -108,7 +162,7 @@ export function promoteUsedTemporaries(fn: ReactiveFunction): void { visitReactiveFunction(fn, new Visitor(), state); } -function promoteIdentifier(identifier: Identifier, state: VisitorState): void { +function promoteIdentifier(identifier: Identifier, state: State): void { CompilerError.invariant(identifier.name === null, { reason: "promoteTemporary: Expected to be called only for temporary variables", diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts index 84e4b45c392d1..bc684ffe36f62 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts @@ -18,6 +18,7 @@ import { isUseRefType, makeInstructionId, Place, + PrunedReactiveScopeBlock, ReactiveFunction, ReactiveInstruction, ReactiveScope, @@ -687,6 +688,19 @@ class PropagationVisitor extends ReactiveFunctionVisitor { scope.scope.dependencies = scopeDependencies; } + override visitPrunedScope( + scopeBlock: PrunedReactiveScopeBlock, + context: Context + ): void { + /* + * NOTE: we explicitly throw away the deps, we only enter() the scope to record its + * declarations + */ + const _scopeDepdencies = context.enter(scopeBlock.scope, () => { + this.visitBlock(scopeBlock.instructions, context); + }); + } + override visitInstruction( instruction: ReactiveInstruction, context: Context diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-reactivity-value-block.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-reactivity-value-block.expect.md index 88e4ee8917d65..baba119a4b09b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-reactivity-value-block.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-reactivity-value-block.expect.md @@ -71,14 +71,15 @@ function Foo() { useNoAlias(); const shouldCaptureObj = obj != null && CONST_TRUE; - let t0; + const t0 = shouldCaptureObj ? identity(obj) : null; + let t1; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 = [shouldCaptureObj ? identity(obj) : null, obj]; - $[0] = t0; + t1 = [t0, obj]; + $[0] = t1; } else { - t0 = $[0]; + t1 = $[0]; } - const result = t0; + const result = t1; useNoAlias(result, obj); if (shouldCaptureObj && result[0] !== obj) {