From 1b27ae9b60e41011d2e1849c9311af17b707cd40 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 5 Sep 2024 20:09:39 -0400 Subject: [PATCH 1/9] Update [ghstack-poisoned] --- .../src/Entrypoint/Pipeline.ts | 24 +- .../src/HIR/CollectHoistablePropertyLoads.ts | 429 +++++++++++++++ .../src/HIR/DeriveMinimalDependenciesHIR.ts | 270 +++++++++ .../src/HIR/PropagateScopeDependenciesHIR.ts | 515 ++++++++++++++++++ .../src/HIR/visitors.ts | 73 +++ .../InferReactiveScopeVariables.ts | 5 + .../src/Utils/utils.ts | 27 +- .../conditional-break-labeled.expect.md | 18 +- .../conditional-early-return.expect.md | 78 ++- .../compiler/conditional-on-mutable.expect.md | 24 +- ...-array-assignment-to-context-var.expect.md | 4 +- ...array-declaration-to-context-var.expect.md | 6 +- ...ucture-array-declaration-to-context-var.js | 2 +- ...rly-return-within-reactive-scope.expect.md | 26 +- ...rly-return-within-reactive-scope.expect.md | 20 +- ...unctionexpr-conditional-access-2.expect.md | 67 +++ .../functionexpr-conditional-access-2.tsx | 12 + ...r\342\200\223conditional-access.expect.md" | 4 +- ...tionexpr\342\200\223conditional-access.js" | 2 +- .../iife-return-modified-later-phi.expect.md | 11 +- ...consequent-alternate-both-return.expect.md | 11 +- ...call-with-optional-property-load.expect.md | 4 +- ...rly-return-within-reactive-scope.expect.md | 22 +- ...ence-array-push-consecutive-phis.expect.md | 18 +- .../phi-type-inference-array-push.expect.md | 11 +- ...hi-type-inference-property-store.expect.md | 11 +- ...less-specific-conditional-access.expect.md | 2 + .../conditional-member-expr.expect.md | 4 +- .../hoist-deps-diff-ssa-instance.expect.md | 107 ++++ .../hoist-deps-diff-ssa-instance.tsx | 32 ++ .../hoist-deps-diff-ssa-instance1.expect.md | 96 ++++ .../hoist-deps-diff-ssa-instance1.tsx | 27 + .../join-uncond-scopes-cond-deps.expect.md | 15 +- .../memberexpr-join-optional-chain.expect.md | 4 +- .../memberexpr-join-optional-chain2.expect.md | 21 +- .../promote-uncond.expect.md | 13 +- .../todo-merge-ssa-phi-access-nodes.expect.md | 110 ++++ .../todo-merge-ssa-phi-access-nodes.ts | 41 ++ .../uncond-access-in-mutable-range.expect.md | 105 ++++ .../uncond-access-in-mutable-range.js | 30 + ...epro-scope-missing-mutable-range.expect.md | 4 +- .../ssa-cascading-eliminated-phis.expect.md | 25 +- .../compiler/ssa-leave-case.expect.md | 11 +- ...ernary-destruction-with-mutation.expect.md | 12 +- ...ssa-renaming-ternary-destruction.expect.md | 11 +- ...a-renaming-ternary-with-mutation.expect.md | 12 +- .../compiler/ssa-renaming-ternary.expect.md | 11 +- ...onditional-ternary-with-mutation.expect.md | 12 +- ...a-renaming-unconditional-ternary.expect.md | 12 +- ...ming-unconditional-with-mutation.expect.md | 12 +- ...-via-destructuring-with-mutation.expect.md | 12 +- .../ssa-renaming-with-mutation.expect.md | 12 +- .../switch-non-final-default.expect.md | 31 +- .../fixtures/compiler/switch.expect.md | 26 +- .../todo-global-load-cached.expect.md | 73 +++ .../compiler/todo-global-load-cached.tsx | 17 + ...todo-global-property-load-cached.expect.md | 78 +++ .../todo-global-property-load-cached.tsx | 20 + .../try-catch-mutate-outer-value.expect.md | 16 +- ...value-modified-in-catch-escaping.expect.md | 11 +- ...atch-try-value-modified-in-catch.expect.md | 11 +- .../useMemo-multiple-if-else.expect.md | 22 +- .../snap/src/sprout/shared-runtime.ts | 7 +- 63 files changed, 2493 insertions(+), 266 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr-conditional-access-2.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr-conditional-access-2.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/uncond-access-in-mutable-range.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/uncond-access-in-mutable-range.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-load-cached.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-load-cached.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-property-load-cached.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-property-load-cached.tsx diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 2f6d8a94021ab..cc0fbadb639ef 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -21,6 +21,7 @@ import { lower, mergeConsecutiveBlocks, mergeOverlappingReactiveScopesHIR, + printFunction, pruneUnusedLabelsHIR, } from '../HIR'; import { @@ -101,6 +102,7 @@ import {propagatePhiTypes} from '../TypeInference/PropagatePhiTypes'; import {lowerContextAccess} from '../Optimization/LowerContextAccess'; import {validateNoSetStateInPassiveEffects} from '../Validation/ValidateNoSetStateInPassiveEffects'; import {validateNoJSXInTryStatement} from '../Validation/ValidateNoJSXInTryStatement'; +import {propagateScopeDependenciesHIR} from '../HIR/PropagateScopeDependenciesHIR'; export type CompilerPipelineValue = | {kind: 'ast'; name: string; value: CodegenFunction} @@ -341,6 +343,14 @@ function* runWithEnvironment( }); assertTerminalSuccessorsExist(hir); assertTerminalPredsExist(hir); + if (env.config.enablePropagateDepsInHIR) { + propagateScopeDependenciesHIR(hir); + yield log({ + kind: 'hir', + name: 'PropagateScopeDependenciesHIR', + value: hir, + }); + } const reactiveFunction = buildReactiveFunction(hir); yield log({ @@ -359,12 +369,14 @@ function* runWithEnvironment( }); assertScopeInstructionsWithinScopes(reactiveFunction); - propagateScopeDependencies(reactiveFunction); - yield log({ - kind: 'reactive', - name: 'PropagateScopeDependencies', - value: reactiveFunction, - }); + if (!env.config.enablePropagateDepsInHIR) { + propagateScopeDependencies(reactiveFunction); + yield log({ + kind: 'reactive', + name: 'PropagateScopeDependencies', + value: reactiveFunction, + }); + } pruneNonEscapingScopes(reactiveFunction); yield log({ diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts new file mode 100644 index 0000000000000..9aaef1ad8c648 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -0,0 +1,429 @@ +import {CompilerError} from '../CompilerError'; +import {inMutableRange} from '../ReactiveScopes/InferReactiveScopeVariables'; +import {Set_intersect, Set_union, getOrInsertDefault} from '../Utils/utils'; +import { + BasicBlock, + BlockId, + DeclarationId, + GeneratedSource, + HIRFunction, + Identifier, + Place, + ReactiveScopeDependency, + ScopeId, +} from './HIR'; + +type CollectHoistablePropertyLoadsResult = { + nodes: Map; + temporaries: Map; + properties: Map; +}; + +export function collectHoistablePropertyLoads( + fn: HIRFunction, + usedOutsideDeclaringScope: Set, +): CollectHoistablePropertyLoadsResult { + const sidemap = new TemporariesSideMap(); + + const nodes = collectNodes(fn, usedOutsideDeclaringScope, sidemap); + deriveNonNull(fn, nodes); + + const nodesKeyedByScopeId = new Map(); + for (const [_, block] of fn.body.blocks) { + if (block.terminal.kind === 'scope') { + nodesKeyedByScopeId.set( + block.terminal.scope.id, + nodes.get(block.terminal.block)!, + ); + } + } + + return { + nodes: nodesKeyedByScopeId, + temporaries: sidemap.temporaries, + properties: sidemap.properties, + }; +} + +export type BlockInfo = { + block: BasicBlock; + assumedNonNullObjects: Set; +}; + +export function getProperty( + object: Place, + propertyName: string, + temporaries: ReadonlyMap, + properties: ReadonlyMap, +): ReactiveScopeDependency { + /* + * (1) Get the base object either from the temporary sidemap (e.g. a LoadLocal) + * or a deep copy of an existing property dependency. + * Example 1: + * $0 = LoadLocal x + * $1 = PropertyLoad $0.y + * getProperty($0, ...) -> resolvedObject = x, resolvedDependency = null + * + * Example 2: + * $0 = LoadLocal x + * $1 = PropertyLoad $0.y + * $2 = PropertyLoad $1.z + * getProperty($1, ...) -> resolvedObject = null, resolvedDependency = x.y + * + * Example 3: + * $0 = Call(...) + * $1 = PropertyLoad $0.y + * getProperty($0, ...) -> resolvedObject = null, resolvedDependency = null + */ + const resolvedObject = resolveTemporary(object, temporaries); + const resolvedDependency = properties.get(resolvedObject); + + /** + * (2) Push the last PropertyLoad + * TODO(mofeiZ): understand optional chaining + */ + let property: ReactiveScopeDependency; + if (resolvedDependency == null) { + property = { + identifier: resolvedObject, + path: [{property: propertyName, optional: false}], + }; + } else { + property = { + identifier: resolvedDependency.identifier, + path: [ + ...resolvedDependency.path, + {property: propertyName, optional: false}, + ], + }; + } + return property; +} + +export function resolveTemporary( + place: Place, + temporaries: ReadonlyMap, +): Identifier { + return temporaries.get(place.identifier) ?? place.identifier; +} + +/** + * Tree data structure to dedupe property loads (e.g. a.b.c) + * and make computing sets intersections simpler. + */ +type RootNode = { + properties: Map; + parent: null; + // Recorded to make later computations simpler + fullPath: ReactiveScopeDependency; + root: Identifier; +}; + +type PropertyLoadNode = + | { + properties: Map; + parent: PropertyLoadNode; + fullPath: ReactiveScopeDependency; + } + | RootNode; + +class Tree { + roots: Map = new Map(); + + #getOrCreateRoot(identifier: Identifier): PropertyLoadNode { + // roots can always be accessed unconditionally in JS + let rootNode = this.roots.get(identifier); + + if (rootNode === undefined) { + rootNode = { + root: identifier, + properties: new Map(), + fullPath: { + identifier, + path: [], + }, + parent: null, + }; + this.roots.set(identifier, rootNode); + } + return rootNode; + } + + static #getOrCreateProperty( + node: PropertyLoadNode, + property: string, + ): PropertyLoadNode { + let child = node.properties.get(property); + if (child == null) { + child = { + properties: new Map(), + parent: node, + fullPath: { + identifier: node.fullPath.identifier, + path: node.fullPath.path.concat([{property, optional: false}]), + }, + }; + node.properties.set(property, child); + } + return child; + } + + getPropertyLoadNode(n: ReactiveScopeDependency): PropertyLoadNode { + CompilerError.invariant(n.path.length > 0, { + reason: + '[CollectHoistablePropertyLoads] Expected property node, found root node', + loc: GeneratedSource, + }); + /** + * We add ReactiveScopeDependencies according to instruction ordering, + * so all subpaths of a PropertyLoad should already exist + * (e.g. a.b is added before a.b.c), + */ + let currNode = this.#getOrCreateRoot(n.identifier); + for (let i = 0; i < n.path.length - 1; i++) { + currNode = assertNonNull(currNode.properties.get(n.path[i].property)); + } + + return Tree.#getOrCreateProperty(currNode, n.path.at(-1)!.property); + } +} + +class TemporariesSideMap { + temporaries: Map = new Map(); + properties: Map = new Map(); + tree: Tree = new Tree(); + + declareTemporary(from: Identifier, to: Identifier): void { + this.temporaries.set(from, to); + } + + declareProperty(from: Identifier, to: ReactiveScopeDependency): void { + this.properties.set(from, to); + } +} + +function collectNodes( + fn: HIRFunction, + usedOutsideDeclaringScope: Set, + c: TemporariesSideMap, +): Map { + const knownImmutableIdentifiers = new Set(); + if (fn.fnType === 'Component' || fn.fnType === 'Hook') { + for (const p of fn.params) { + if (p.kind === 'Identifier') { + knownImmutableIdentifiers.add(p.identifier); + } + } + } + const nodes = new Map(); + for (const [blockId, block] of fn.body.blocks) { + const assumedNonNullObjects = new Set(); + for (const instr of block.instructions) { + const {value, lvalue} = instr; + const usedOutside = usedOutsideDeclaringScope.has( + lvalue.identifier.declarationId, + ); + if (value.kind === 'PropertyLoad') { + const property = getProperty( + value.object, + value.property, + c.temporaries, + c.properties, + ); + if (!usedOutside) { + c.declareProperty(lvalue.identifier, property); + } + const propertyNode = c.tree.getPropertyLoadNode(property); + /** + * Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges + * are not valid with respect to current instruction id numbering. + * We use attached reactive scope ranges as a proxy for mutable range, but this + * is an overestimate as (1) scope ranges merge and align to form valid program + * blocks and (2) passes like MemoizeFbtAndMacroOperands may assign scopes to + * non-mutable identifiers. + * + * Due to current limitations of mutable range inference, there are edge cases in + * which we infer known-immutable values (e.g. props or hook params) to have a + * mutable range and scope. + * (see `destructure-array-declaration-to-context-var` fixture) + */ + const isMutableAtInstr = + value.object.identifier.mutableRange.end > + value.object.identifier.mutableRange.start + 1 && + value.object.identifier.scope != null && + inMutableRange(instr, value.object.identifier.scope.range); + if ( + !isMutableAtInstr || + knownImmutableIdentifiers.has(propertyNode.fullPath.identifier) + ) { + let curr = propertyNode.parent; + while (curr != null) { + assumedNonNullObjects.add(curr); + curr = curr.parent; + } + } + } else if (value.kind === 'LoadLocal') { + if ( + lvalue.identifier.name == null && + value.place.identifier.name !== null && + !usedOutside + ) { + c.declareTemporary(lvalue.identifier, value.place.identifier); + } + } + } + + nodes.set(blockId, { + block, + assumedNonNullObjects, + }); + } + return nodes; +} + +function deriveNonNull(fn: HIRFunction, nodes: Map): void { + const succ = new Map>(); + const terminalPreds = new Set(); + + for (const [blockId, block] of fn.body.blocks) { + for (const pred of block.preds) { + const predVal = getOrInsertDefault(succ, pred, new Set()); + predVal.add(blockId); + } + if (block.terminal.kind === 'throw' || block.terminal.kind === 'return') { + terminalPreds.add(blockId); + } + } + + function recursivelyDeriveNonNull( + nodeId: BlockId, + kind: 'succ' | 'pred', + traversalState: Map, + result: Map>, + ): boolean { + if (traversalState.has(nodeId)) { + return false; + } + traversalState.set(nodeId, 'active'); + + const node = nodes.get(nodeId); + if (node == null) { + CompilerError.invariant(false, { + reason: `Bad node ${nodeId}, kind: ${kind}`, + loc: GeneratedSource, + }); + } + const neighbors = Array.from( + kind === 'succ' ? (succ.get(nodeId) ?? []) : node.block.preds, + ); + + let changed = false; + for (const pred of neighbors) { + if (!traversalState.has(pred)) { + const neighborChanged = recursivelyDeriveNonNull( + pred, + kind, + traversalState, + result, + ); + changed ||= neighborChanged; + } + } + /** + * Active neighbors can be filtered out as we're solving for the following + * relation. + * X = Intersect(X_neighbors, X) + * Non-active neighbors with no recorded results can occur due to backedges. + * it's not safe to assume they can be filtered out (e.g. not intersected) + */ + const neighborAccesses = Set_intersect([ + ...(Array.from(neighbors) + .filter(n => traversalState.get(n) === 'done') + .map(n => result.get(n) ?? new Set()) as Array>), + ]); + + const prevSize = result.get(nodeId)?.size; + result.set(nodeId, Set_union(node.assumedNonNullObjects, neighborAccesses)); + traversalState.set(nodeId, 'done'); + + changed ||= prevSize !== result.get(nodeId)!.size; + CompilerError.invariant( + prevSize == null || prevSize <= result.get(nodeId)!.size, + { + reason: '[CollectHoistablePropertyLoads] Nodes shrank!', + description: `${nodeId} ${kind} ${prevSize} ${ + result.get(nodeId)!.size + }`, + loc: GeneratedSource, + }, + ); + return changed; + } + const fromEntry = new Map>(); + const fromExit = new Map>(); + let changed = true; + const traversalState = new Map(); + const reversedBlocks = [...fn.body.blocks]; + reversedBlocks.reverse(); + let i = 0; + + while (changed) { + i++; + changed = false; + for (const [blockId] of fn.body.blocks) { + const changed_ = recursivelyDeriveNonNull( + blockId, + 'pred', + traversalState, + fromEntry, + ); + changed ||= changed_; + } + traversalState.clear(); + for (const [blockId] of reversedBlocks) { + const changed_ = recursivelyDeriveNonNull( + blockId, + 'succ', + traversalState, + fromExit, + ); + changed ||= changed_; + } + traversalState.clear(); + } + + // TODO: I can't come up with a case that requires fixed-point iteration + CompilerError.invariant(i === 2, { + reason: 'require fixed-point iteration', + loc: GeneratedSource, + }); + + CompilerError.invariant( + fromEntry.size === fromExit.size && fromEntry.size === nodes.size, + { + reason: + 'bad sizes after calculating fromEntry + fromExit ' + + `${fromEntry.size} ${fromExit.size} ${nodes.size}`, + loc: GeneratedSource, + }, + ); + + for (const [id, node] of nodes) { + node.assumedNonNullObjects = Set_union( + assertNonNull(fromEntry.get(id)), + assertNonNull(fromExit.get(id)), + ); + } +} + +function assertNonNull, U>( + value: T | null | undefined, + source?: string, +): T { + CompilerError.invariant(value != null, { + reason: 'Unexpected null', + description: source != null ? `(from ${source})` : null, + loc: GeneratedSource, + }); + return value; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts new file mode 100644 index 0000000000000..a6a8a2d5e3de3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts @@ -0,0 +1,270 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {CompilerError} from '../CompilerError'; +import {GeneratedSource, Identifier, ReactiveScopeDependency} from '../HIR'; +import {printIdentifier} from '../HIR/PrintHIR'; +import {ReactiveScopePropertyDependency} from '../ReactiveScopes/DeriveMinimalDependencies'; + +const ENABLE_DEBUG_INVARIANTS = true; + +/** + * Simpler fork of DeriveMinimalDependencies, see PropagateScopeDependenciesHIR + * for detailed explanation. + */ +export class ReactiveScopeDependencyTreeHIR { + #roots: Map = new Map(); + + #getOrCreateRoot( + identifier: Identifier, + isHoistable: boolean, + ): DependencyNode { + // roots can always be accessed unconditionally in JS + let rootNode = this.#roots.get(identifier); + + if (rootNode === undefined) { + rootNode = { + properties: new Map(), + accessType: isHoistable + ? PropertyAccessType.HoistableAccess + : PropertyAccessType.Access, + }; + this.#roots.set(identifier, rootNode); + } + return rootNode; + } + + addDependency(dep: ReactiveScopePropertyDependency): void { + const {path} = dep; + let currNode = this.#getOrCreateRoot(dep.identifier, false); + + const accessType = PropertyAccessType.Access; + + currNode.accessType = merge(currNode.accessType, accessType); + + for (const property of path) { + // all properties read 'on the way' to a dependency are marked as 'access' + let currChild = getOrMakeProperty(currNode, property.property); + currChild.accessType = merge(currChild.accessType, accessType); + currNode = currChild; + } + + /* + * If this property does not have a conditional path (i.e. a.b.c), the + * final property node should be marked as an conditional/unconditional + * `dependency` as based on control flow. + */ + currNode.accessType = merge( + currNode.accessType, + PropertyAccessType.Dependency, + ); + } + + markNodesHoistable(dep: ReactiveScopePropertyDependency): void { + const accessType = PropertyAccessType.HoistableAccess; + let currNode = this.#roots.get(dep.identifier); + + let cursor = 0; + while (currNode != null && cursor < dep.path.length) { + currNode.accessType = merge(currNode.accessType, accessType); + currNode = currNode.properties.get(dep.path[cursor++].property); + } + if (currNode != null) { + currNode.accessType = merge(currNode.accessType, accessType); + } + } + + /** + * Derive a set of minimal dependencies that are safe to + * access unconditionally (with respect to nullthrows behavior) + */ + deriveMinimalDependencies(): Set { + const results = new Set(); + for (const [rootId, rootNode] of this.#roots.entries()) { + if (ENABLE_DEBUG_INVARIANTS) { + assertWellFormedTree(rootNode); + } + const deps = deriveMinimalDependenciesInSubtree(rootNode, []); + + for (const dep of deps) { + results.add({ + identifier: rootId, + path: dep.path.map(s => ({property: s, optional: false})), + }); + } + } + + return results; + } + + /* + * Prints dependency tree to string for debugging. + * @param includeAccesses + * @returns string representation of DependencyTree + */ + printDeps(includeAccesses: boolean): string { + let res: Array> = []; + + for (const [rootId, rootNode] of this.#roots.entries()) { + const rootResults = printSubtree(rootNode, includeAccesses).map( + result => `${printIdentifier(rootId)}.${result}`, + ); + res.push(rootResults); + } + return res.flat().join('\n'); + } +} + +enum PropertyAccessType { + Access = 'Access', + HoistableAccess = 'HoistableAccess', + Dependency = 'Dependency', + HoistableDependency = 'HoistableDependency', +} + +const MIN_ACCESS_TYPE = PropertyAccessType.Access; +/** + * "Hoistable" means that PropertyReads from a node are side-effect free. + * In other words, this means that control flow analysis shows that + * we can assume this node is a non-null object. + */ +function isHoistable(access: PropertyAccessType): boolean { + return ( + access === PropertyAccessType.HoistableAccess || + access === PropertyAccessType.HoistableDependency + ); +} +function isDependency(access: PropertyAccessType): boolean { + return ( + access === PropertyAccessType.Dependency || + access === PropertyAccessType.HoistableDependency + ); +} + +function merge( + access1: PropertyAccessType, + access2: PropertyAccessType, +): PropertyAccessType { + const resultisHoistable = isHoistable(access1) || isHoistable(access2); + const resultIsDependency = isDependency(access1) || isDependency(access2); + + /* + * Straightforward merge. + * This can be represented as bitwise OR, but is written out for readability + * + * Observe that `HoistableAccess | Dependency` produces an + * unconditionally accessed conditional dependency. We currently use these + * as we use unconditional dependencies. (i.e. to codegen change variables) + */ + if (resultisHoistable) { + if (resultIsDependency) { + return PropertyAccessType.HoistableDependency; + } else { + return PropertyAccessType.HoistableAccess; + } + } else { + if (resultIsDependency) { + return PropertyAccessType.Dependency; + } else { + return PropertyAccessType.Access; + } + } +} + +type DependencyNode = { + properties: Map; + accessType: PropertyAccessType; +}; + +type ReduceResultNode = { + path: Array; +}; + +function assertWellFormedTree(node: DependencyNode): void { + let hoistableInChildren = false; + for (const childNode of node.properties.values()) { + assertWellFormedTree(childNode); + hoistableInChildren ||= isHoistable(childNode.accessType); + } + if (hoistableInChildren) { + CompilerError.invariant(isHoistable(node.accessType), { + reason: + '[DeriveMinimialDependencies] Not well formed tree, unexpected hoistable node', + description: node.accessType, + loc: GeneratedSource, + }); + } +} + +function deriveMinimalDependenciesInSubtree( + node: DependencyNode, + path: Array, +): Array { + if (isDependency(node.accessType)) { + /** + * If this node is a dependency, we truncate the subtree + * and return this node. e.g. deps=[`obj.a`, `obj.a.b`] + * reduces to deps=[`obj.a`] + */ + return [{path}]; + } else { + if (isHoistable(node.accessType)) { + /* + * Only recurse into subtree dependencies if this node + * is known to be non-null. + */ + const result: Array = []; + for (const [childName, childNode] of node.properties) { + result.push( + ...deriveMinimalDependenciesInSubtree(childNode, [ + ...path, + childName, + ]), + ); + } + return result; + } else { + /* + * This only occurs when this subtree contains a dependency, + * but this node is potentially nullish. As we currently + * don't record optional property paths as scope dependencies, + * we truncate and record this node as a dependency. + */ + return [{path}]; + } + } +} + +function printSubtree( + node: DependencyNode, + includeAccesses: boolean, +): Array { + const results: Array = []; + for (const [propertyName, propertyNode] of node.properties) { + if (includeAccesses || isDependency(propertyNode.accessType)) { + results.push(`${propertyName} (${propertyNode.accessType})`); + } + const propertyResults = printSubtree(propertyNode, includeAccesses); + results.push(...propertyResults.map(result => `${propertyName}.${result}`)); + } + return results; +} + +function getOrMakeProperty( + node: DependencyNode, + property: string, +): DependencyNode { + let child = node.properties.get(property); + if (child == null) { + child = { + properties: new Map(), + accessType: MIN_ACCESS_TYPE, + }; + node.properties.set(property, child); + } + return child; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts new file mode 100644 index 0000000000000..20a9fbc01f544 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -0,0 +1,515 @@ +import { + ScopeId, + HIRFunction, + Place, + Instruction, + ReactiveScopeDependency, + Identifier, + ReactiveScope, + isObjectMethodType, + isRefValueType, + isUseRefType, + makeInstructionId, + InstructionId, + InstructionKind, + GeneratedSource, + DeclarationId, + areEqualPaths, +} from './HIR'; +import { + BlockInfo, + collectHoistablePropertyLoads, + getProperty, +} from './CollectHoistablePropertyLoads'; +import { + ScopeBlockTraversal, + eachInstructionOperand, + eachInstructionValueOperand, + eachPatternOperand, + eachTerminalOperand, +} from './visitors'; +import {Stack, empty} from '../Utils/Stack'; +import {CompilerError} from '../CompilerError'; +import {Iterable_some} from '../Utils/utils'; +import {ReactiveScopeDependencyTreeHIR} from './DeriveMinimalDependenciesHIR'; + +export function propagateScopeDependenciesHIR(fn: HIRFunction): void { + const usedOutsideDeclaringScope = + findTemporariesUsedOutsideDeclaringScope(fn); + + /** + * + */ + const {nodes, temporaries, properties} = collectHoistablePropertyLoads( + fn, + usedOutsideDeclaringScope, + ); + + const scopeDeps = collectDependencies( + fn, + usedOutsideDeclaringScope, + temporaries, + properties, + ); + + /** + * Derive the minimal set of hoistable dependencies for each scope. + */ + for (const [scope, deps] of scopeDeps) { + const tree = new ReactiveScopeDependencyTreeHIR(); + + /** + * Step 1: Add every dependency used by this scope (e.g. `a.b.c`) + */ + for (const dep of deps) { + tree.addDependency({...dep}); + } + /** + * Step 2: Mark hoistable dependencies, given the basic block in + * which the scope begins. + */ + recordHoistablePropertyReads(nodes, scope.id, tree); + const candidates = tree.deriveMinimalDependencies(); + for (const candidateDep of candidates) { + if ( + !Iterable_some( + scope.dependencies, + existingDep => + existingDep.identifier.declarationId === + candidateDep.identifier.declarationId && + areEqualPaths(existingDep.path, candidateDep.path), + ) + ) + scope.dependencies.add(candidateDep); + } + } +} + +function findTemporariesUsedOutsideDeclaringScope( + fn: HIRFunction, +): Set { + /* + * tracks all relevant LoadLocal and PropertyLoad lvalues + * and the scope where they are defined + */ + const declarations = new Map(); + const prunedScopes = new Set(); + const scopeTraversal = new ScopeBlockTraversal(); + const usedOutsideDeclaringScope = new Set(); + + function handlePlace(place: Place): void { + const declaringScope = declarations.get(place.identifier.declarationId); + if ( + declaringScope != null && + !scopeTraversal.isScopeActive(declaringScope) && + !prunedScopes.has(declaringScope) + ) { + // Declaring scope is not active === used outside declaring scope + usedOutsideDeclaringScope.add(place.identifier.declarationId); + } + } + + function handleInstruction(instr: Instruction): void { + const scope = scopeTraversal.currentScope; + if (scope == null || prunedScopes.has(scope)) { + return; + } + switch (instr.value.kind) { + case 'LoadLocal': + case 'LoadContext': + case 'PropertyLoad': { + declarations.set(instr.lvalue.identifier.declarationId, scope); + break; + } + default: { + break; + } + } + } + + for (const [blockId, block] of fn.body.blocks) { + scopeTraversal.recordScopes(block); + const scopeStartInfo = scopeTraversal.blockInfos.get(blockId); + if (scopeStartInfo?.kind === 'begin' && scopeStartInfo.pruned) { + prunedScopes.add(scopeStartInfo.scope.id); + } + for (const instr of block.instructions) { + for (const place of eachInstructionOperand(instr)) { + handlePlace(place); + } + handleInstruction(instr); + } + + for (const place of eachTerminalOperand(block.terminal)) { + handlePlace(place); + } + } + return usedOutsideDeclaringScope; +} + +type Decl = { + id: InstructionId; + scope: Stack; +}; + +class Context { + #temporariesUsedOutsideScope: Set; + #declarations: Map = new Map(); + #reassignments: Map = new Map(); + // Reactive dependencies used in the current reactive scope. + #dependencies: Stack> = empty(); + /* + * We keep a sidemap for temporaries created by PropertyLoads, and do + * not store any control flow (i.e. #inConditionalWithinScope) here. + * - a ReactiveScope (A) containing a PropertyLoad may differ from the + * ReactiveScope (B) that uses the produced temporary. + * - codegen will inline these PropertyLoads back into scope (B) + */ + #properties: ReadonlyMap; + #temporaries: ReadonlyMap; + #scopes: Stack = empty(); + deps: Map> = new Map(); + + constructor( + temporariesUsedOutsideScope: Set, + temporaries: Map, + properties: Map, + ) { + this.#temporariesUsedOutsideScope = temporariesUsedOutsideScope; + this.#temporaries = temporaries; + this.#properties = properties; + } + + enterScope(scope: ReactiveScope): void { + // Set context for new scope + this.#dependencies = this.#dependencies.push([]); + this.#scopes = this.#scopes.push(scope); + } + + exitScope(scope: ReactiveScope, pruned: boolean): void { + // Save dependencies we collected from the exiting scope + const scopedDependencies = this.#dependencies.value; + CompilerError.invariant(scopedDependencies != null, { + reason: '[PropagateScopeDeps]: Unexpected scope mismatch', + loc: scope.loc, + }); + + // Restore context of previous scope + this.#scopes = this.#scopes.pop(); + this.#dependencies = this.#dependencies.pop(); + + /* + * Collect dependencies we recorded for the exiting scope and propagate + * them upward using the same rules as normal dependency collection. + * Child scopes may have dependencies on values created within the outer + * scope, which necessarily cannot be dependencies of the outer scope. + */ + for (const dep of scopedDependencies) { + if (this.#checkValidDependency(dep)) { + this.#dependencies.value?.push(dep); + } + } + + if (!pruned) { + this.deps.set(scope, scopedDependencies); + } + } + + isUsedOutsideDeclaringScope(place: Place): boolean { + return this.#temporariesUsedOutsideScope.has( + place.identifier.declarationId, + ); + } + + /* + * Records where a value was declared, and optionally, the scope where the value originated from. + * This is later used to determine if a dependency should be added to a scope; if the current + * scope we are visiting is the same scope where the value originates, it can't be a dependency + * on itself. + */ + declare(identifier: Identifier, decl: Decl): void { + if (!this.#declarations.has(identifier.declarationId)) { + this.#declarations.set(identifier.declarationId, decl); + } + this.#reassignments.set(identifier, decl); + } + + resolveTemporary(place: Place): Identifier { + return this.#temporaries.get(place.identifier) ?? place.identifier; + } + + // Checks if identifier is a valid dependency in the current scope + #checkValidDependency(maybeDependency: ReactiveScopeDependency): boolean { + // ref.current access is not a valid dep + if ( + isUseRefType(maybeDependency.identifier) && + maybeDependency.path.at(0)?.property === 'current' + ) { + return false; + } + + // ref value is not a valid dep + if (isRefValueType(maybeDependency.identifier)) { + return false; + } + + /* + * object methods are not deps because they will be codegen'ed back in to + * the object literal. + */ + if (isObjectMethodType(maybeDependency.identifier)) { + return false; + } + + const identifier = maybeDependency.identifier; + /* + * If this operand is used in a scope, has a dynamic value, and was defined + * before this scope, then its a dependency of the scope. + */ + const currentDeclaration = + this.#reassignments.get(identifier) ?? + this.#declarations.get(identifier.declarationId); + const currentScope = this.currentScope.value; + return ( + currentScope != null && + currentDeclaration !== undefined && + currentDeclaration.id < currentScope.range.start + ); + } + + #isScopeActive(scope: ReactiveScope): boolean { + if (this.#scopes === null) { + return false; + } + return this.#scopes.find(state => state === scope); + } + + get currentScope(): Stack { + return this.#scopes; + } + + visitOperand(place: Place): void { + const resolved = this.resolveTemporary(place); + /* + * if this operand is a temporary created for a property load, try to resolve it to + * the expanded Place. Fall back to using the operand as-is. + */ + + let dependency: ReactiveScopeDependency | null = null; + if (resolved.name === null) { + const propertyDependency = this.#properties.get(resolved); + if (propertyDependency !== undefined) { + dependency = {...propertyDependency}; + } + } + this.visitDependency( + dependency ?? { + identifier: resolved, + path: [], + }, + ); + } + + visitProperty(object: Place, property: string): void { + const nextDependency = getProperty( + object, + property, + this.#temporaries, + this.#properties, + ); + this.visitDependency(nextDependency); + } + + visitDependency(maybeDependency: ReactiveScopeDependency): void { + /* + * Any value used after its originally defining scope has concluded must be added as an + * output of its defining scope. Regardless of whether its a const or not, + * some later code needs access to the value. If the current + * scope we are visiting is the same scope where the value originates, it can't be a dependency + * on itself. + */ + + /* + * if originalDeclaration is undefined here, then this is not a local var + * (all decls e.g. `let x;` should be initialized in BuildHIR) + */ + const originalDeclaration = this.#declarations.get( + maybeDependency.identifier.declarationId, + ); + if ( + originalDeclaration !== undefined && + originalDeclaration.scope.value !== null + ) { + originalDeclaration.scope.each(scope => { + if ( + !this.#isScopeActive(scope) && + !Iterable_some( + scope.declarations.values(), + decl => + decl.identifier.declarationId === + maybeDependency.identifier.declarationId, + ) + ) { + scope.declarations.set(maybeDependency.identifier.id, { + identifier: maybeDependency.identifier, + scope: originalDeclaration.scope.value!, + }); + } + }); + } + + if (this.#checkValidDependency(maybeDependency)) { + this.#dependencies.value!.push(maybeDependency); + } + } + + /* + * Record a variable that is declared in some other scope and that is being reassigned in the + * current one as a {@link ReactiveScope.reassignments} + */ + visitReassignment(place: Place): void { + const currentScope = this.currentScope.value; + if ( + currentScope != null && + !Iterable_some( + currentScope.reassignments, + identifier => + identifier.declarationId === place.identifier.declarationId, + ) && + this.#checkValidDependency({identifier: place.identifier, path: []}) + ) { + currentScope.reassignments.add(place.identifier); + } + } +} + +function handleInstruction(instr: Instruction, context: Context) { + const {id, value, lvalue} = instr; + if (value.kind === 'LoadLocal') { + if ( + value.place.identifier.name === null || + lvalue.identifier.name !== null || + context.isUsedOutsideDeclaringScope(lvalue) + ) { + context.visitOperand(value.place); + } + } else if (value.kind === 'PropertyLoad') { + if (context.isUsedOutsideDeclaringScope(lvalue)) { + context.visitProperty(value.object, value.property); + } + } else if (value.kind === 'StoreLocal') { + context.visitOperand(value.value); + if (value.lvalue.kind === InstructionKind.Reassign) { + context.visitReassignment(value.lvalue.place); + } + context.declare(value.lvalue.place.identifier, { + id, + scope: context.currentScope, + }); + } else if (value.kind === 'DeclareLocal' || value.kind === 'DeclareContext') { + /* + * Some variables may be declared and never initialized. We need + * to retain (and hoist) these declarations if they are included + * in a reactive scope. One approach is to simply add all `DeclareLocal`s + * as scope declarations. + */ + + /* + * We add context variable declarations here, not at `StoreContext`, since + * context Store / Loads are modeled as reads and mutates to the underlying + * variable reference (instead of through intermediate / inlined temporaries) + */ + context.declare(value.lvalue.place.identifier, { + id, + scope: context.currentScope, + }); + } else if (value.kind === 'Destructure') { + context.visitOperand(value.value); + for (const place of eachPatternOperand(value.lvalue.pattern)) { + if (value.lvalue.kind === InstructionKind.Reassign) { + context.visitReassignment(place); + } + context.declare(place.identifier, { + id, + scope: context.currentScope, + }); + } + } else { + for (const operand of eachInstructionValueOperand(value)) { + context.visitOperand(operand); + } + } + + context.declare(lvalue.identifier, { + id, + scope: context.currentScope, + }); +} + +function collectDependencies( + fn: HIRFunction, + usedOutsideDeclaringScope: Set, + temporaries: Map, + properties: Map, +) { + const context = new Context( + usedOutsideDeclaringScope, + temporaries, + properties, + ); + + for (const param of fn.params) { + if (param.kind === 'Identifier') { + context.declare(param.identifier, { + id: makeInstructionId(0), + scope: empty(), + }); + } else { + context.declare(param.place.identifier, { + id: makeInstructionId(0), + scope: empty(), + }); + } + } + + const scopeTraversal = new ScopeBlockTraversal(); + + for (const [blockId, block] of fn.body.blocks) { + scopeTraversal.recordScopes(block); + const scopeBlockInfo = scopeTraversal.blockInfos.get(blockId); + if (scopeBlockInfo?.kind === 'begin') { + context.enterScope(scopeBlockInfo.scope); + } else if (scopeBlockInfo?.kind === 'end') { + context.exitScope(scopeBlockInfo.scope, scopeBlockInfo?.pruned); + } + + for (const instr of block.instructions) { + handleInstruction(instr, context); + } + for (const place of eachTerminalOperand(block.terminal)) { + context.visitOperand(place); + } + } + return context.deps; +} + +/** + * Compute the set of hoistable property reads. + */ +function recordHoistablePropertyReads( + nodes: Map, + scopeId: ScopeId, + tree: ReactiveScopeDependencyTreeHIR, +): void { + const node = nodes.get(scopeId); + CompilerError.invariant(node != null, { + reason: '[PropagateScopeDependencies] Scope not found in tracked blocks', + loc: GeneratedSource, + }); + + for (const item of node.assumedNonNullObjects) { + tree.markNodesHoistable({ + ...item.fullPath, + }); + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts index 904b7a4038dec..eabc6e9edc25c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts @@ -6,7 +6,9 @@ */ import {assertExhaustive} from '../Utils/utils'; +import {CompilerError} from '..'; import { + BasicBlock, BlockId, Instruction, InstructionValue, @@ -14,7 +16,9 @@ import { Pattern, Place, ReactiveInstruction, + ReactiveScope, ReactiveValue, + ScopeId, SpreadPattern, Terminal, } from './HIR'; @@ -1149,3 +1153,72 @@ export function* eachTerminalOperand(terminal: Terminal): Iterable { } } } + +/** + * Helper class for traversing scope blocks in HIR-form. + */ +export class ScopeBlockTraversal { + // Live stack of active scopes + #activeScopes: Array = []; + blockInfos: Map< + BlockId, + | { + kind: 'end'; + scope: ReactiveScope; + pruned: boolean; + } + | { + kind: 'begin'; + scope: ReactiveScope; + pruned: boolean; + fallthrough: BlockId; + } + > = new Map(); + + recordScopes(block: BasicBlock): void { + const blockInfo = this.blockInfos.get(block.id); + if (blockInfo?.kind === 'begin') { + this.#activeScopes.push(blockInfo.scope.id); + } else if (blockInfo?.kind === 'end') { + const top = this.#activeScopes.at(-1); + CompilerError.invariant(blockInfo.scope.id === top, { + reason: + 'Expected traversed block fallthrough to match top-most active scope', + loc: block.instructions[0]?.loc ?? block.terminal.id, + }); + this.#activeScopes.pop(); + } + + if ( + block.terminal.kind === 'scope' || + block.terminal.kind === 'pruned-scope' + ) { + CompilerError.invariant( + !this.blockInfos.has(block.terminal.block) && + !this.blockInfos.has(block.terminal.fallthrough), + { + reason: 'Expected unique scope blocks and fallthroughs', + loc: block.terminal.loc, + }, + ); + this.blockInfos.set(block.terminal.block, { + kind: 'begin', + scope: block.terminal.scope, + pruned: block.terminal.kind === 'pruned-scope', + fallthrough: block.terminal.fallthrough, + }); + this.blockInfos.set(block.terminal.fallthrough, { + kind: 'end', + scope: block.terminal.scope, + pruned: block.terminal.kind === 'pruned-scope', + }); + } + } + + isScopeActive(scopeId: ScopeId): boolean { + return this.#activeScopes.indexOf(scopeId) !== -1; + } + get currentScope(): ScopeId | null { + return this.#activeScopes.at(-1) ?? null; + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts index 126772f591b41..1382591731b14 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -13,6 +13,7 @@ import { HIRFunction, Identifier, Instruction, + MutableRange, Place, ReactiveScope, makeInstructionId, @@ -191,6 +192,10 @@ export function isMutable({id}: Instruction, place: Place): boolean { return id >= range.start && id < range.end; } +export function inMutableRange({id}: Instruction, range: MutableRange): boolean { + return id >= range.start && id < range.end; +} + function mayAllocate(env: Environment, instruction: Instruction): boolean { const {value} = instruction; switch (value.kind) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts b/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts index 6e07e14a8d0e6..fec5ab6a96e50 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts @@ -84,15 +84,32 @@ export function getOrInsertDefault( } export function Set_union(a: Set, b: Set): Set { - const union = new Set(); - for (const item of a) { - if (b.has(item)) { - union.add(item); - } + const union = new Set(a); + for (const item of b) { + union.add(item); } return union; } +export function Set_intersect(sets: Array>): Set { + if (sets.length === 0 || sets.some(s => s.size === 0)) { + return new Set(); + } else if (sets.length === 1) { + return new Set(sets[0]); + } + const result: Set = new Set(); + const first = sets[0]; + outer: for (const e of first) { + for (let i = 1; i < sets.length; i++) { + if (!sets[i].has(e)) { + continue outer; + } + } + result.add(e); + } + return result; +} + export function Iterable_some( iter: Iterable, pred: (item: T) => boolean, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md index 76648c251a101..3f795b604e382 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md @@ -33,9 +33,14 @@ import { c as _c } from "react/compiler-runtime"; /** * props.b *does* influence `a` */ function Component(props) { - const $ = _c(2); + const $ = _c(5); let a; - if ($[0] !== props) { + if ( + $[0] !== props.a || + $[1] !== props.b || + $[2] !== props.c || + $[3] !== props.d + ) { a = []; a.push(props.a); bb0: { @@ -47,10 +52,13 @@ function Component(props) { } a.push(props.d); - $[0] = props; - $[1] = a; + $[0] = props.a; + $[1] = props.b; + $[2] = props.c; + $[3] = props.d; + $[4] = a; } else { - a = $[1]; + a = $[4]; } return a; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-early-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-early-return.expect.md index 82537902bfa19..5e708b95c6fe9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-early-return.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-early-return.expect.md @@ -70,10 +70,10 @@ import { c as _c } from "react/compiler-runtime"; /** * props.b does *not* influence `a` */ function ComponentA(props) { - const $ = _c(3); + const $ = _c(5); let a_DEBUG; let t0; - if ($[0] !== props) { + if ($[0] !== props.a || $[1] !== props.b || $[2] !== props.d) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { a_DEBUG = []; @@ -85,12 +85,14 @@ function ComponentA(props) { a_DEBUG.push(props.d); } - $[0] = props; - $[1] = a_DEBUG; - $[2] = t0; + $[0] = props.a; + $[1] = props.b; + $[2] = props.d; + $[3] = a_DEBUG; + $[4] = t0; } else { - a_DEBUG = $[1]; - t0 = $[2]; + a_DEBUG = $[3]; + t0 = $[4]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; @@ -102,9 +104,14 @@ function ComponentA(props) { * props.b *does* influence `a` */ function ComponentB(props) { - const $ = _c(2); + const $ = _c(5); let a; - if ($[0] !== props) { + if ( + $[0] !== props.a || + $[1] !== props.b || + $[2] !== props.c || + $[3] !== props.d + ) { a = []; a.push(props.a); if (props.b) { @@ -112,10 +119,13 @@ function ComponentB(props) { } a.push(props.d); - $[0] = props; - $[1] = a; + $[0] = props.a; + $[1] = props.b; + $[2] = props.c; + $[3] = props.d; + $[4] = a; } else { - a = $[1]; + a = $[4]; } return a; } @@ -124,10 +134,15 @@ function ComponentB(props) { * props.b *does* influence `a`, but only in a way that is never observable */ function ComponentC(props) { - const $ = _c(3); + const $ = _c(6); let a; let t0; - if ($[0] !== props) { + if ( + $[0] !== props.a || + $[1] !== props.b || + $[2] !== props.c || + $[3] !== props.d + ) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { a = []; @@ -140,12 +155,15 @@ function ComponentC(props) { a.push(props.d); } - $[0] = props; - $[1] = a; - $[2] = t0; + $[0] = props.a; + $[1] = props.b; + $[2] = props.c; + $[3] = props.d; + $[4] = a; + $[5] = t0; } else { - a = $[1]; - t0 = $[2]; + a = $[4]; + t0 = $[5]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; @@ -157,10 +175,15 @@ function ComponentC(props) { * props.b *does* influence `a` */ function ComponentD(props) { - const $ = _c(3); + const $ = _c(6); let a; let t0; - if ($[0] !== props) { + if ( + $[0] !== props.a || + $[1] !== props.b || + $[2] !== props.c || + $[3] !== props.d + ) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { a = []; @@ -173,12 +196,15 @@ function ComponentD(props) { a.push(props.d); } - $[0] = props; - $[1] = a; - $[2] = t0; + $[0] = props.a; + $[1] = props.b; + $[2] = props.c; + $[3] = props.d; + $[4] = a; + $[5] = t0; } else { - a = $[1]; - t0 = $[2]; + a = $[4]; + t0 = $[5]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-on-mutable.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-on-mutable.expect.md index ad638cf28d871..fa8348c200972 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-on-mutable.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/conditional-on-mutable.expect.md @@ -36,9 +36,9 @@ function mayMutate() {} ```javascript import { c as _c } from "react/compiler-runtime"; function ComponentA(props) { - const $ = _c(2); + const $ = _c(4); let t0; - if ($[0] !== props) { + if ($[0] !== props.p0 || $[1] !== props.p1 || $[2] !== props.p2) { const a = []; const b = []; if (b) { @@ -49,18 +49,20 @@ function ComponentA(props) { } t0 = ; - $[0] = props; - $[1] = t0; + $[0] = props.p0; + $[1] = props.p1; + $[2] = props.p2; + $[3] = t0; } else { - t0 = $[1]; + t0 = $[3]; } return t0; } function ComponentB(props) { - const $ = _c(2); + const $ = _c(4); let t0; - if ($[0] !== props) { + if ($[0] !== props.p0 || $[1] !== props.p1 || $[2] !== props.p2) { const a = []; const b = []; if (mayMutate(b)) { @@ -71,10 +73,12 @@ function ComponentB(props) { } t0 = ; - $[0] = props; - $[1] = t0; + $[0] = props.p0; + $[1] = props.p1; + $[2] = props.p2; + $[3] = t0; } else { - t0 = $[1]; + t0 = $[3]; } return t0; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md index 7febb3fecb6e7..1b016e3ec18d9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md @@ -30,7 +30,7 @@ import { identity } from "shared-runtime"; function Component(props) { const $ = _c(4); let x; - if ($[0] !== props.value) { + if ($[0] !== props) { const [t0] = props.value; x = t0; const foo = () => { @@ -38,7 +38,7 @@ function Component(props) { }; foo(); - $[0] = props.value; + $[0] = props; $[1] = x; } else { x = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md index fdb7203785716..26b56ea2a4f4d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md @@ -10,7 +10,7 @@ function Component(props) { x = identity(props.value[0]); }; foo(); - return {x}; + return
{x}
; } export const FIXTURE_ENTRYPOINT = { @@ -45,7 +45,7 @@ function Component(props) { const t0 = x; let t1; if ($[2] !== t0) { - t1 = { x: t0 }; + t1 =
{t0}
; $[2] = t0; $[3] = t1; } else { @@ -62,4 +62,4 @@ export const FIXTURE_ENTRYPOINT = { ``` ### Eval output -(kind: ok) {"x":42} \ No newline at end of file +(kind: ok)
42
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.js index 55675de9abb40..bc9324a35f06d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.js @@ -6,7 +6,7 @@ function Component(props) { x = identity(props.value[0]); }; foo(); - return {x}; + return
{x}
; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/early-return-nested-early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/early-return-nested-early-return-within-reactive-scope.expect.md index 2d33981f73fd1..5db4756ad3606 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/early-return-nested-early-return-within-reactive-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/early-return-nested-early-return-within-reactive-scope.expect.md @@ -31,9 +31,9 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(5); + const $ = _c(7); let t0; - if ($[0] !== props) { + if ($[0] !== props.cond || $[1] !== props.a || $[2] !== props.b) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { const x = []; @@ -41,12 +41,12 @@ function Component(props) { x.push(props.a); if (props.b) { let t1; - if ($[2] !== props.b) { + if ($[4] !== props.b) { t1 = [props.b]; - $[2] = props.b; - $[3] = t1; + $[4] = props.b; + $[5] = t1; } else { - t1 = $[3]; + t1 = $[5]; } const y = t1; x.push(y); @@ -58,20 +58,22 @@ function Component(props) { break bb0; } else { let t1; - if ($[4] === Symbol.for("react.memo_cache_sentinel")) { + if ($[6] === Symbol.for("react.memo_cache_sentinel")) { t1 = foo(); - $[4] = t1; + $[6] = t1; } else { - t1 = $[4]; + t1 = $[6]; } t0 = t1; break bb0; } } - $[0] = props; - $[1] = t0; + $[0] = props.cond; + $[1] = props.a; + $[2] = props.b; + $[3] = t0; } else { - t0 = $[1]; + t0 = $[3]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/early-return-within-reactive-scope.expect.md index 6c3525e9e77eb..42caf4e39b39e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/early-return-within-reactive-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/early-return-within-reactive-scope.expect.md @@ -45,9 +45,9 @@ import { c as _c } from "react/compiler-runtime"; import { makeArray } from "shared-runtime"; function Component(props) { - const $ = _c(4); + const $ = _c(6); let t0; - if ($[0] !== props) { + if ($[0] !== props.cond || $[1] !== props.a || $[2] !== props.b) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { const x = []; @@ -57,21 +57,23 @@ function Component(props) { break bb0; } else { let t1; - if ($[2] !== props.b) { + if ($[4] !== props.b) { t1 = makeArray(props.b); - $[2] = props.b; - $[3] = t1; + $[4] = props.b; + $[5] = t1; } else { - t1 = $[3]; + t1 = $[5]; } t0 = t1; break bb0; } } - $[0] = props; - $[1] = t0; + $[0] = props.cond; + $[1] = props.a; + $[2] = props.b; + $[3] = t0; } else { - t0 = $[1]; + t0 = $[3]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr-conditional-access-2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr-conditional-access-2.expect.md new file mode 100644 index 0000000000000..8cbaeb3f89465 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr-conditional-access-2.expect.md @@ -0,0 +1,67 @@ + +## Input + +```javascript +// @enableTreatFunctionDepsAsConditional @enablePropagateDepsInHIR:false +import {Stringify} from 'shared-runtime'; + +function Component({props}) { + const f = () => props.a.b; + + return {} : f} />; +} +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{props: null}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableTreatFunctionDepsAsConditional @enablePropagateDepsInHIR:false +import { Stringify } from "shared-runtime"; + +function Component(t0) { + const $ = _c(7); + const { props } = t0; + let t1; + if ($[0] !== props) { + t1 = () => props.a.b; + $[0] = props; + $[1] = t1; + } else { + t1 = $[1]; + } + const f = t1; + let t2; + if ($[2] !== props || $[3] !== f) { + t2 = props == null ? _temp : f; + $[2] = props; + $[3] = f; + $[4] = t2; + } else { + t2 = $[4]; + } + let t3; + if ($[5] !== t2) { + t3 = ; + $[5] = t2; + $[6] = t3; + } else { + t3 = $[6]; + } + return t3; +} +function _temp() {} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ props: null }], +}; + +``` + +### Eval output +(kind: ok)
{"f":"[[ function params=0 ]]"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr-conditional-access-2.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr-conditional-access-2.tsx new file mode 100644 index 0000000000000..2ede54db5f364 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr-conditional-access-2.tsx @@ -0,0 +1,12 @@ +// @enableTreatFunctionDepsAsConditional @enablePropagateDepsInHIR:false +import {Stringify} from 'shared-runtime'; + +function Component({props}) { + const f = () => props.a.b; + + return {} : f} />; +} +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{props: null}], +}; diff --git "a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr\342\200\223conditional-access.expect.md" "b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr\342\200\223conditional-access.expect.md" index eb05286fa29a5..f2fa20feb5477 100644 --- "a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr\342\200\223conditional-access.expect.md" +++ "b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr\342\200\223conditional-access.expect.md" @@ -2,7 +2,7 @@ ## Input ```javascript -// @enableTreatFunctionDepsAsConditional +// @enableTreatFunctionDepsAsConditional @enablePropagateDepsInHIR:false function Component(props) { function getLength() { return props.bar.length; @@ -21,7 +21,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @enableTreatFunctionDepsAsConditional +import { c as _c } from "react/compiler-runtime"; // @enableTreatFunctionDepsAsConditional @enablePropagateDepsInHIR:false function Component(props) { const $ = _c(5); let t0; diff --git "a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr\342\200\223conditional-access.js" "b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr\342\200\223conditional-access.js" index 6e59fb947d150..9bff3e5cdb53b 100644 --- "a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr\342\200\223conditional-access.js" +++ "b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/functionexpr\342\200\223conditional-access.js" @@ -1,4 +1,4 @@ -// @enableTreatFunctionDepsAsConditional +// @enableTreatFunctionDepsAsConditional @enablePropagateDepsInHIR:false function Component(props) { function getLength() { return props.bar.length; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/iife-return-modified-later-phi.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/iife-return-modified-later-phi.expect.md index bed1c329f0d10..a578e4a41d28f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/iife-return-modified-later-phi.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/iife-return-modified-later-phi.expect.md @@ -26,9 +26,9 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(2); + const $ = _c(3); let items; - if ($[0] !== props) { + if ($[0] !== props.cond || $[1] !== props.a) { let t0; if (props.cond) { t0 = []; @@ -38,10 +38,11 @@ function Component(props) { items = t0; items?.push(props.a); - $[0] = props; - $[1] = items; + $[0] = props.cond; + $[1] = props.a; + $[2] = items; } else { - items = $[1]; + items = $[2]; } return items; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-mutated-in-consequent-alternate-both-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-mutated-in-consequent-alternate-both-return.expect.md index 8a20f9186b447..b5534114c08a1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-mutated-in-consequent-alternate-both-return.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-mutated-in-consequent-alternate-both-return.expect.md @@ -29,9 +29,9 @@ import { c as _c } from "react/compiler-runtime"; import { makeObject_Primitives } from "shared-runtime"; function Component(props) { - const $ = _c(2); + const $ = _c(3); let t0; - if ($[0] !== props) { + if ($[0] !== props.cond || $[1] !== props.value) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { const object = makeObject_Primitives(); @@ -45,10 +45,11 @@ function Component(props) { break bb0; } } - $[0] = props; - $[1] = t0; + $[0] = props.cond; + $[1] = props.value; + $[2] = t0; } else { - t0 = $[1]; + t0 = $[2]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-optional-property-load.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-optional-property-load.expect.md index d95461adf90e6..795ab61cca997 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-optional-property-load.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/optional-call-with-optional-property-load.expect.md @@ -15,9 +15,9 @@ import { c as _c } from "react/compiler-runtime"; function Component(props) { const $ = _c(2); let t0; - if ($[0] !== props?.items) { + if ($[0] !== props) { t0 = props?.items?.map?.(render)?.filter(Boolean) ?? []; - $[0] = props?.items; + $[0] = props; $[1] = t0; } else { t0 = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/partial-early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/partial-early-return-within-reactive-scope.expect.md index 398161f0c6a24..266d87628c511 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/partial-early-return-within-reactive-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/partial-early-return-within-reactive-scope.expect.md @@ -30,10 +30,10 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(4); + const $ = _c(6); let y; let t0; - if ($[0] !== props) { + if ($[0] !== props.cond || $[1] !== props.a || $[2] !== props.b) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { const x = []; @@ -43,11 +43,11 @@ function Component(props) { break bb0; } else { let t1; - if ($[3] === Symbol.for("react.memo_cache_sentinel")) { + if ($[5] === Symbol.for("react.memo_cache_sentinel")) { t1 = foo(); - $[3] = t1; + $[5] = t1; } else { - t1 = $[3]; + t1 = $[5]; } y = t1; if (props.b) { @@ -56,12 +56,14 @@ function Component(props) { } } } - $[0] = props; - $[1] = y; - $[2] = t0; + $[0] = props.cond; + $[1] = props.a; + $[2] = props.b; + $[3] = y; + $[4] = t0; } else { - y = $[1]; - t0 = $[2]; + y = $[3]; + t0 = $[4]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-array-push-consecutive-phis.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-array-push-consecutive-phis.expect.md index f17bcc92cb7ce..16edbf2e23690 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-array-push-consecutive-phis.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-array-push-consecutive-phis.expect.md @@ -49,7 +49,7 @@ import { c as _c } from "react/compiler-runtime"; import { makeArray } from "shared-runtime"; function Component(props) { - const $ = _c(3); + const $ = _c(6); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = {}; @@ -59,7 +59,12 @@ function Component(props) { } const x = t0; let t1; - if ($[1] !== props) { + if ( + $[1] !== props.cond || + $[2] !== props.cond2 || + $[3] !== props.value || + $[4] !== props.value2 + ) { let y; if (props.cond) { if (props.cond2) { @@ -74,10 +79,13 @@ function Component(props) { y.push(x); t1 = [x, y]; - $[1] = props; - $[2] = t1; + $[1] = props.cond; + $[2] = props.cond2; + $[3] = props.value; + $[4] = props.value2; + $[5] = t1; } else { - t1 = $[2]; + t1 = $[5]; } return t1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-array-push.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-array-push.expect.md index f58eed10fda5a..58e2c8f869a6a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-array-push.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-array-push.expect.md @@ -36,7 +36,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(3); + const $ = _c(4); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = {}; @@ -46,7 +46,7 @@ function Component(props) { } const x = t0; let t1; - if ($[1] !== props) { + if ($[1] !== props.cond || $[2] !== props.value) { let y; if (props.cond) { y = [props.value]; @@ -57,10 +57,11 @@ function Component(props) { y.push(x); t1 = [x, y]; - $[1] = props; - $[2] = t1; + $[1] = props.cond; + $[2] = props.value; + $[3] = t1; } else { - t1 = $[2]; + t1 = $[3]; } return t1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-property-store.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-property-store.expect.md index 70551c8e9d7b0..641711e893884 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-property-store.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-property-store.expect.md @@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; // @debug function Component(props) { - const $ = _c(3); + const $ = _c(4); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = {}; @@ -42,7 +42,7 @@ function Component(props) { } const x = t0; let t1; - if ($[1] !== props) { + if ($[1] !== props.cond || $[2] !== props.a) { let y; if (props.cond) { y = {}; @@ -53,10 +53,11 @@ function Component(props) { y.x = x; t1 = [x, y]; - $[1] = props; - $[2] = t1; + $[1] = props.cond; + $[2] = props.a; + $[3] = t1; } else { - t1 = $[2]; + t1 = $[3]; } return t1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md index 25d7bf5f7ccce..0cda150692955 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-infer-less-specific-conditional-access.expect.md @@ -44,6 +44,8 @@ function Component({propA, propB}) { | ^^^^^^^^^^^^^^^^^ > 14 | }, [propA?.a, propB.x.y]); | ^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (6:14) + +CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (6:14) 15 | } 16 | ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md index 2dd61732f689f..59b48898fda16 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/conditional-member-expr.expect.md @@ -29,10 +29,10 @@ import { c as _c } from "react/compiler-runtime"; // To preserve the nullthrows function Component(props) { const $ = _c(2); let x; - if ($[0] !== props.a?.b) { + if ($[0] !== props.a) { x = []; x.push(props.a?.b); - $[0] = props.a?.b; + $[0] = props.a; $[1] = x; } else { x = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.expect.md new file mode 100644 index 0000000000000..0115f068a855d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.expect.md @@ -0,0 +1,107 @@ + +## Input + +```javascript +import { + makeObject_Primitives, + setPropertyByKey, +} from 'shared-runtime'; + +function useFoo({value, cond}) { + let x: any = makeObject_Primitives(); + if (cond) { + setPropertyByKey(x, 'a', null); + } else { + setPropertyByKey(x, 'a', {b: 2}); + } + + /** + * y should take a dependency on `x`, not `x.a.b` here + */ + const y = []; + if (!cond) { + y.push(x.a.b); + } + + x = makeObject_Primitives(); + setPropertyByKey(x, 'a', {b: value}); + + return [y, x.a.b]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{value:3,cond: true}], + sequentialRenders: [{value:3,cond: true}, {value:3,cond: false}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { makeObject_Primitives, setPropertyByKey } from "shared-runtime"; + +function useFoo(t0) { + const $ = _c(10); + const { value, cond } = t0; + let x; + if ($[0] !== cond) { + x = makeObject_Primitives(); + if (cond) { + setPropertyByKey(x, "a", null); + } else { + setPropertyByKey(x, "a", { b: 2 }); + } + $[0] = cond; + $[1] = x; + } else { + x = $[1]; + } + let y; + if ($[2] !== cond || $[3] !== x) { + y = []; + if (!cond) { + y.push(x.a.b); + } + $[2] = cond; + $[3] = x; + $[4] = y; + } else { + y = $[4]; + } + if ($[5] !== value) { + x = makeObject_Primitives(); + setPropertyByKey(x, "a", { b: value }); + $[5] = value; + $[6] = x; + } else { + x = $[6]; + } + let t1; + if ($[7] !== y || $[8] !== x.a.b) { + t1 = [y, x.a.b]; + $[7] = y; + $[8] = x.a.b; + $[9] = t1; + } else { + t1 = $[9]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ value: 3, cond: true }], + sequentialRenders: [ + { value: 3, cond: true }, + { value: 3, cond: false }, + ], +}; + +``` + +### Eval output +(kind: ok) [[],3] +[[2],3] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.tsx new file mode 100644 index 0000000000000..3cfcd7da5c371 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.tsx @@ -0,0 +1,32 @@ +import { + makeObject_Primitives, + setPropertyByKey, +} from 'shared-runtime'; + +function useFoo({value, cond}) { + let x: any = makeObject_Primitives(); + if (cond) { + setPropertyByKey(x, 'a', null); + } else { + setPropertyByKey(x, 'a', {b: 2}); + } + + /** + * y should take a dependency on `x`, not `x.a.b` here + */ + const y = []; + if (!cond) { + y.push(x.a.b); + } + + x = makeObject_Primitives(); + setPropertyByKey(x, 'a', {b: value}); + + return [y, x.a.b]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{value:3,cond: true}], + sequentialRenders: [{value:3,cond: true}, {value:3,cond: false}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.expect.md new file mode 100644 index 0000000000000..745df97b6da76 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.expect.md @@ -0,0 +1,96 @@ + +## Input + +```javascript +import { identity, shallowCopy, Stringify, useIdentity } from "shared-runtime"; + +type HasA = {kind: "hasA", a: {value: number}}; +type HasC = {kind: "hasC", c: {value: number}}; +function Foo({ cond }: {cond: boolean}) { + let x: HasA | HasC = shallowCopy({kind: "hasA", a: {value: 2}}); + /** + * This read of x.a.value is outside of x's identifier mutable + * range + scope range. We mark this ssa instance (x_@0) as having + * a non-null object property `x.a`. + */ + Math.max(x.a.value, 2); + if (cond) { + x = shallowCopy({kind: "hasC", c: {value: 3}}); + } + + /** + * Since this x (x_@2 = phi(x_@0, x_@1)) is a different ssa instance, + * we cannot safely hoist a read of `x.a.value` + */ + return ; +} +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{cond: false}], + sequentialRenders: [{cond: false}, {cond: true}] +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { identity, shallowCopy, Stringify, useIdentity } from "shared-runtime"; + +type HasA = { kind: "hasA"; a: { value: number } }; +type HasC = { kind: "hasC"; c: { value: number } }; +function Foo(t0) { + const $ = _c(7); + const { cond } = t0; + let t1; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t1 = shallowCopy({ kind: "hasA", a: { value: 2 } }); + $[0] = t1; + } else { + t1 = $[0]; + } + let x = t1; + + Math.max(x.a.value, 2); + if (cond) { + let t2; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t2 = shallowCopy({ kind: "hasC", c: { value: 3 } }); + $[1] = t2; + } else { + t2 = $[1]; + } + x = t2; + } + let t2; + if ($[2] !== cond || $[3] !== x) { + t2 = !cond && [(x as HasA).a.value + 2]; + $[2] = cond; + $[3] = x; + $[4] = t2; + } else { + t2 = $[4]; + } + let t3; + if ($[5] !== t2) { + t3 = ; + $[5] = t2; + $[6] = t3; + } else { + t3 = $[6]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ cond: false }], + sequentialRenders: [{ cond: false }, { cond: true }], +}; + +``` + +### Eval output +(kind: ok)
{"val":[4]}
+
{"val":false}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.tsx new file mode 100644 index 0000000000000..2ac73b25f8f5e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.tsx @@ -0,0 +1,27 @@ +import { identity, shallowCopy, Stringify, useIdentity } from "shared-runtime"; + +type HasA = {kind: "hasA", a: {value: number}}; +type HasC = {kind: "hasC", c: {value: number}}; +function Foo({ cond }: {cond: boolean}) { + let x: HasA | HasC = shallowCopy({kind: "hasA", a: {value: 2}}); + /** + * This read of x.a.value is outside of x's identifier mutable + * range + scope range. We mark this ssa instance (x_@0) as having + * a non-null object property `x.a`. + */ + Math.max(x.a.value, 2); + if (cond) { + x = shallowCopy({kind: "hasC", c: {value: 3}}); + } + + /** + * Since this x (x_@2 = phi(x_@0, x_@1)) is a different ssa instance, + * we cannot safely hoist a read of `x.a.value` + */ + return ; +} +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{cond: false}], + sequentialRenders: [{cond: false}, {cond: true}] +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/join-uncond-scopes-cond-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/join-uncond-scopes-cond-deps.expect.md index c54d0828ecefe..37d347cd9a0b4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/join-uncond-scopes-cond-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/join-uncond-scopes-cond-deps.expect.md @@ -61,20 +61,13 @@ import { c as _c } from "react/compiler-runtime"; // This tests an optimization, import { CONST_TRUE, setProperty } from "shared-runtime"; function useJoinCondDepsInUncondScopes(props) { - const $ = _c(4); + const $ = _c(2); let t0; if ($[0] !== props.a.b) { const y = {}; - let x; - if ($[2] !== props) { - x = {}; - if (CONST_TRUE) { - setProperty(x, props.a.b); - } - $[2] = props; - $[3] = x; - } else { - x = $[3]; + const x = {}; + if (CONST_TRUE) { + setProperty(x, props.a.b); } setProperty(y, props.a.b); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md index a66540655ab79..7362cd8317091 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain.expect.md @@ -44,11 +44,11 @@ import { c as _c } from "react/compiler-runtime"; // To preserve the nullthrows function Component(props) { const $ = _c(2); let x; - if ($[0] !== props.a.b) { + if ($[0] !== props.a) { x = []; x.push(props.a?.b); x.push(props.a.b.c); - $[0] = props.a.b; + $[0] = props.a; $[1] = x; } else { x = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md index 8d69c008c573b..f25ea2a31e552 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/memberexpr-join-optional-chain2.expect.md @@ -21,25 +21,16 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(5); + const $ = _c(2); let x; - if ($[0] !== props.items?.length || $[1] !== props.items?.edges) { + if ($[0] !== props.items) { x = []; x.push(props.items?.length); - let t0; - if ($[3] !== props.items?.edges) { - t0 = props.items?.edges?.map?.(render)?.filter?.(Boolean) ?? []; - $[3] = props.items?.edges; - $[4] = t0; - } else { - t0 = $[4]; - } - x.push(t0); - $[0] = props.items?.length; - $[1] = props.items?.edges; - $[2] = x; + x.push(props.items?.edges?.map?.(render)?.filter?.(Boolean) ?? []); + $[0] = props.items; + $[1] = x; } else { - x = $[2]; + x = $[1]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/promote-uncond.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/promote-uncond.expect.md index 09806d8b4b15d..9186ec84d6e21 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/promote-uncond.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/promote-uncond.expect.md @@ -34,19 +34,20 @@ import { identity } from "shared-runtime"; // and promote it to an unconditional dependency. function usePromoteUnconditionalAccessToDependency(props, other) { - const $ = _c(3); + const $ = _c(4); let x; - if ($[0] !== props.a || $[1] !== other) { + if ($[0] !== props.a.a.a || $[1] !== props.a.b || $[2] !== other) { x = {}; x.a = props.a.a.a; if (identity(other)) { x.c = props.a.b.c; } - $[0] = props.a; - $[1] = other; - $[2] = x; + $[0] = props.a.a.a; + $[1] = props.a.b; + $[2] = other; + $[3] = x; } else { - x = $[2]; + x = $[3]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.expect.md new file mode 100644 index 0000000000000..49d872e773a1c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.expect.md @@ -0,0 +1,110 @@ + +## Input + +```javascript +import { identity, makeObject_Primitives, setPropertyByKey } from "shared-runtime"; + +/** + * A bit of an edge case, but we could further optimize here by merging + * re-orderability of nodes across phis. + */ +function useFoo(cond) { + let x; + if (cond) { + /** start of scope for x_@0 */ + x = {}; + setPropertyByKey(x, 'a', {b: 2}); + /** end of scope for x_@0 */ + Math.max(x.a.b, 0); + } else { + /** start of scope for x_@1 */ + x = makeObject_Primitives(); + setPropertyByKey(x, 'a', {b: 3}); + /** end of scope for x_@1 */ + Math.max(x.a.b, 0); + } + /** + * At this point, we have a phi node. + * x_@2 = phi(x_@0, x_@1) + * + * We can assume that both x_@0 and x_@1 both have non-null `x.a` properties, + * so we can infer that x_@2 does as well. + */ + + // Here, y should take a dependency on `x.a.b` + const y = []; + if (identity(cond)) { + y.push(x.a.b); + } + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [true] +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { + identity, + makeObject_Primitives, + setPropertyByKey, +} from "shared-runtime"; + +/** + * A bit of an edge case, but we could further optimize here by merging + * re-orderability of nodes across phis. + */ +function useFoo(cond) { + const $ = _c(5); + let x; + if (cond) { + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + x = {}; + setPropertyByKey(x, "a", { b: 2 }); + $[0] = x; + } else { + x = $[0]; + } + + Math.max(x.a.b, 0); + } else { + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + x = makeObject_Primitives(); + setPropertyByKey(x, "a", { b: 3 }); + $[1] = x; + } else { + x = $[1]; + } + + Math.max(x.a.b, 0); + } + let y; + if ($[2] !== cond || $[3] !== x) { + y = []; + if (identity(cond)) { + y.push(x.a.b); + } + $[2] = cond; + $[3] = x; + $[4] = y; + } else { + y = $[4]; + } + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [true], +}; + +``` + +### Eval output +(kind: ok) [2] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.ts new file mode 100644 index 0000000000000..bdcf9b7b0ec26 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.ts @@ -0,0 +1,41 @@ +import { identity, makeObject_Primitives, setPropertyByKey } from "shared-runtime"; + +/** + * A bit of an edge case, but we could further optimize here by merging + * re-orderability of nodes across phis. + */ +function useFoo(cond) { + let x; + if (cond) { + /** start of scope for x_@0 */ + x = {}; + setPropertyByKey(x, 'a', {b: 2}); + /** end of scope for x_@0 */ + Math.max(x.a.b, 0); + } else { + /** start of scope for x_@1 */ + x = makeObject_Primitives(); + setPropertyByKey(x, 'a', {b: 3}); + /** end of scope for x_@1 */ + Math.max(x.a.b, 0); + } + /** + * At this point, we have a phi node. + * x_@2 = phi(x_@0, x_@1) + * + * We can assume that both x_@0 and x_@1 both have non-null `x.a` properties, + * so we can infer that x_@2 does as well. + */ + + // Here, y should take a dependency on `x.a.b` + const y = []; + if (identity(cond)) { + y.push(x.a.b); + } + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [true] +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/uncond-access-in-mutable-range.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/uncond-access-in-mutable-range.expect.md new file mode 100644 index 0000000000000..34979e9de9485 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/uncond-access-in-mutable-range.expect.md @@ -0,0 +1,105 @@ + +## Input + +```javascript +// x.a.b was accessed unconditionally within the mutable range of x. +// As a result, we cannot infer anything about whether `x` or `x.a` +// may be null. This means that it's not safe to hoist reads from x +// (e.g. take `x.a` or `x.a.b` as a dependency). + +import {identity, makeObject_Primitives, setProperty} from 'shared-runtime'; + +function Component({cond, other}) { + const x = makeObject_Primitives(); + setProperty(x, {b: 3, other}, 'a'); + identity(x.a.b); + if (!cond) { + x.a = null; + } + + const y = [identity(cond) && x.a.b]; + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false}], + sequentialRenders: [ + {cond: false}, + {cond: false}, + {cond: false, other: 8}, + {cond: true}, + {cond: true}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // x.a.b was accessed unconditionally within the mutable range of x. +// As a result, we cannot infer anything about whether `x` or `x.a` +// may be null. This means that it's not safe to hoist reads from x +// (e.g. take `x.a` or `x.a.b` as a dependency). + +import { identity, makeObject_Primitives, setProperty } from "shared-runtime"; + +function Component(t0) { + const $ = _c(8); + const { cond, other } = t0; + let x; + if ($[0] !== other || $[1] !== cond) { + x = makeObject_Primitives(); + setProperty(x, { b: 3, other }, "a"); + identity(x.a.b); + if (!cond) { + x.a = null; + } + $[0] = other; + $[1] = cond; + $[2] = x; + } else { + x = $[2]; + } + let t1; + if ($[3] !== cond || $[4] !== x) { + t1 = identity(cond) && x.a.b; + $[3] = cond; + $[4] = x; + $[5] = t1; + } else { + t1 = $[5]; + } + let t2; + if ($[6] !== t1) { + t2 = [t1]; + $[6] = t1; + $[7] = t2; + } else { + t2 = $[7]; + } + const y = t2; + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: false }], + sequentialRenders: [ + { cond: false }, + { cond: false }, + { cond: false, other: 8 }, + { cond: true }, + { cond: true }, + ], +}; + +``` + +### Eval output +(kind: ok) [false] +[false] +[false] +[null] +[null] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/uncond-access-in-mutable-range.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/uncond-access-in-mutable-range.js new file mode 100644 index 0000000000000..c4e4819f6b8fe --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/uncond-access-in-mutable-range.js @@ -0,0 +1,30 @@ +// x.a.b was accessed unconditionally within the mutable range of x. +// As a result, we cannot infer anything about whether `x` or `x.a` +// may be null. This means that it's not safe to hoist reads from x +// (e.g. take `x.a` or `x.a.b` as a dependency). + +import {identity, makeObject_Primitives, setProperty} from 'shared-runtime'; + +function Component({cond, other}) { + const x = makeObject_Primitives(); + setProperty(x, {b: 3, other}, 'a'); + identity(x.a.b); + if (!cond) { + x.a = null; + } + + const y = [identity(cond) && x.a.b]; + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false}], + sequentialRenders: [ + {cond: false}, + {cond: false}, + {cond: false, other: 8}, + {cond: true}, + {cond: true}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md index d8e59c486a55b..2ce8ffbe4c92e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md @@ -23,14 +23,14 @@ function HomeDiscoStoreItemTileRating(props) { const $ = _c(4); const item = useFragment(); let count; - if ($[0] !== item?.aggregates) { + if ($[0] !== item) { count = 0; const aggregates = item?.aggregates || []; aggregates.forEach((aggregate) => { count = count + (aggregate.count || 0); count; }); - $[0] = item?.aggregates; + $[0] = item; $[1] = count; } else { count = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-cascading-eliminated-phis.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-cascading-eliminated-phis.expect.md index cd123dbdba56e..a2cef3d876bf4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-cascading-eliminated-phis.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-cascading-eliminated-phis.expect.md @@ -31,10 +31,16 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(4); + const $ = _c(7); let x = 0; let values; - if ($[0] !== props || $[1] !== x) { + if ( + $[0] !== props.a || + $[1] !== props.b || + $[2] !== props.c || + $[3] !== props.d || + $[4] !== x + ) { values = []; const y = props.a || props.b; values.push(y); @@ -48,13 +54,16 @@ function Component(props) { } values.push(x); - $[0] = props; - $[1] = x; - $[2] = values; - $[3] = x; + $[0] = props.a; + $[1] = props.b; + $[2] = props.c; + $[3] = props.d; + $[4] = x; + $[5] = values; + $[6] = x; } else { - values = $[2]; - x = $[3]; + values = $[5]; + x = $[6]; } return values; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-leave-case.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-leave-case.expect.md index a7389041bb12d..851e45da94089 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-leave-case.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-leave-case.expect.md @@ -24,9 +24,9 @@ function Component(props) { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(2); + const $ = _c(3); let t0; - if ($[0] !== props) { + if ($[0] !== props.p0 || $[1] !== props.p1) { const x = []; let y; if (props.p0) { @@ -40,10 +40,11 @@ function Component(props) { {y} ); - $[0] = props; - $[1] = t0; + $[0] = props.p0; + $[1] = props.p1; + $[2] = t0; } else { - t0 = $[1]; + t0 = $[2]; } return t0; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-destruction-with-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-destruction-with-mutation.expect.md index 9ce43a45b4c22..5e3fe9f8d2494 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-destruction-with-mutation.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-destruction-with-mutation.expect.md @@ -17,17 +17,19 @@ function foo(props) { ```javascript import { c as _c } from "react/compiler-runtime"; function foo(props) { - const $ = _c(2); + const $ = _c(4); let x; - if ($[0] !== props) { + if ($[0] !== props.bar || $[1] !== props.cond || $[2] !== props.foo) { x = []; x.push(props.bar); props.cond ? (([x] = [[]]), x.push(props.foo)) : null; mut(x); - $[0] = props; - $[1] = x; + $[0] = props.bar; + $[1] = props.cond; + $[2] = props.foo; + $[3] = x; } else { - x = $[1]; + x = $[3]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-destruction.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-destruction.expect.md index e20dc2606fa37..aac49f168cfdb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-destruction.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-destruction.expect.md @@ -22,7 +22,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function foo(props) { - const $ = _c(4); + const $ = _c(5); let x; if ($[0] !== props.bar) { x = []; @@ -32,12 +32,13 @@ function foo(props) { } else { x = $[1]; } - if ($[2] !== props) { + if ($[2] !== props.cond || $[3] !== props.foo) { props.cond ? (([x] = [[]]), x.push(props.foo)) : null; - $[2] = props; - $[3] = x; + $[2] = props.cond; + $[3] = props.foo; + $[4] = x; } else { - x = $[3]; + x = $[4]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-with-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-with-mutation.expect.md index 3ec90ca8879bc..06c398c3130cc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-with-mutation.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary-with-mutation.expect.md @@ -17,17 +17,19 @@ function foo(props) { ```javascript import { c as _c } from "react/compiler-runtime"; function foo(props) { - const $ = _c(2); + const $ = _c(4); let x; - if ($[0] !== props) { + if ($[0] !== props.bar || $[1] !== props.cond || $[2] !== props.foo) { x = []; x.push(props.bar); props.cond ? ((x = []), x.push(props.foo)) : null; mut(x); - $[0] = props; - $[1] = x; + $[0] = props.bar; + $[1] = props.cond; + $[2] = props.foo; + $[3] = x; } else { - x = $[1]; + x = $[3]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary.expect.md index 1b9453341153b..369931dcbde97 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-ternary.expect.md @@ -22,7 +22,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function foo(props) { - const $ = _c(4); + const $ = _c(5); let x; if ($[0] !== props.bar) { x = []; @@ -32,12 +32,13 @@ function foo(props) { } else { x = $[1]; } - if ($[2] !== props) { + if ($[2] !== props.cond || $[3] !== props.foo) { props.cond ? ((x = []), x.push(props.foo)) : null; - $[2] = props; - $[3] = x; + $[2] = props.cond; + $[3] = props.foo; + $[4] = x; } else { - x = $[3]; + x = $[4]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.expect.md index bd77cc9c173a0..8ec5a3079843e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary-with-mutation.expect.md @@ -31,17 +31,19 @@ export const FIXTURE_ENTRYPOINT = { import { c as _c } from "react/compiler-runtime"; import { arrayPush } from "shared-runtime"; function foo(props) { - const $ = _c(2); + const $ = _c(4); let x; - if ($[0] !== props) { + if ($[0] !== props.bar || $[1] !== props.cond || $[2] !== props.foo) { x = []; x.push(props.bar); props.cond ? ((x = []), x.push(props.foo)) : ((x = []), x.push(props.bar)); arrayPush(x, 4); - $[0] = props; - $[1] = x; + $[0] = props.bar; + $[1] = props.cond; + $[2] = props.foo; + $[3] = x; } else { - x = $[1]; + x = $[3]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary.expect.md index b518df5ab8e8f..ad118193a4a24 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-ternary.expect.md @@ -24,7 +24,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function foo(props) { - const $ = _c(4); + const $ = _c(6); let x; if ($[0] !== props.bar) { x = []; @@ -34,12 +34,14 @@ function foo(props) { } else { x = $[1]; } - if ($[2] !== props) { + if ($[2] !== props.cond || $[3] !== props.foo || $[4] !== props.bar) { props.cond ? ((x = []), x.push(props.foo)) : ((x = []), x.push(props.bar)); - $[2] = props; - $[3] = x; + $[2] = props.cond; + $[3] = props.foo; + $[4] = props.bar; + $[5] = x; } else { - x = $[3]; + x = $[5]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-with-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-with-mutation.expect.md index e27e00db70f05..3911d2f0f8b4b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-with-mutation.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-unconditional-with-mutation.expect.md @@ -25,9 +25,9 @@ function foo(props) { ```javascript import { c as _c } from "react/compiler-runtime"; function foo(props) { - const $ = _c(2); + const $ = _c(4); let x; - if ($[0] !== props) { + if ($[0] !== props.bar || $[1] !== props.cond || $[2] !== props.foo) { x = []; x.push(props.bar); if (props.cond) { @@ -39,10 +39,12 @@ function foo(props) { } mut(x); - $[0] = props; - $[1] = x; + $[0] = props.bar; + $[1] = props.cond; + $[2] = props.foo; + $[3] = x; } else { - x = $[1]; + x = $[3]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-via-destructuring-with-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-via-destructuring-with-mutation.expect.md index 7ad946b2f494f..8af9325b4ccf1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-via-destructuring-with-mutation.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-via-destructuring-with-mutation.expect.md @@ -21,9 +21,9 @@ function foo(props) { ```javascript import { c as _c } from "react/compiler-runtime"; function foo(props) { - const $ = _c(2); + const $ = _c(4); let x; - if ($[0] !== props) { + if ($[0] !== props.bar || $[1] !== props.cond || $[2] !== props.foo) { ({ x } = { x: [] }); x.push(props.bar); if (props.cond) { @@ -32,10 +32,12 @@ function foo(props) { } mut(x); - $[0] = props; - $[1] = x; + $[0] = props.bar; + $[1] = props.cond; + $[2] = props.foo; + $[3] = x; } else { - x = $[1]; + x = $[3]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-with-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-with-mutation.expect.md index 76ccbae06a937..12758a0f1c287 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-with-mutation.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-renaming-with-mutation.expect.md @@ -21,9 +21,9 @@ function foo(props) { ```javascript import { c as _c } from "react/compiler-runtime"; function foo(props) { - const $ = _c(2); + const $ = _c(4); let x; - if ($[0] !== props) { + if ($[0] !== props.bar || $[1] !== props.cond || $[2] !== props.foo) { x = []; x.push(props.bar); if (props.cond) { @@ -32,10 +32,12 @@ function foo(props) { } mut(x); - $[0] = props; - $[1] = x; + $[0] = props.bar; + $[1] = props.cond; + $[2] = props.foo; + $[3] = x; } else { - x = $[1]; + x = $[3]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-non-final-default.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-non-final-default.expect.md index 915218fcfa467..0a5e7103c602e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-non-final-default.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-non-final-default.expect.md @@ -33,10 +33,10 @@ function Component(props) { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(7); + const $ = _c(8); let y; let t0; - if ($[0] !== props) { + if ($[0] !== props.p0 || $[1] !== props.p2) { const x = []; bb0: switch (props.p0) { case 1: { @@ -45,11 +45,11 @@ function Component(props) { case true: { x.push(props.p2); let t1; - if ($[3] === Symbol.for("react.memo_cache_sentinel")) { + if ($[4] === Symbol.for("react.memo_cache_sentinel")) { t1 = []; - $[3] = t1; + $[4] = t1; } else { - t1 = $[3]; + t1 = $[4]; } y = t1; } @@ -62,23 +62,24 @@ function Component(props) { } t0 = ; - $[0] = props; - $[1] = y; - $[2] = t0; + $[0] = props.p0; + $[1] = props.p2; + $[2] = y; + $[3] = t0; } else { - y = $[1]; - t0 = $[2]; + y = $[2]; + t0 = $[3]; } const child = t0; y.push(props.p4); let t1; - if ($[4] !== y || $[5] !== child) { + if ($[5] !== y || $[6] !== child) { t1 = {child}; - $[4] = y; - $[5] = child; - $[6] = t1; + $[5] = y; + $[6] = child; + $[7] = t1; } else { - t1 = $[6]; + t1 = $[7]; } return t1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch.expect.md index 0c5aea9c7da9c..b83c1fcb7b68f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch.expect.md @@ -28,10 +28,10 @@ function Component(props) { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(6); + const $ = _c(8); let y; let t0; - if ($[0] !== props) { + if ($[0] !== props.p0 || $[1] !== props.p2 || $[2] !== props.p3) { const x = []; switch (props.p0) { case true: { @@ -44,23 +44,25 @@ function Component(props) { } t0 = ; - $[0] = props; - $[1] = y; - $[2] = t0; + $[0] = props.p0; + $[1] = props.p2; + $[2] = props.p3; + $[3] = y; + $[4] = t0; } else { - y = $[1]; - t0 = $[2]; + y = $[3]; + t0 = $[4]; } const child = t0; y.push(props.p4); let t1; - if ($[3] !== y || $[4] !== child) { + if ($[5] !== y || $[6] !== child) { t1 = {child}; - $[3] = y; - $[4] = child; - $[5] = t1; + $[5] = y; + $[6] = child; + $[7] = t1; } else { - t1 = $[5]; + t1 = $[7]; } return t1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-load-cached.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-load-cached.expect.md new file mode 100644 index 0000000000000..a2a0e3bef9b76 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-load-cached.expect.md @@ -0,0 +1,73 @@ + +## Input + +```javascript +import {Stringify} from 'shared-runtime'; +import {makeArray} from 'shared-runtime'; + +/** + * Here, we don't need to memoize Stringify as it is a read off of a global. + * TODO: in PropagateScopeDeps (hir), we should produce a sidemap of global rvals + * and avoid adding them to `temporariesUsedOutsideDefiningScope`. + */ +function Component({num}: {num: number}) { + const arr = makeArray(num); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{num: 2}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify } from "shared-runtime"; +import { makeArray } from "shared-runtime"; + +/** + * Here, we don't need to memoize Stringify as it is a read off of a global. + * TODO: in PropagateScopeDeps (hir), we should produce a sidemap of global rvals + * and avoid adding them to `temporariesUsedOutsideDefiningScope`. + */ +function Component(t0) { + const $ = _c(6); + const { num } = t0; + let T0; + let t1; + if ($[0] !== num) { + const arr = makeArray(num); + T0 = Stringify; + t1 = arr.push(num); + $[0] = num; + $[1] = T0; + $[2] = t1; + } else { + T0 = $[1]; + t1 = $[2]; + } + let t2; + if ($[3] !== T0 || $[4] !== t1) { + t2 = ; + $[3] = T0; + $[4] = t1; + $[5] = t2; + } else { + t2 = $[5]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ num: 2 }], +}; + +``` + +### Eval output +(kind: ok)
{"value":2}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-load-cached.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-load-cached.tsx new file mode 100644 index 0000000000000..ff000fd86699d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-load-cached.tsx @@ -0,0 +1,17 @@ +import {Stringify} from 'shared-runtime'; +import {makeArray} from 'shared-runtime'; + +/** + * Here, we don't need to memoize Stringify as it is a read off of a global. + * TODO: in PropagateScopeDeps (hir), we should produce a sidemap of global rvals + * and avoid adding them to `temporariesUsedOutsideDefiningScope`. + */ +function Component({num}: {num: number}) { + const arr = makeArray(num); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{num: 2}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-property-load-cached.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-property-load-cached.expect.md new file mode 100644 index 0000000000000..cf2ad80e7ac51 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-property-load-cached.expect.md @@ -0,0 +1,78 @@ + +## Input + +```javascript +import * as SharedRuntime from 'shared-runtime'; +import {makeArray} from 'shared-runtime'; + +/** + * Here, we don't need to memoize SharedRuntime.Stringify as it is a PropertyLoad + * off of a global. + * TODO: in PropagateScopeDeps (hir), we should produce a sidemap of global rvals + * and avoid adding them to `temporariesUsedOutsideDefiningScope`. + */ +function Component({num}: {num: number}) { + const arr = makeArray(num); + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{num: 2}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import * as SharedRuntime from "shared-runtime"; +import { makeArray } from "shared-runtime"; + +/** + * Here, we don't need to memoize SharedRuntime.Stringify as it is a PropertyLoad + * off of a global. + * TODO: in PropagateScopeDeps (hir), we should produce a sidemap of global rvals + * and avoid adding them to `temporariesUsedOutsideDefiningScope`. + */ +function Component(t0) { + const $ = _c(6); + const { num } = t0; + let T0; + let t1; + if ($[0] !== num) { + const arr = makeArray(num); + + T0 = SharedRuntime.Stringify; + t1 = arr.push(num); + $[0] = num; + $[1] = T0; + $[2] = t1; + } else { + T0 = $[1]; + t1 = $[2]; + } + let t2; + if ($[3] !== T0 || $[4] !== t1) { + t2 = ; + $[3] = T0; + $[4] = t1; + $[5] = t2; + } else { + t2 = $[5]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ num: 2 }], +}; + +``` + +### Eval output +(kind: ok)
{"value":2}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-property-load-cached.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-property-load-cached.tsx new file mode 100644 index 0000000000000..be9f3e7ab9ce1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-property-load-cached.tsx @@ -0,0 +1,20 @@ +import * as SharedRuntime from 'shared-runtime'; +import {makeArray} from 'shared-runtime'; + +/** + * Here, we don't need to memoize SharedRuntime.Stringify as it is a PropertyLoad + * off of a global. + * TODO: in PropagateScopeDeps (hir), we should produce a sidemap of global rvals + * and avoid adding them to `temporariesUsedOutsideDefiningScope`. + */ +function Component({num}: {num: number}) { + const arr = makeArray(num); + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{num: 2}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-mutate-outer-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-mutate-outer-value.expect.md index 856d1326407ab..cab72226d27ef 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-mutate-outer-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-mutate-outer-value.expect.md @@ -28,9 +28,9 @@ import { c as _c } from "react/compiler-runtime"; const { shallowCopy, throwErrorWithMessage } = require("shared-runtime"); function Component(props) { - const $ = _c(3); + const $ = _c(5); let x; - if ($[0] !== props.a) { + if ($[0] !== props) { x = []; try { let t0; @@ -42,9 +42,17 @@ function Component(props) { } x.push(t0); } catch { - x.push(shallowCopy({ a: props.a })); + let t0; + if ($[3] !== props.a) { + t0 = shallowCopy({ a: props.a }); + $[3] = props.a; + $[4] = t0; + } else { + t0 = $[4]; + } + x.push(t0); } - $[0] = props.a; + $[0] = props; $[1] = x; } else { x = $[1]; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch-escaping.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch-escaping.expect.md index 700df01c5cca5..a6d4af8410edf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch-escaping.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch-escaping.expect.md @@ -31,9 +31,9 @@ import { c as _c } from "react/compiler-runtime"; const { throwInput } = require("shared-runtime"); function Component(props) { - const $ = _c(3); + const $ = _c(2); let x; - if ($[0] !== props.y || $[1] !== props.e) { + if ($[0] !== props) { try { const y = []; y.push(props.y); @@ -43,11 +43,10 @@ function Component(props) { e.push(props.e); x = e; } - $[0] = props.y; - $[1] = props.e; - $[2] = x; + $[0] = props; + $[1] = x; } else { - x = $[2]; + x = $[1]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch.expect.md index c740c7d6b6c1c..83817c19c1e5c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-try-value-modified-in-catch.expect.md @@ -30,9 +30,9 @@ import { c as _c } from "react/compiler-runtime"; const { throwInput } = require("shared-runtime"); function Component(props) { - const $ = _c(3); + const $ = _c(2); let t0; - if ($[0] !== props.y || $[1] !== props.e) { + if ($[0] !== props) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { try { @@ -46,11 +46,10 @@ function Component(props) { break bb0; } } - $[0] = props.y; - $[1] = props.e; - $[2] = t0; + $[0] = props; + $[1] = t0; } else { - t0 = $[2]; + t0 = $[1]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-if-else.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-if-else.expect.md index 05e465000d6cd..03725703f73d9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-if-else.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-multiple-if-else.expect.md @@ -33,11 +33,16 @@ import { c as _c } from "react/compiler-runtime"; import { useMemo } from "react"; function Component(props) { - const $ = _c(3); + const $ = _c(6); let t0; bb0: { let y; - if ($[0] !== props) { + if ( + $[0] !== props.cond || + $[1] !== props.a || + $[2] !== props.cond2 || + $[3] !== props.b + ) { y = []; if (props.cond) { y.push(props.a); @@ -48,12 +53,15 @@ function Component(props) { } y.push(props.b); - $[0] = props; - $[1] = y; - $[2] = t0; + $[0] = props.cond; + $[1] = props.a; + $[2] = props.cond2; + $[3] = props.b; + $[4] = y; + $[5] = t0; } else { - y = $[1]; - t0 = $[2]; + y = $[4]; + t0 = $[5]; } t0 = y; } diff --git a/compiler/packages/snap/src/sprout/shared-runtime.ts b/compiler/packages/snap/src/sprout/shared-runtime.ts index bb1c65a6574ac..12223b3ad88c2 100644 --- a/compiler/packages/snap/src/sprout/shared-runtime.ts +++ b/compiler/packages/snap/src/sprout/shared-runtime.ts @@ -96,6 +96,11 @@ export function setProperty(arg: any, property: any): void { } } +export function setPropertyByKey(arg: any, key: string, property: any): void { + arg[key] = property; + return arg; +} + export function arrayPush(arr: Array, ...values: Array): void { arr.push(...values); } @@ -123,7 +128,7 @@ export function calculateExpensiveNumber(x: number): number { /** * Functions that do not mutate their parameters */ -export function shallowCopy(obj: object): object { +export function shallowCopy(obj: T): T { return Object.assign({}, obj); } From 81ff76b3251f0f4669afc5d7a5761f7f6313babe Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 5 Sep 2024 20:14:35 -0400 Subject: [PATCH 2/9] Update [ghstack-poisoned] --- .../src/HIR/CollectHoistablePropertyLoads.ts | 4 +- .../InferReactiveScopeVariables.ts | 2 +- ...and-local-variables-with-default.expect.md | 74 ++++++++++--------- .../hoist-deps-diff-ssa-instance.expect.md | 12 +-- .../hoist-deps-diff-ssa-instance.tsx | 12 +-- .../hoist-deps-diff-ssa-instance1.expect.md | 16 ++-- .../hoist-deps-diff-ssa-instance1.tsx | 16 ++-- .../todo-merge-ssa-phi-access-nodes.expect.md | 12 ++- .../todo-merge-ssa-phi-access-nodes.ts | 12 ++- ...vider-tagged-template-expression.expect.md | 10 +-- 10 files changed, 91 insertions(+), 79 deletions(-) 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 9aaef1ad8c648..263f03ea3f88f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -1,5 +1,5 @@ import {CompilerError} from '../CompilerError'; -import {inMutableRange} from '../ReactiveScopes/InferReactiveScopeVariables'; +import {inRange} from '../ReactiveScopes/InferReactiveScopeVariables'; import {Set_intersect, Set_union, getOrInsertDefault} from '../Utils/utils'; import { BasicBlock, @@ -251,7 +251,7 @@ function collectNodes( value.object.identifier.mutableRange.end > value.object.identifier.mutableRange.start + 1 && value.object.identifier.scope != null && - inMutableRange(instr, value.object.identifier.scope.range); + inRange(instr, value.object.identifier.scope.range); if ( !isMutableAtInstr || knownImmutableIdentifiers.has(propertyNode.fullPath.identifier) diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts index 1382591731b14..15b18b7bfb8ad 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -192,7 +192,7 @@ export function isMutable({id}: Instruction, place: Place): boolean { return id >= range.start && id < range.end; } -export function inMutableRange({id}: Instruction, range: MutableRange): boolean { +export function inRange({id}: Instruction, range: MutableRange): boolean { return id >= range.start && id < range.end; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md index 17dd0f835942d..5e8f199206f58 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md @@ -63,63 +63,67 @@ function useFragment(_arg1, _arg2) { } function Component(props) { - const $ = _c(8); - const post = useFragment( - graphql` + const $ = _c(9); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = graphql` fragment F on T { id } - `, - props.post, - ); - let t0; - if ($[0] !== post) { + `; + $[0] = t0; + } else { + t0 = $[0]; + } + const post = useFragment(t0, props.post); + let t1; + if ($[1] !== post) { const allUrls = []; - const { media: t1, comments: t2, urls: t3 } = post; - const media = t1 === undefined ? null : t1; - let t4; - if ($[2] !== t2) { - t4 = t2 === undefined ? [] : t2; - $[2] = t2; - $[3] = t4; - } else { - t4 = $[3]; - } - const comments = t4; + const { media: t2, comments: t3, urls: t4 } = post; + const media = t2 === undefined ? null : t2; let t5; - if ($[4] !== t3) { + if ($[3] !== t3) { t5 = t3 === undefined ? [] : t3; - $[4] = t3; - $[5] = t5; + $[3] = t3; + $[4] = t5; } else { - t5 = $[5]; + t5 = $[4]; } - const urls = t5; + const comments = t5; let t6; - if ($[6] !== comments.length) { - t6 = (e) => { + if ($[5] !== t4) { + t6 = t4 === undefined ? [] : t4; + $[5] = t4; + $[6] = t6; + } else { + t6 = $[6]; + } + const urls = t6; + let t7; + if ($[7] !== comments.length) { + t7 = (e) => { if (!comments.length) { return; } console.log(comments.length); }; - $[6] = comments.length; - $[7] = t6; + $[7] = comments.length; + $[8] = t7; } else { - t6 = $[7]; + t7 = $[8]; } - const onClick = t6; + const onClick = t7; allUrls.push(...urls); - t0 = ; - $[0] = post; - $[1] = t0; + t1 = ; + $[1] = post; + $[2] = t1; } else { - t0 = $[1]; + t1 = $[2]; } - return t0; + return t1; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.expect.md index 0115f068a855d..701702f9dd7ff 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.expect.md @@ -2,10 +2,7 @@ ## Input ```javascript -import { - makeObject_Primitives, - setPropertyByKey, -} from 'shared-runtime'; +import {makeObject_Primitives, setPropertyByKey} from 'shared-runtime'; function useFoo({value, cond}) { let x: any = makeObject_Primitives(); @@ -31,8 +28,11 @@ function useFoo({value, cond}) { export const FIXTURE_ENTRYPOINT = { fn: useFoo, - params: [{value:3,cond: true}], - sequentialRenders: [{value:3,cond: true}, {value:3,cond: false}], + params: [{value: 3, cond: true}], + sequentialRenders: [ + {value: 3, cond: true}, + {value: 3, cond: false}, + ], }; ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.tsx index 3cfcd7da5c371..3f75571bd700c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance.tsx @@ -1,7 +1,4 @@ -import { - makeObject_Primitives, - setPropertyByKey, -} from 'shared-runtime'; +import {makeObject_Primitives, setPropertyByKey} from 'shared-runtime'; function useFoo({value, cond}) { let x: any = makeObject_Primitives(); @@ -27,6 +24,9 @@ function useFoo({value, cond}) { export const FIXTURE_ENTRYPOINT = { fn: useFoo, - params: [{value:3,cond: true}], - sequentialRenders: [{value:3,cond: true}, {value:3,cond: false}], + params: [{value: 3, cond: true}], + sequentialRenders: [ + {value: 3, cond: true}, + {value: 3, cond: false}, + ], }; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.expect.md index 745df97b6da76..54ee5676b3f6d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.expect.md @@ -2,12 +2,12 @@ ## Input ```javascript -import { identity, shallowCopy, Stringify, useIdentity } from "shared-runtime"; +import {identity, shallowCopy, Stringify, useIdentity} from 'shared-runtime'; -type HasA = {kind: "hasA", a: {value: number}}; -type HasC = {kind: "hasC", c: {value: number}}; -function Foo({ cond }: {cond: boolean}) { - let x: HasA | HasC = shallowCopy({kind: "hasA", a: {value: 2}}); +type HasA = {kind: 'hasA'; a: {value: number}}; +type HasC = {kind: 'hasC'; c: {value: number}}; +function Foo({cond}: {cond: boolean}) { + let x: HasA | HasC = shallowCopy({kind: 'hasA', a: {value: 2}}); /** * This read of x.a.value is outside of x's identifier mutable * range + scope range. We mark this ssa instance (x_@0) as having @@ -15,7 +15,7 @@ function Foo({ cond }: {cond: boolean}) { */ Math.max(x.a.value, 2); if (cond) { - x = shallowCopy({kind: "hasC", c: {value: 3}}); + x = shallowCopy({kind: 'hasC', c: {value: 3}}); } /** @@ -27,8 +27,8 @@ function Foo({ cond }: {cond: boolean}) { export const FIXTURE_ENTRYPOINT = { fn: Foo, params: [{cond: false}], - sequentialRenders: [{cond: false}, {cond: true}] -} + sequentialRenders: [{cond: false}, {cond: true}], +}; ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.tsx index 2ac73b25f8f5e..147ca85809355 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/hoist-deps-diff-ssa-instance1.tsx @@ -1,9 +1,9 @@ -import { identity, shallowCopy, Stringify, useIdentity } from "shared-runtime"; +import {identity, shallowCopy, Stringify, useIdentity} from 'shared-runtime'; -type HasA = {kind: "hasA", a: {value: number}}; -type HasC = {kind: "hasC", c: {value: number}}; -function Foo({ cond }: {cond: boolean}) { - let x: HasA | HasC = shallowCopy({kind: "hasA", a: {value: 2}}); +type HasA = {kind: 'hasA'; a: {value: number}}; +type HasC = {kind: 'hasC'; c: {value: number}}; +function Foo({cond}: {cond: boolean}) { + let x: HasA | HasC = shallowCopy({kind: 'hasA', a: {value: 2}}); /** * This read of x.a.value is outside of x's identifier mutable * range + scope range. We mark this ssa instance (x_@0) as having @@ -11,7 +11,7 @@ function Foo({ cond }: {cond: boolean}) { */ Math.max(x.a.value, 2); if (cond) { - x = shallowCopy({kind: "hasC", c: {value: 3}}); + x = shallowCopy({kind: 'hasC', c: {value: 3}}); } /** @@ -23,5 +23,5 @@ function Foo({ cond }: {cond: boolean}) { export const FIXTURE_ENTRYPOINT = { fn: Foo, params: [{cond: false}], - sequentialRenders: [{cond: false}, {cond: true}] -} + sequentialRenders: [{cond: false}, {cond: true}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.expect.md index 49d872e773a1c..ccd81b3e14425 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.expect.md @@ -2,7 +2,11 @@ ## Input ```javascript -import { identity, makeObject_Primitives, setPropertyByKey } from "shared-runtime"; +import { + identity, + makeObject_Primitives, + setPropertyByKey, +} from 'shared-runtime'; /** * A bit of an edge case, but we could further optimize here by merging @@ -26,7 +30,7 @@ function useFoo(cond) { /** * At this point, we have a phi node. * x_@2 = phi(x_@0, x_@1) - * + * * We can assume that both x_@0 and x_@1 both have non-null `x.a` properties, * so we can infer that x_@2 does as well. */ @@ -41,8 +45,8 @@ function useFoo(cond) { export const FIXTURE_ENTRYPOINT = { fn: useFoo, - params: [true] -} + params: [true], +}; ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.ts index bdcf9b7b0ec26..749b6c0a831c0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/todo-merge-ssa-phi-access-nodes.ts @@ -1,4 +1,8 @@ -import { identity, makeObject_Primitives, setPropertyByKey } from "shared-runtime"; +import { + identity, + makeObject_Primitives, + setPropertyByKey, +} from 'shared-runtime'; /** * A bit of an edge case, but we could further optimize here by merging @@ -22,7 +26,7 @@ function useFoo(cond) { /** * At this point, we have a phi node. * x_@2 = phi(x_@0, x_@1) - * + * * We can assume that both x_@0 and x_@1 both have non-null `x.a` properties, * so we can infer that x_@2 does as well. */ @@ -37,5 +41,5 @@ function useFoo(cond) { export const FIXTURE_ENTRYPOINT = { fn: useFoo, - params: [true] -} + params: [true], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-provider-tagged-template-expression.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-provider-tagged-template-expression.expect.md index 03bfef9fb2eff..dcc89fcc1eca7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-provider-tagged-template-expression.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-provider-tagged-template-expression.expect.md @@ -37,13 +37,13 @@ import { graphql } from "shared-runtime"; export function Component(t0) { const $ = _c(1); - const fragment = graphql` - fragment Foo on User { - name - } - `; let t1; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const fragment = graphql` + fragment Foo on User { + name + } + `; t1 =
{fragment}
; $[0] = t1; } else { From cce7b0d0dd4ef1d70d893139a7df0edbb07eea44 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Fri, 6 Sep 2024 13:17:30 -0400 Subject: [PATCH 3/9] Update [ghstack-poisoned] --- .../src/HIR/CollectHoistablePropertyLoads.ts | 153 ++++++++++-------- 1 file changed, 88 insertions(+), 65 deletions(-) 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 000d72259c2ea..db2869a5472f5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -9,8 +9,10 @@ import { HIRFunction, Identifier, Place, + PropertyLoad, ReactiveScopeDependency, ScopeId, + TInstruction, } from './HIR'; export type CollectHoistablePropertyLoadsResult = { @@ -72,9 +74,8 @@ export function collectHoistablePropertyLoads( fn: HIRFunction, usedOutsideDeclaringScope: Set, ): CollectHoistablePropertyLoadsResult { - const sidemap = new TemporariesSideMap(); - - const nodes = collectNodes(fn, usedOutsideDeclaringScope, sidemap); + const sidemap = collectSidemap(fn, usedOutsideDeclaringScope); + const nodes = collectNodes(fn, sidemap); deriveNonNull(fn, nodes); const nodesKeyedByScopeId = new Map(); @@ -236,25 +237,63 @@ class Tree { return Tree.#getOrCreateProperty(currNode, n.path.at(-1)!.property); } } +type PropertyLoadInfo = { + instr: TInstruction; + property: ReactiveScopeDependency; +}; -class TemporariesSideMap { - temporaries: Map = new Map(); - properties: Map = new Map(); - tree: Tree = new Tree(); - - declareTemporary(from: Identifier, to: Identifier): void { - this.temporaries.set(from, to); - } +type TemporariesSidemap = { + temporaries: Map; + properties: Map; + propertyLoadInfo: Map>; +}; - declareProperty(from: Identifier, to: ReactiveScopeDependency): void { - this.properties.set(from, to); +function collectSidemap( + fn: HIRFunction, + usedOutsideDeclaringScope: Set, +): TemporariesSidemap { + const temporaries = new Map(); + const properties = new Map(); + const propertyLoadInfo = new Map>(); + for (const [_, block] of fn.body.blocks) { + const propertyLoads: Array = []; + for (const instr of block.instructions) { + const {value, lvalue} = instr; + const usedOutside = usedOutsideDeclaringScope.has( + lvalue.identifier.declarationId, + ); + if (value.kind === 'PropertyLoad') { + const property = getProperty( + value.object, + value.property, + temporaries, + properties, + ); + if (!usedOutside) { + properties.set(lvalue.identifier, property); + } + propertyLoads.push({ + instr: instr as TInstruction, + property, + }); + } else if (value.kind === 'LoadLocal') { + if ( + lvalue.identifier.name == null && + value.place.identifier.name !== null && + !usedOutside + ) { + temporaries.set(lvalue.identifier, value.place.identifier); + } + } + } + propertyLoadInfo.set(block.id, propertyLoads); } + return {temporaries, properties, propertyLoadInfo}; } function collectNodes( fn: HIRFunction, - usedOutsideDeclaringScope: Set, - c: TemporariesSideMap, + sidemap: TemporariesSidemap, ): Map { /** * Due to current limitations of mutable range inference, there are edge cases in @@ -272,62 +311,46 @@ function collectNodes( } } } + const tree = new Tree(); const nodes = new Map(); - for (const [blockId, block] of fn.body.blocks) { + for (const [_, block] of fn.body.blocks) { + const propertyLoadInfos = sidemap.propertyLoadInfo.get(block.id); + CompilerError.invariant(propertyLoadInfos != null, { + reason: '[CollectHoistablePropertyLoads] block info missing', + loc: GeneratedSource, + }); + const assumedNonNullObjects = new Set(); - for (const instr of block.instructions) { - const {value, lvalue} = instr; - const usedOutside = usedOutsideDeclaringScope.has( - lvalue.identifier.declarationId, - ); - if (value.kind === 'PropertyLoad') { - const property = getProperty( - value.object, - value.property, - c.temporaries, - c.properties, - ); - if (!usedOutside) { - c.declareProperty(lvalue.identifier, property); - } - const propertyNode = c.tree.getPropertyLoadNode(property); - /** - * Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges - * are not valid with respect to current instruction id numbering. - * We use attached reactive scope ranges as a proxy for mutable range, but this - * is an overestimate as (1) scope ranges merge and align to form valid program - * blocks and (2) passes like MemoizeFbtAndMacroOperands may assign scopes to - * non-mutable identifiers. - * - * See comment at top of function for why we track known immutable identifiers. - */ - const isMutableAtInstr = - value.object.identifier.mutableRange.end > - value.object.identifier.mutableRange.start + 1 && - value.object.identifier.scope != null && - inRange(instr, value.object.identifier.scope.range); - if ( - !isMutableAtInstr || - knownImmutableIdentifiers.has(propertyNode.fullPath.identifier) - ) { - let curr = propertyNode.parent; - while (curr != null) { - assumedNonNullObjects.add(curr); - curr = curr.parent; - } - } - } else if (value.kind === 'LoadLocal') { - if ( - lvalue.identifier.name == null && - value.place.identifier.name !== null && - !usedOutside - ) { - c.declareTemporary(lvalue.identifier, value.place.identifier); + for (const {instr, property} of propertyLoadInfos) { + const object = instr.value.object.identifier; + const propertyNode = tree.getPropertyLoadNode(property); + /** + * Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges + * are not valid with respect to current instruction id numbering. + * We use attached reactive scope ranges as a proxy for mutable range, but this + * is an overestimate as (1) scope ranges merge and align to form valid program + * blocks and (2) passes like MemoizeFbtAndMacroOperands may assign scopes to + * non-mutable identifiers. + * + * 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(instr, object.scope.range); + if ( + !isMutableAtInstr || + knownImmutableIdentifiers.has(propertyNode.fullPath.identifier) + ) { + let curr = propertyNode.parent; + while (curr != null) { + assumedNonNullObjects.add(curr); + curr = curr.parent; } } } - nodes.set(blockId, { + nodes.set(block.id, { block, assumedNonNullObjects, }); From 30080d777999565561bce4acb941e8513e1d9d9b Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Fri, 6 Sep 2024 13:20:47 -0400 Subject: [PATCH 4/9] Update [ghstack-poisoned] --- .../src/HIR/DeriveMinimalDependenciesHIR.ts | 53 +++++++++---------- .../src/HIR/PropagateScopeDependenciesHIR.ts | 2 +- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts index a6a8a2d5e3de3..ecc1844b006aa 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts @@ -19,18 +19,15 @@ const ENABLE_DEBUG_INVARIANTS = true; export class ReactiveScopeDependencyTreeHIR { #roots: Map = new Map(); - #getOrCreateRoot( - identifier: Identifier, - isHoistable: boolean, - ): DependencyNode { + #getOrCreateRoot(identifier: Identifier, isNonNull: boolean): DependencyNode { // roots can always be accessed unconditionally in JS let rootNode = this.#roots.get(identifier); if (rootNode === undefined) { rootNode = { properties: new Map(), - accessType: isHoistable - ? PropertyAccessType.HoistableAccess + accessType: isNonNull + ? PropertyAccessType.NonNullAccess : PropertyAccessType.Access, }; this.#roots.set(identifier, rootNode); @@ -64,8 +61,8 @@ export class ReactiveScopeDependencyTreeHIR { ); } - markNodesHoistable(dep: ReactiveScopePropertyDependency): void { - const accessType = PropertyAccessType.HoistableAccess; + markNodesNonNull(dep: ReactiveScopePropertyDependency): void { + const accessType = PropertyAccessType.NonNullAccess; let currNode = this.#roots.get(dep.identifier); let cursor = 0; @@ -121,27 +118,27 @@ export class ReactiveScopeDependencyTreeHIR { enum PropertyAccessType { Access = 'Access', - HoistableAccess = 'HoistableAccess', + NonNullAccess = 'NonNullAccess', Dependency = 'Dependency', - HoistableDependency = 'HoistableDependency', + NonNullDependency = 'NonNullDependency', } const MIN_ACCESS_TYPE = PropertyAccessType.Access; /** - * "Hoistable" means that PropertyReads from a node are side-effect free. - * In other words, this means that control flow analysis shows that - * we can assume this node is a non-null object. + * "NonNull" means that PropertyReads from a node are side-effect free, + * as the node is (1) immutable and (2) has unconditional propertyloads + * somewhere in the cfg. */ -function isHoistable(access: PropertyAccessType): boolean { +function isNonNull(access: PropertyAccessType): boolean { return ( - access === PropertyAccessType.HoistableAccess || - access === PropertyAccessType.HoistableDependency + access === PropertyAccessType.NonNullAccess || + access === PropertyAccessType.NonNullDependency ); } function isDependency(access: PropertyAccessType): boolean { return ( access === PropertyAccessType.Dependency || - access === PropertyAccessType.HoistableDependency + access === PropertyAccessType.NonNullDependency ); } @@ -149,22 +146,22 @@ function merge( access1: PropertyAccessType, access2: PropertyAccessType, ): PropertyAccessType { - const resultisHoistable = isHoistable(access1) || isHoistable(access2); + const resultisNonNull = isNonNull(access1) || isNonNull(access2); const resultIsDependency = isDependency(access1) || isDependency(access2); /* * Straightforward merge. * This can be represented as bitwise OR, but is written out for readability * - * Observe that `HoistableAccess | Dependency` produces an + * Observe that `NonNullAccess | Dependency` produces an * unconditionally accessed conditional dependency. We currently use these * as we use unconditional dependencies. (i.e. to codegen change variables) */ - if (resultisHoistable) { + if (resultisNonNull) { if (resultIsDependency) { - return PropertyAccessType.HoistableDependency; + return PropertyAccessType.NonNullDependency; } else { - return PropertyAccessType.HoistableAccess; + return PropertyAccessType.NonNullAccess; } } else { if (resultIsDependency) { @@ -185,15 +182,15 @@ type ReduceResultNode = { }; function assertWellFormedTree(node: DependencyNode): void { - let hoistableInChildren = false; + let nonNullInChildren = false; for (const childNode of node.properties.values()) { assertWellFormedTree(childNode); - hoistableInChildren ||= isHoistable(childNode.accessType); + nonNullInChildren ||= isNonNull(childNode.accessType); } - if (hoistableInChildren) { - CompilerError.invariant(isHoistable(node.accessType), { + if (nonNullInChildren) { + CompilerError.invariant(isNonNull(node.accessType), { reason: - '[DeriveMinimialDependencies] Not well formed tree, unexpected hoistable node', + '[DeriveMinimialDependencies] Not well formed tree, unexpected non-null node', description: node.accessType, loc: GeneratedSource, }); @@ -212,7 +209,7 @@ function deriveMinimalDependenciesInSubtree( */ return [{path}]; } else { - if (isHoistable(node.accessType)) { + if (isNonNull(node.accessType)) { /* * Only recurse into subtree dependencies if this node * is known to be non-null. diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index ec47b7fcc6615..7d2bb48d516bb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -505,7 +505,7 @@ function recordHoistablePropertyReads( }); for (const item of node.assumedNonNullObjects) { - tree.markNodesHoistable({ + tree.markNodesNonNull({ ...item.fullPath, }); } From 6c8da56bae08d8289ffd03b55e32253653e86c71 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Fri, 6 Sep 2024 13:22:52 -0400 Subject: [PATCH 5/9] Update [ghstack-poisoned] --- ...and-local-variables-with-default.expect.md | 74 +++++++++---------- ...vider-tagged-template-expression.expect.md | 10 +-- 2 files changed, 40 insertions(+), 44 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md index 5e8f199206f58..17dd0f835942d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md @@ -63,67 +63,63 @@ function useFragment(_arg1, _arg2) { } function Component(props) { - const $ = _c(9); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 = graphql` + const $ = _c(8); + const post = useFragment( + graphql` fragment F on T { id } - `; - $[0] = t0; - } else { - t0 = $[0]; - } - const post = useFragment(t0, props.post); - let t1; - if ($[1] !== post) { + `, + props.post, + ); + let t0; + if ($[0] !== post) { const allUrls = []; - const { media: t2, comments: t3, urls: t4 } = post; - const media = t2 === undefined ? null : t2; + const { media: t1, comments: t2, urls: t3 } = post; + const media = t1 === undefined ? null : t1; + let t4; + if ($[2] !== t2) { + t4 = t2 === undefined ? [] : t2; + $[2] = t2; + $[3] = t4; + } else { + t4 = $[3]; + } + const comments = t4; let t5; - if ($[3] !== t3) { + if ($[4] !== t3) { t5 = t3 === undefined ? [] : t3; - $[3] = t3; - $[4] = t5; + $[4] = t3; + $[5] = t5; } else { - t5 = $[4]; + t5 = $[5]; } - const comments = t5; + const urls = t5; let t6; - if ($[5] !== t4) { - t6 = t4 === undefined ? [] : t4; - $[5] = t4; - $[6] = t6; - } else { - t6 = $[6]; - } - const urls = t6; - let t7; - if ($[7] !== comments.length) { - t7 = (e) => { + if ($[6] !== comments.length) { + t6 = (e) => { if (!comments.length) { return; } console.log(comments.length); }; - $[7] = comments.length; - $[8] = t7; + $[6] = comments.length; + $[7] = t6; } else { - t7 = $[8]; + t6 = $[7]; } - const onClick = t7; + const onClick = t6; allUrls.push(...urls); - t1 = ; - $[1] = post; - $[2] = t1; + t0 = ; + $[0] = post; + $[1] = t0; } else { - t1 = $[2]; + t0 = $[1]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-provider-tagged-template-expression.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-provider-tagged-template-expression.expect.md index dcc89fcc1eca7..03bfef9fb2eff 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-provider-tagged-template-expression.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-provider-tagged-template-expression.expect.md @@ -37,13 +37,13 @@ import { graphql } from "shared-runtime"; export function Component(t0) { const $ = _c(1); + const fragment = graphql` + fragment Foo on User { + name + } + `; let t1; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - const fragment = graphql` - fragment Foo on User { - name - } - `; t1 =
{fragment}
; $[0] = t1; } else { From 999bb6bebcd83570c936fe6f25f0b15de5000140 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Fri, 6 Sep 2024 16:51:19 -0400 Subject: [PATCH 6/9] Update [ghstack-poisoned] --- .../src/HIR/CollectHoistablePropertyLoads.ts | 23 +++++++++++-------- .../src/HIR/PropagateScopeDependenciesHIR.ts | 23 ++++++++----------- 2 files changed, 22 insertions(+), 24 deletions(-) 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 db2869a5472f5..fbe03a82b1422 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -16,9 +16,9 @@ import { } from './HIR'; export type CollectHoistablePropertyLoadsResult = { - nodes: Map; - temporaries: Map; - properties: Map; + nodes: ReadonlyMap; + temporaries: ReadonlyMap; + properties: ReadonlyMap; }; /** @@ -72,7 +72,7 @@ export type CollectHoistablePropertyLoadsResult = { */ export function collectHoistablePropertyLoads( fn: HIRFunction, - usedOutsideDeclaringScope: Set, + usedOutsideDeclaringScope: ReadonlySet, ): CollectHoistablePropertyLoadsResult { const sidemap = collectSidemap(fn, usedOutsideDeclaringScope); const nodes = collectNodes(fn, sidemap); @@ -243,14 +243,14 @@ type PropertyLoadInfo = { }; type TemporariesSidemap = { - temporaries: Map; - properties: Map; - propertyLoadInfo: Map>; + temporaries: ReadonlyMap; + properties: ReadonlyMap; + propertyLoadInfo: ReadonlyMap>; }; function collectSidemap( fn: HIRFunction, - usedOutsideDeclaringScope: Set, + usedOutsideDeclaringScope: ReadonlySet, ): TemporariesSidemap { const temporaries = new Map(); const properties = new Map(); @@ -294,7 +294,7 @@ function collectSidemap( function collectNodes( fn: HIRFunction, sidemap: TemporariesSidemap, -): Map { +): ReadonlyMap { /** * Due to current limitations of mutable range inference, there are edge cases in * which we infer known-immutable values (e.g. props or hook params) to have a @@ -358,7 +358,10 @@ function collectNodes( return nodes; } -function deriveNonNull(fn: HIRFunction, nodes: Map): void { +function deriveNonNull( + fn: HIRFunction, + nodes: ReadonlyMap, +): void { const succ = new Map>(); const terminalPreds = new Set(); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index 7d2bb48d516bb..35b39851cf242 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -84,7 +84,7 @@ export function propagateScopeDependenciesHIR(fn: HIRFunction): void { function findTemporariesUsedOutsideDeclaringScope( fn: HIRFunction, -): Set { +): ReadonlySet { /* * tracks all relevant LoadLocal and PropertyLoad lvalues * and the scope where they are defined @@ -150,25 +150,20 @@ type Decl = { }; class Context { - #temporariesUsedOutsideScope: Set; #declarations: Map = new Map(); #reassignments: Map = new Map(); + + #scopes: Stack = empty(); // Reactive dependencies used in the current reactive scope. #dependencies: Stack> = empty(); - /* - * We keep a sidemap for temporaries created by PropertyLoads, and do - * not store any control flow (i.e. #inConditionalWithinScope) here. - * - a ReactiveScope (A) containing a PropertyLoad may differ from the - * ReactiveScope (B) that uses the produced temporary. - * - codegen will inline these PropertyLoads back into scope (B) - */ + deps: Map> = new Map(); + #properties: ReadonlyMap; #temporaries: ReadonlyMap; - #scopes: Stack = empty(); - deps: Map> = new Map(); + #temporariesUsedOutsideScope: ReadonlySet; constructor( - temporariesUsedOutsideScope: Set, + temporariesUsedOutsideScope: ReadonlySet, temporaries: ReadonlyMap, properties: ReadonlyMap, ) { @@ -445,7 +440,7 @@ function handleInstruction(instr: Instruction, context: Context): void { function collectDependencies( fn: HIRFunction, - usedOutsideDeclaringScope: Set, + usedOutsideDeclaringScope: ReadonlySet, temporaries: ReadonlyMap, properties: ReadonlyMap, ): Map> { @@ -494,7 +489,7 @@ function collectDependencies( * Compute the set of hoistable property reads. */ function recordHoistablePropertyReads( - nodes: Map, + nodes: ReadonlyMap, scopeId: ScopeId, tree: ReactiveScopeDependencyTreeHIR, ): void { From 614ebd46d8512222c4b5f0cf8eafa61d8ce36b04 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Wed, 11 Sep 2024 18:51:12 -0400 Subject: [PATCH 7/9] Update [ghstack-poisoned] --- .../src/HIR/CollectHoistablePropertyLoads.ts | 233 ++++++++---------- .../src/HIR/PropagateScopeDependenciesHIR.ts | 128 +++++++--- .../InferReactiveScopeVariables.ts | 11 +- .../src/Utils/utils.ts | 4 +- ...utate-call-after-dependency-load.expect.md | 83 +++++++ ...order-mutate-call-after-dependency-load.ts | 23 ++ ...tate-store-after-dependency-load.expect.md | 83 +++++++ ...rder-mutate-store-after-dependency-load.ts | 23 ++ .../snap/src/sprout/shared-runtime.ts | 6 +- 9 files changed, 412 insertions(+), 182 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-call-after-dependency-load.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-call-after-dependency-load.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-store-after-dependency-load.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-store-after-dependency-load.ts 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 bd98adf644f94..f8c7c86bb3923 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -4,23 +4,15 @@ import {Set_intersect, Set_union, getOrInsertDefault} from '../Utils/utils'; import { BasicBlock, BlockId, - DeclarationId, GeneratedSource, HIRFunction, Identifier, + IdentifierId, Place, - PropertyLoad, ReactiveScopeDependency, ScopeId, - TInstruction, } from './HIR'; -export type CollectHoistablePropertyLoadsResult = { - nodes: ReadonlyMap; - temporaries: ReadonlyMap; - properties: ReadonlyMap; -}; - /** * Helper function for `PropagateScopeDependencies`. * Uses control flow graph analysis to determine which `Identifier`s can @@ -72,10 +64,9 @@ export type CollectHoistablePropertyLoadsResult = { */ export function collectHoistablePropertyLoads( fn: HIRFunction, - usedOutsideDeclaringScope: ReadonlySet, -): CollectHoistablePropertyLoadsResult { - const sidemap = collectSidemap(fn, usedOutsideDeclaringScope); - const nodes = collectNodes(fn, sidemap); + temporaries: ReadonlyMap, +): ReadonlyMap { + const nodes = collectPropertyLoadsInBlocks(fn, temporaries); deriveNonNull(fn, nodes); const nodesKeyedByScopeId = new Map(); @@ -88,23 +79,18 @@ export function collectHoistablePropertyLoads( } } - return { - nodes: nodesKeyedByScopeId, - temporaries: sidemap.temporaries, - properties: sidemap.properties, - }; + return nodesKeyedByScopeId; } export type BlockInfo = { block: BasicBlock; - assumedNonNullObjects: Set; + assumedNonNullObjects: ReadonlySet; }; export function getProperty( object: Place, propertyName: string, - temporaries: ReadonlyMap, - properties: ReadonlyMap, + temporaries: ReadonlyMap, ): ReactiveScopeDependency { /* * (1) Get the base object either from the temporary sidemap (e.g. a LoadLocal) @@ -125,8 +111,7 @@ export function getProperty( * $1 = PropertyLoad $0.y * getProperty($0, ...) -> resolvedObject = null, resolvedDependency = null */ - const resolvedObject = resolveTemporary(object, temporaries); - const resolvedDependency = properties.get(resolvedObject); + const resolvedDependency = temporaries.get(object.identifier.id); /** * (2) Push the last PropertyLoad @@ -135,7 +120,7 @@ export function getProperty( let property: ReactiveScopeDependency; if (resolvedDependency == null) { property = { - identifier: resolvedObject, + identifier: object.identifier, path: [{property: propertyName, optional: false}], }; } else { @@ -152,9 +137,9 @@ export function getProperty( export function resolveTemporary( place: Place, - temporaries: ReadonlyMap, + temporaries: ReadonlyMap, ): Identifier { - return temporaries.get(place.identifier) ?? place.identifier; + return temporaries.get(place.identifier.id) ?? place.identifier; } /** @@ -237,63 +222,10 @@ class Tree { return Tree.#getOrCreateProperty(currNode, n.path.at(-1)!.property); } } -type PropertyLoadInfo = { - instr: TInstruction; - property: ReactiveScopeDependency; -}; - -type TemporariesSidemap = { - temporaries: ReadonlyMap; - properties: ReadonlyMap; - propertyLoadInfo: ReadonlyMap>; -}; -function collectSidemap( +function collectPropertyLoadsInBlocks( fn: HIRFunction, - usedOutsideDeclaringScope: ReadonlySet, -): TemporariesSidemap { - const temporaries = new Map(); - const properties = new Map(); - const propertyLoadInfo = new Map>(); - for (const [_, block] of fn.body.blocks) { - const propertyLoads: Array = []; - for (const instr of block.instructions) { - const {value, lvalue} = instr; - const usedOutside = usedOutsideDeclaringScope.has( - lvalue.identifier.declarationId, - ); - if (value.kind === 'PropertyLoad') { - const property = getProperty( - value.object, - value.property, - temporaries, - properties, - ); - if (!usedOutside) { - properties.set(lvalue.identifier, property); - } - propertyLoads.push({ - instr: instr as TInstruction, - property, - }); - } else if (value.kind === 'LoadLocal') { - if ( - lvalue.identifier.name == null && - value.place.identifier.name !== null && - !usedOutside - ) { - temporaries.set(lvalue.identifier, value.place.identifier); - } - } - } - propertyLoadInfo.set(block.id, propertyLoads); - } - return {temporaries, properties, propertyLoadInfo}; -} - -function collectNodes( - fn: HIRFunction, - sidemap: TemporariesSidemap, + temporaries: ReadonlyMap, ): ReadonlyMap { /** * Due to current limitations of mutable range inference, there are edge cases in @@ -314,40 +246,42 @@ function collectNodes( const tree = new Tree(); const nodes = new Map(); for (const [_, block] of fn.body.blocks) { - const propertyLoadInfos = sidemap.propertyLoadInfo.get(block.id); - CompilerError.invariant(propertyLoadInfos != null, { - reason: '[CollectHoistablePropertyLoads] block info missing', - loc: GeneratedSource, - }); - const assumedNonNullObjects = new Set(); - for (const {instr, property} of propertyLoadInfos) { - const object = instr.value.object.identifier; - const propertyNode = tree.getPropertyLoadNode(property); - /** - * Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges - * are not valid with respect to current instruction id numbering. - * We use attached reactive scope ranges as a proxy for mutable range, but this - * is an overestimate as (1) scope ranges merge and align to form valid program - * blocks and (2) passes like MemoizeFbtAndMacroOperands may assign scopes to - * non-mutable identifiers. - * - * 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(instr, object.scope.range); - if ( - !isMutableAtInstr || - knownImmutableIdentifiers.has(propertyNode.fullPath.identifier) - ) { - let curr = propertyNode.parent; - while (curr != null) { - assumedNonNullObjects.add(curr); - curr = curr.parent; + for (const instr of block.instructions) { + if (instr.value.kind === 'PropertyLoad') { + const property = getProperty( + instr.value.object, + instr.value.property, + temporaries, + ); + const propertyNode = tree.getPropertyLoadNode(property); + const object = instr.value.object.identifier; + /** + * Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges + * are not valid with respect to current instruction id numbering. + * We use attached reactive scope ranges as a proxy for mutable range, but this + * is an overestimate as (1) scope ranges merge and align to form valid program + * blocks and (2) passes like MemoizeFbtAndMacroOperands may assign scopes to + * non-mutable identifiers. + * + * 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(instr, object.scope.range); + if ( + !isMutableAtInstr || + knownImmutableIdentifiers.has(propertyNode.fullPath.identifier) + ) { + let curr = propertyNode.parent; + while (curr != null) { + assumedNonNullObjects.add(curr); + curr = curr.parent; + } } } + // TODO handle destructuring } nodes.set(block.id, { @@ -374,11 +308,16 @@ function deriveNonNull( } } + /** + * In the context of a control flow graph, the identifiers that a block + * can assume are non-null can be calculated from the following: + * X = Union(Intersect(X_neighbors), X) + */ function recursivelyDeriveNonNull( nodeId: BlockId, direction: 'forward' | 'backward', traversalState: Map, - nonNullObjectsByBlock: Map>, + nonNullObjectsByBlock: Map>, ): boolean { /** * Avoid re-visiting computed or currently active nodes, which can @@ -397,7 +336,9 @@ function deriveNonNull( }); } const neighbors = Array.from( - direction === 'backward' ? (blockSuccessors.get(nodeId) ?? []) : node.block.preds, + direction === 'backward' + ? (blockSuccessors.get(nodeId) ?? []) + : node.block.preds, ); let changed = false; @@ -413,24 +354,39 @@ function deriveNonNull( } } /** - * Active neighbors can be filtered out as we're solving for the following - * relation. - * X = Intersect(X_neighbors, X) + * Note that a predecessor / successor can only be active (status != 'done') + * if it is a self-loop or other transitive cycle. Active neighbors can be + * filtered out (i.e. not included in the intersection) + * Example: self loop. + * X = Union(Intersect(X, ...X_other_neighbors), X) + * + * Example: transitive cycle through node Y, for some Y that is a + * predecessor / successor of X. + * X = Union( + * Intersect( + * Union(Intersect(X, ...Y_other_neighbors), Y), + * ...X_neighbors + * ), + * X + * ) + * * Non-active neighbors with no recorded results can occur due to backedges. - * it's not safe to assume they can be filtered out (e.g. not intersected) + * it's not safe to assume they can be filtered out (e.g. not included in + * the intersection) */ - const neighborAccesses = Set_intersect([ - ...(Array.from(neighbors) + const neighborAccesses = Set_intersect( + Array.from(neighbors) .filter(n => traversalState.get(n) === 'done') - .map(n => nonNullObjectsByBlock.get(n) ?? new Set()) as Array< - Set - >), - ]); + .map(n => assertNonNull(nonNullObjectsByBlock.get(n))), + ); const prevSize = nonNullObjectsByBlock.get(nodeId)?.size; nonNullObjectsByBlock.set( nodeId, - Set_union(node.assumedNonNullObjects, neighborAccesses), + Set_union( + assertNonNull(nonNullObjectsByBlock.get(nodeId)), + neighborAccesses, + ), ); traversalState.set(nodeId, 'done'); @@ -447,45 +403,50 @@ function deriveNonNull( ); return changed; } - const fromEntry = new Map>(); - const fromExit = new Map>(); - let changed = true; + const fromEntry = new Map>(); + const fromExit = new Map>(); + for (const [blockId, blockInfo] of nodes) { + fromEntry.set(blockId, blockInfo.assumedNonNullObjects); + fromExit.set(blockId, blockInfo.assumedNonNullObjects); + } const traversalState = new Map(); const reversedBlocks = [...fn.body.blocks]; reversedBlocks.reverse(); - let i = 0; - while (changed) { + let i = 0; + let changed; + do { i++; changed = false; for (const [blockId] of fn.body.blocks) { - const changed_ = recursivelyDeriveNonNull( + const forwardChanged = recursivelyDeriveNonNull( blockId, 'forward', traversalState, fromEntry, ); - changed ||= changed_; + changed ||= forwardChanged; } traversalState.clear(); for (const [blockId] of reversedBlocks) { - const changed_ = recursivelyDeriveNonNull( + const backwardChanged = recursivelyDeriveNonNull( blockId, 'backward', traversalState, fromExit, ); - changed ||= changed_; + changed ||= backwardChanged; } traversalState.clear(); - } + } while (changed); /** * TODO: validate against meta internal code, then remove in future PR. * Currently cannot come up with a case that requires fixed-point iteration. */ - CompilerError.invariant(i === 2, { + CompilerError.invariant(i <= 2, { reason: 'require fixed-point iteration', + description: `#iterations = ${i}`, loc: GeneratedSource, }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index 35b39851cf242..4c7dac004d80a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -15,6 +15,7 @@ import { GeneratedSource, DeclarationId, areEqualPaths, + IdentifierId, } from './HIR'; import { BlockInfo, @@ -36,17 +37,14 @@ import {ReactiveScopeDependencyTreeHIR} from './DeriveMinimalDependenciesHIR'; export function propagateScopeDependenciesHIR(fn: HIRFunction): void { const usedOutsideDeclaringScope = findTemporariesUsedOutsideDeclaringScope(fn); + const temporaries = collectTemporariesSidemap(fn, usedOutsideDeclaringScope); - const {nodes, temporaries, properties} = collectHoistablePropertyLoads( - fn, - usedOutsideDeclaringScope, - ); + const hoistablePropertyLoads = collectHoistablePropertyLoads(fn, temporaries); const scopeDeps = collectDependencies( fn, usedOutsideDeclaringScope, temporaries, - properties, ); /** @@ -65,7 +63,7 @@ export function propagateScopeDependenciesHIR(fn: HIRFunction): void { * Step 2: Mark hoistable dependencies, given the basic block in * which the scope begins. */ - recordHoistablePropertyReads(nodes, scope.id, tree); + recordHoistablePropertyReads(hoistablePropertyLoads, scope.id, tree); const candidates = tree.deriveMinimalDependencies(); for (const candidateDep of candidates) { if ( @@ -144,6 +142,84 @@ function findTemporariesUsedOutsideDeclaringScope( return usedOutsideDeclaringScope; } +/** + * @returns mapping of LoadLocal and PropertyLoad to the source of the load. + * ```js + * // source + * foo(a.b); + * + * // HIR: a potential sidemap is {0: a, 1: a.b, 2: foo} + * $0 = LoadLocal 'a' + * $1 = PropertyLoad $0, 'b' + * $2 = LoadLocal 'foo' + * $3 = CallExpression $2($1) + * ``` + * Only map LoadLocal and PropertyLoad lvalues to their source if we know that + * reordering the read (from the time-of-load to time-of-use) is valid. + * + * If a LoadLocal or PropertyLoad instruction is within the reactive scope range + * (a proxy for mutable range) of the load source, later instructions may + * reassign / mutate the source value. Since it's incorrect to reorder these + * load instructions to after their scope ranges, we also do not store them in + * identifier sidemaps. + * + * Take this example (from fixture + * `evaluation-order-mutate-call-after-dependency-load`) + * ```js + * // source + * function useFoo(arg) { + * const arr = [1, 2, 3, ...arg]; + * return [ + * arr.length, + * arr.push(0) + * ]; + * } + * + * // IR pseudocode + * scope @0 { + * $0 = arr = ArrayExpression [1, 2, 3, ...arg] + * $1 = arr.length + * $2 = arr.push(0) + * } + * scope @1 { + * $3 = ArrayExpression [$1, $2] + * } + * ``` + * Here, it's invalid for scope@1 to take `arr.length` as a dependency instead + * of $1, as the evaluation of `arr.length` changes between instructions $1 and + * $3. We do not track $1 -> arr.length in this case. + */ +function collectTemporariesSidemap( + fn: HIRFunction, + usedOutsideDeclaringScope: ReadonlySet, +): ReadonlyMap { + const temporaries = new Map(); + for (const [_, block] of fn.body.blocks) { + for (const instr of block.instructions) { + const {value, lvalue} = instr; + const usedOutside = usedOutsideDeclaringScope.has( + lvalue.identifier.declarationId, + ); + + if (value.kind === 'PropertyLoad' && !usedOutside) { + const property = getProperty(value.object, value.property, temporaries); + temporaries.set(lvalue.identifier.id, property); + } else if ( + value.kind === 'LoadLocal' && + lvalue.identifier.name == null && + value.place.identifier.name !== null && + !usedOutside + ) { + temporaries.set(lvalue.identifier.id, { + identifier: value.place.identifier, + path: [], + }); + } + } + } + return temporaries; +} + type Decl = { id: InstructionId; scope: Stack; @@ -158,18 +234,15 @@ class Context { #dependencies: Stack> = empty(); deps: Map> = new Map(); - #properties: ReadonlyMap; - #temporaries: ReadonlyMap; + #temporaries: ReadonlyMap; #temporariesUsedOutsideScope: ReadonlySet; constructor( temporariesUsedOutsideScope: ReadonlySet, - temporaries: ReadonlyMap, - properties: ReadonlyMap, + temporaries: ReadonlyMap, ) { this.#temporariesUsedOutsideScope = temporariesUsedOutsideScope; this.#temporaries = temporaries; - this.#properties = properties; } enterScope(scope: ReactiveScope): void { @@ -226,10 +299,6 @@ class Context { this.#reassignments.set(identifier, decl); } - resolveTemporary(place: Place): Identifier { - return this.#temporaries.get(place.identifier) ?? place.identifier; - } - // Checks if identifier is a valid dependency in the current scope #checkValidDependency(maybeDependency: ReactiveScopeDependency): boolean { // ref.current access is not a valid dep @@ -281,34 +350,20 @@ class Context { } visitOperand(place: Place): void { - const resolved = this.resolveTemporary(place); /* * if this operand is a temporary created for a property load, try to resolve it to * the expanded Place. Fall back to using the operand as-is. */ - - let dependency: ReactiveScopeDependency | null = null; - if (resolved.name === null) { - const propertyDependency = this.#properties.get(resolved); - if (propertyDependency !== undefined) { - dependency = {...propertyDependency}; - } - } this.visitDependency( - dependency ?? { - identifier: resolved, + this.#temporaries.get(place.identifier.id) ?? { + identifier: place.identifier, path: [], }, ); } visitProperty(object: Place, property: string): void { - const nextDependency = getProperty( - object, - property, - this.#temporaries, - this.#properties, - ); + const nextDependency = getProperty(object, property, this.#temporaries); this.visitDependency(nextDependency); } @@ -441,14 +496,9 @@ function handleInstruction(instr: Instruction, context: Context): void { function collectDependencies( fn: HIRFunction, usedOutsideDeclaringScope: ReadonlySet, - temporaries: ReadonlyMap, - properties: ReadonlyMap, + temporaries: ReadonlyMap, ): Map> { - const context = new Context( - usedOutsideDeclaringScope, - temporaries, - properties, - ); + const context = new Context(usedOutsideDeclaringScope, temporaries); for (const param of fn.params) { if (param.kind === 'Identifier') { diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts index 15b18b7bfb8ad..0d0b37ce58afe 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -13,6 +13,7 @@ import { HIRFunction, Identifier, Instruction, + InstructionId, MutableRange, Place, ReactiveScope, @@ -187,12 +188,14 @@ function mergeLocation(l: SourceLocation, r: SourceLocation): SourceLocation { } // Is the operand mutable at this given instruction -export function isMutable({id}: Instruction, place: Place): boolean { - const range = place.identifier.mutableRange; - return id >= range.start && id < range.end; +export function isMutable(instr: {id: InstructionId}, place: Place): boolean { + return inRange(instr, place.identifier.mutableRange); } -export function inRange({id}: Instruction, range: MutableRange): boolean { +export function inRange( + {id}: {id: InstructionId}, + range: MutableRange, +): boolean { return id >= range.start && id < range.end; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts b/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts index fec5ab6a96e50..6b813d597560c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts @@ -83,7 +83,7 @@ export function getOrInsertDefault( } } -export function Set_union(a: Set, b: Set): Set { +export function Set_union(a: ReadonlySet, b: ReadonlySet): Set { const union = new Set(a); for (const item of b) { union.add(item); @@ -91,7 +91,7 @@ export function Set_union(a: Set, b: Set): Set { return union; } -export function Set_intersect(sets: Array>): Set { +export function Set_intersect(sets: Array>): Set { if (sets.length === 0 || sets.some(s => s.size === 0)) { return new Set(); } else if (sets.length === 1) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-call-after-dependency-load.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-call-after-dependency-load.expect.md new file mode 100644 index 0000000000000..acad3c3092126 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-call-after-dependency-load.expect.md @@ -0,0 +1,83 @@ + +## Input + +```javascript +/** + * Test that we preserve order of evaluation on the following case scope@0 + * ```js + * // simplified HIR + * scope@0 + * ... + * $0 = arr.length + * $1 = arr.push(...) + * + * scope@1 <-- here we should depend on $0 (the value of the property load before the + * mutable call) + * [$0, $1] + * ``` + */ +function useFoo(source: Array): [number, number] { + const arr = [1, 2, 3, ...source]; + return [arr.length, arr.push(0)]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [[5, 6]], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; /** + * Test that we preserve order of evaluation on the following case scope@0 + * ```js + * // simplified HIR + * scope@0 + * ... + * $0 = arr.length + * $1 = arr.push(...) + * + * scope@1 <-- here we should depend on $0 (the value of the property load before the + * mutable call) + * [$0, $1] + * ``` + */ +function useFoo(source) { + const $ = _c(6); + let t0; + let t1; + if ($[0] !== source) { + const arr = [1, 2, 3, ...source]; + t0 = arr.length; + t1 = arr.push(0); + $[0] = source; + $[1] = t0; + $[2] = t1; + } else { + t0 = $[1]; + t1 = $[2]; + } + let t2; + if ($[3] !== t0 || $[4] !== t1) { + t2 = [t0, t1]; + $[3] = t0; + $[4] = t1; + $[5] = t2; + } else { + t2 = $[5]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [[5, 6]], +}; + +``` + +### Eval output +(kind: ok) [5,6] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-call-after-dependency-load.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-call-after-dependency-load.ts new file mode 100644 index 0000000000000..c2fa617f51c73 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-call-after-dependency-load.ts @@ -0,0 +1,23 @@ +/** + * Test that we preserve order of evaluation on the following case scope@0 + * ```js + * // simplified HIR + * scope@0 + * ... + * $0 = arr.length + * $1 = arr.push(...) + * + * scope@1 <-- here we should depend on $0 (the value of the property load before the + * mutable call) + * [$0, $1] + * ``` + */ +function useFoo(source: Array): [number, number] { + const arr = [1, 2, 3, ...source]; + return [arr.length, arr.push(0)]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [[5, 6]], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-store-after-dependency-load.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-store-after-dependency-load.expect.md new file mode 100644 index 0000000000000..b2bf1e36a4878 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-store-after-dependency-load.expect.md @@ -0,0 +1,83 @@ + +## Input + +```javascript +/** + * Test that we preserve order of evaluation on the following case scope@0 + * ```js + * // simplified HIR + * scope@0 + * ... + * $0 = arr.length + * $1 = arr.length = 0 + * + * scope@1 <-- here we should depend on $0 (the value of the property load before the + * property store) + * [$0, $1] + * ``` + */ +function useFoo(source: Array): [number, number] { + const arr = [1, 2, 3, ...source]; + return [arr.length, (arr.length = 0)]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [[5, 6]], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; /** + * Test that we preserve order of evaluation on the following case scope@0 + * ```js + * // simplified HIR + * scope@0 + * ... + * $0 = arr.length + * $1 = arr.length = 0 + * + * scope@1 <-- here we should depend on $0 (the value of the property load before the + * property store) + * [$0, $1] + * ``` + */ +function useFoo(source) { + const $ = _c(6); + let t0; + let t1; + if ($[0] !== source) { + const arr = [1, 2, 3, ...source]; + t0 = arr.length; + t1 = arr.length = 0; + $[0] = source; + $[1] = t0; + $[2] = t1; + } else { + t0 = $[1]; + t1 = $[2]; + } + let t2; + if ($[3] !== t0 || $[4] !== t1) { + t2 = [t0, t1]; + $[3] = t0; + $[4] = t1; + $[5] = t2; + } else { + t2 = $[5]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [[5, 6]], +}; + +``` + +### Eval output +(kind: ok) [5,0] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-store-after-dependency-load.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-store-after-dependency-load.ts new file mode 100644 index 0000000000000..8798cd99c7f77 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/evaluation-order-mutate-store-after-dependency-load.ts @@ -0,0 +1,23 @@ +/** + * Test that we preserve order of evaluation on the following case scope@0 + * ```js + * // simplified HIR + * scope@0 + * ... + * $0 = arr.length + * $1 = arr.length = 0 + * + * scope@1 <-- here we should depend on $0 (the value of the property load before the + * property store) + * [$0, $1] + * ``` + */ +function useFoo(source: Array): [number, number] { + const arr = [1, 2, 3, ...source]; + return [arr.length, (arr.length = 0)]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [[5, 6]], +}; diff --git a/compiler/packages/snap/src/sprout/shared-runtime.ts b/compiler/packages/snap/src/sprout/shared-runtime.ts index 694142d1c463b..0f3e09b12e127 100644 --- a/compiler/packages/snap/src/sprout/shared-runtime.ts +++ b/compiler/packages/snap/src/sprout/shared-runtime.ts @@ -98,7 +98,11 @@ export function setProperty(arg: any, property: any): void { } } -export function setPropertyByKey(arg: any, key: string, property: any): void { +export function setPropertyByKey< + T, + TKey extends keyof T, + TProperty extends T[TKey], +>(arg: T, key: TKey, property: TProperty): T { arg[key] = property; return arg; } From 87936e3fc1c420a24b322c5003ae851fd401944a Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Wed, 11 Sep 2024 19:30:16 -0400 Subject: [PATCH 8/9] Update [ghstack-poisoned] --- .../src/HIR/CollectHoistablePropertyLoads.ts | 30 ++++++------------- 1 file changed, 9 insertions(+), 21 deletions(-) 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 f8c7c86bb3923..f49d022262910 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -166,7 +166,10 @@ class Tree { roots: Map = new Map(); #getOrCreateRoot(identifier: Identifier): PropertyLoadNode { - // roots can always be accessed unconditionally in JS + /** + * Reads from a statically scoped variable are always safe in JS, + * with the exception of TDZ (not addressed by this pass). + */ let rootNode = this.roots.get(identifier); if (rootNode === undefined) { @@ -380,27 +383,12 @@ function deriveNonNull( .map(n => assertNonNull(nonNullObjectsByBlock.get(n))), ); - const prevSize = nonNullObjectsByBlock.get(nodeId)?.size; - nonNullObjectsByBlock.set( - nodeId, - Set_union( - assertNonNull(nonNullObjectsByBlock.get(nodeId)), - neighborAccesses, - ), - ); - traversalState.set(nodeId, 'done'); + const prevObjects = assertNonNull(nonNullObjectsByBlock.get(nodeId)); + const newObjects = Set_union(prevObjects, neighborAccesses); - changed ||= prevSize !== nonNullObjectsByBlock.get(nodeId)!.size; - CompilerError.invariant( - prevSize == null || prevSize <= nonNullObjectsByBlock.get(nodeId)!.size, - { - reason: '[CollectHoistablePropertyLoads] Nodes shrank!', - description: `${nodeId} ${direction} ${prevSize} ${ - nonNullObjectsByBlock.get(nodeId)!.size - }`, - loc: GeneratedSource, - }, - ); + nonNullObjectsByBlock.set(nodeId, newObjects); + traversalState.set(nodeId, 'done'); + changed ||= prevObjects.size !== newObjects.size; return changed; } const fromEntry = new Map>(); From 9e5adea8e7c88cac9885b0fa67a75801b7def2e7 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 12 Sep 2024 16:59:40 -0400 Subject: [PATCH 9/9] Update [ghstack-poisoned] --- .../src/HIR/CollectHoistablePropertyLoads.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 f49d022262910..941c60dea9d4f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -67,7 +67,7 @@ export function collectHoistablePropertyLoads( temporaries: ReadonlyMap, ): ReadonlyMap { const nodes = collectPropertyLoadsInBlocks(fn, temporaries); - deriveNonNull(fn, nodes); + propagateNonNull(fn, nodes); const nodesKeyedByScopeId = new Map(); for (const [_, block] of fn.body.blocks) { @@ -295,7 +295,7 @@ function collectPropertyLoadsInBlocks( return nodes; } -function deriveNonNull( +function propagateNonNull( fn: HIRFunction, nodes: ReadonlyMap, ): void { @@ -316,7 +316,7 @@ function deriveNonNull( * can assume are non-null can be calculated from the following: * X = Union(Intersect(X_neighbors), X) */ - function recursivelyDeriveNonNull( + function recursivelyPropagateNonNull( nodeId: BlockId, direction: 'forward' | 'backward', traversalState: Map, @@ -347,7 +347,7 @@ function deriveNonNull( let changed = false; for (const pred of neighbors) { if (!traversalState.has(pred)) { - const neighborChanged = recursivelyDeriveNonNull( + const neighborChanged = recursivelyPropagateNonNull( pred, direction, traversalState, @@ -407,7 +407,7 @@ function deriveNonNull( i++; changed = false; for (const [blockId] of fn.body.blocks) { - const forwardChanged = recursivelyDeriveNonNull( + const forwardChanged = recursivelyPropagateNonNull( blockId, 'forward', traversalState, @@ -417,7 +417,7 @@ function deriveNonNull( } traversalState.clear(); for (const [blockId] of reversedBlocks) { - const backwardChanged = recursivelyDeriveNonNull( + const backwardChanged = recursivelyPropagateNonNull( blockId, 'backward', traversalState,