diff --git a/compiler/.gitignore b/compiler/.gitignore index d41f59333aa09..1f35fa334a6e9 100644 --- a/compiler/.gitignore +++ b/compiler/.gitignore @@ -6,6 +6,9 @@ debug/ target/ +nocommit*.js +nocommit*.expect.md + # These are backup files generated by rustfmt **/*.rs.bk diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 99b8c189ee0fd..991ac9e72d16c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -300,6 +300,10 @@ export type FunctionEffect = places: ReadonlySet; effect: Effect; loc: SourceLocation; + } + | { + kind: 'CaptureEffect'; + places: ReadonlySet; }; /* diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index c8182c9e72a7c..5325494f57ab8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -546,12 +546,23 @@ export function printInstructionValue(instrValue: ReactiveValue): string { const effects = instrValue.loweredFunc.func.effects ?.map(effect => { - if (effect.kind === 'ContextMutation') { - return `ContextMutation places=[${[...effect.places] - .map(place => printPlace(place)) - .join(', ')}] effect=${effect.effect}`; - } else { - return `GlobalMutation`; + switch (effect.kind) { + case 'ContextMutation': { + return `ContextMutation places=[${[...effect.places] + .map(place => printPlace(place)) + .join(', ')}] effect=${effect.effect}`; + } + case 'GlobalMutation': { + return 'GlobalMutation'; + } + case 'ReactMutation': { + return 'ReactMutation'; + } + case 'CaptureEffect': { + return `CaptureEffect places=[${[...effect.places] + .map(place => printPlace(place)) + .join(', ')}]`; + } } }) .join(', ') ?? ''; @@ -720,7 +731,7 @@ function isMutable(range: MutableRange): boolean { } const DEBUG_MUTABLE_RANGES = false; -function printMutableRange(identifier: Identifier): string { +export function printMutableRange(identifier: Identifier): string { if (DEBUG_MUTABLE_RANGES) { // if debugging, print both the identifier and scope range if they differ const range = identifier.mutableRange; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts index a439b4cd01232..6a4da94d706e2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts @@ -11,6 +11,7 @@ import { HIRFunction, Identifier, LoweredFunction, + Place, isRefOrRefValue, makeInstructionId, } from '../HIR'; @@ -19,6 +20,12 @@ import {inferReactiveScopeVariables} from '../ReactiveScopes'; import {rewriteInstructionKindsBasedOnReassignment} from '../SSA'; import {inferMutableRanges} from './InferMutableRanges'; import inferReferenceEffects from './InferReferenceEffects'; +import DisjointSet from '../Utils/DisjointSet'; +import { + eachInstructionLValue, + eachInstructionValueOperand, +} from '../HIR/visitors'; +import {Iterable_some} from '../Utils/utils'; export default function analyseFunctions(func: HIRFunction): void { for (const [_, block] of func.body.blocks) { @@ -26,8 +33,8 @@ export default function analyseFunctions(func: HIRFunction): void { switch (instr.value.kind) { case 'ObjectMethod': case 'FunctionExpression': { - lower(instr.value.loweredFunc.func); - infer(instr.value.loweredFunc); + const aliases = lower(instr.value.loweredFunc.func); + infer(instr.value.loweredFunc, aliases); /** * Reset mutable range for outer inferReferenceEffects @@ -44,11 +51,11 @@ export default function analyseFunctions(func: HIRFunction): void { } } -function lower(func: HIRFunction): void { +function lower(func: HIRFunction): DisjointSet { analyseFunctions(func); inferReferenceEffects(func, {isFunctionExpression: true}); deadCodeElimination(func); - inferMutableRanges(func); + const aliases = inferMutableRanges(func); rewriteInstructionKindsBasedOnReassignment(func); inferReactiveScopeVariables(func); func.env.logger?.debugLogIRs?.({ @@ -56,9 +63,61 @@ function lower(func: HIRFunction): void { name: 'AnalyseFunction (inner)', value: func, }); + inferAliasesForCapturing(func, aliases); + return aliases; +} + +/** + * The alias sets returned by InferMutableRanges() accounts only for aliases that + * are known to mutate together. Notably this skips cases where a value is captured + * into some other value, but neither is subsequently mutated. An example is pushing + * a mutable value onto an array, where neither the array or value are subsequently + * mutated. + * + * This function extends the aliases sets to account for such capturing, so that we + * can detect cases where one of the values in a set is mutated later (in an outer function) + * we can correctly infer them as mutating together. + */ +function inferAliasesForCapturing( + fn: HIRFunction, + aliases: DisjointSet, +): void { + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + const {lvalue, value} = instr; + const hasCapture = + lvalue.effect === Effect.Store || + Iterable_some( + eachInstructionValueOperand(value), + operand => operand.effect === Effect.Capture, + ); + if (!hasCapture) { + continue; + } + const operands: Array = []; + for (const lvalue of eachInstructionLValue(instr)) { + operands.push(lvalue.identifier); + } + for (const operand of eachInstructionValueOperand(instr.value)) { + if ( + operand.effect === Effect.Store || + operand.effect === Effect.Mutate || + operand.effect === Effect.Capture + ) { + operands.push(operand.identifier); + } + } + if (operands.length > 1) { + aliases.union(operands); + } + } + } } -function infer(loweredFunc: LoweredFunction): void { +function infer( + loweredFunc: LoweredFunction, + aliases: DisjointSet, +): void { for (const operand of loweredFunc.func.context) { const identifier = operand.identifier; CompilerError.invariant(operand.effect === Effect.Unknown, { @@ -85,6 +144,23 @@ function infer(loweredFunc: LoweredFunction): void { operand.effect = Effect.Read; } } + const contextIdentifiers = new Map( + loweredFunc.func.context.map(place => [place.identifier, place]), + ); + for (const set of aliases.buildSets()) { + const contextOperands: Set = new Set( + [...set] + .map(identifier => contextIdentifiers.get(identifier)) + .filter(place => place != null) as Array, + ); + if (contextOperands.size !== 0) { + loweredFunc.func.effects ??= []; + loweredFunc.func.effects?.push({ + kind: 'CaptureEffect', + places: contextOperands, + }); + } + } } function isMutatedOrReassigned(id: Identifier): boolean { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts index 80422c8391f46..fd7ec6becd701 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts @@ -60,6 +60,10 @@ function inferInstr( alias = instrValue.value; break; } + case 'IteratorNext': { + alias = instrValue.collection; + break; + } default: return; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasesForFunctionCaptureEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasesForFunctionCaptureEffects.ts new file mode 100644 index 0000000000000..8bc7cee4f9a79 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasesForFunctionCaptureEffects.ts @@ -0,0 +1,33 @@ +/** + * 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 {HIRFunction, Identifier} from '../HIR/HIR'; +import DisjointSet from '../Utils/DisjointSet'; + +export function inferAliasForFunctionCaptureEffects( + func: HIRFunction, + aliases: DisjointSet, +): void { + for (const [_, block] of func.body.blocks) { + for (const instr of block.instructions) { + const {value} = instr; + if ( + value.kind !== 'FunctionExpression' && + value.kind !== 'ObjectMethod' + ) { + continue; + } + const loweredFunction = value.loweredFunc.func; + for (const effect of loweredFunction.effects ?? []) { + if (effect.kind !== 'CaptureEffect') { + continue; + } + aliases.union([...effect.places].map(place => place.identifier)); + } + } + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts index a58ae440219b9..ba09bcba1e9a4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts @@ -95,45 +95,58 @@ function inheritFunctionEffects( return effects .flatMap(effect => { - if (effect.kind === 'GlobalMutation' || effect.kind === 'ReactMutation') { - return [effect]; - } else { - const effects: Array = []; - CompilerError.invariant(effect.kind === 'ContextMutation', { - reason: 'Expected ContextMutation', - loc: null, - }); - /** - * Contextual effects need to be replayed against the current inference - * state, which may know more about the value to which the effect applied. - * The main cases are: - * 1. The mutated context value is _still_ a context value in the current scope, - * so we have to continue propagating the original context mutation. - * 2. The mutated context value is a mutable value in the current scope, - * so the context mutation was fine and we can skip propagating the effect. - * 3. The mutated context value is an immutable value in the current scope, - * resulting in a non-ContextMutation FunctionEffect. We propagate that new, - * more detailed effect to the current function context. - */ - for (const place of effect.places) { - if (state.isDefined(place)) { - const replayedEffect = inferOperandEffect(state, { - ...place, - loc: effect.loc, - effect: effect.effect, - }); - if (replayedEffect != null) { - if (replayedEffect.kind === 'ContextMutation') { - // Case 1, still a context value so propagate the original effect - effects.push(effect); - } else { - // Case 3, immutable value so propagate the more precise effect - effects.push(replayedEffect); - } - } // else case 2, local mutable value so this effect was fine + switch (effect.kind) { + case 'GlobalMutation': + case 'ReactMutation': { + return [effect]; + } + case 'ContextMutation': { + const effects: Array = []; + CompilerError.invariant(effect.kind === 'ContextMutation', { + reason: 'Expected ContextMutation', + loc: null, + }); + /** + * Contextual effects need to be replayed against the current inference + * state, which may know more about the value to which the effect applied. + * The main cases are: + * 1. The mutated context value is _still_ a context value in the current scope, + * so we have to continue propagating the original context mutation. + * 2. The mutated context value is a mutable value in the current scope, + * so the context mutation was fine and we can skip propagating the effect. + * 3. The mutated context value is an immutable value in the current scope, + * resulting in a non-ContextMutation FunctionEffect. We propagate that new, + * more detailed effect to the current function context. + */ + for (const place of effect.places) { + if (state.isDefined(place)) { + const replayedEffect = inferOperandEffect(state, { + ...place, + loc: effect.loc, + effect: effect.effect, + }); + if (replayedEffect != null) { + if (replayedEffect.kind === 'ContextMutation') { + // Case 1, still a context value so propagate the original effect + effects.push(effect); + } else { + // Case 3, immutable value so propagate the more precise effect + effects.push(replayedEffect); + } + } // else case 2, local mutable value so this effect was fine + } } + return effects; + } + case 'CaptureEffect': { + return []; + } + default: { + assertExhaustive( + effect, + `Unexpected effect kind '${(effect as any).kind}'`, + ); } - return effects; } }) .filter((effect): effect is FunctionEffect => effect != null); @@ -298,26 +311,31 @@ export function inferTerminalFunctionEffects( export function transformFunctionEffectErrors( functionEffects: Array, ): Array { - return functionEffects.map(eff => { - switch (eff.kind) { - case 'ReactMutation': - case 'GlobalMutation': { - return eff.error; - } - case 'ContextMutation': { - return { - severity: ErrorSeverity.Invariant, - reason: `Unexpected ContextMutation in top-level function effects`, - loc: eff.loc, - }; + return functionEffects + .map(eff => { + switch (eff.kind) { + case 'ReactMutation': + case 'GlobalMutation': { + return eff.error; + } + case 'ContextMutation': { + return { + severity: ErrorSeverity.Invariant, + reason: `Unexpected ContextMutation in top-level function effects`, + loc: eff.loc, + }; + } + case 'CaptureEffect': { + return null; + } + default: + assertExhaustive( + eff, + `Unexpected function effect kind \`${(eff as any).kind}\``, + ); } - default: - assertExhaustive( - eff, - `Unexpected function effect kind \`${(eff as any).kind}\``, - ); - } - }); + }) + .filter(eff => eff != null) as Array; } function isEffectSafeOutsideRender(effect: FunctionEffect): boolean { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts index 624c302fbf7ee..3b022c13f7839 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts @@ -5,16 +5,21 @@ * LICENSE file in the root directory of this source tree. */ +import prettyFormat from 'pretty-format'; import {HIRFunction, Identifier} from '../HIR/HIR'; +import DisjointSet from '../Utils/DisjointSet'; import {inferAliasForUncalledFunctions} from './InerAliasForUncalledFunctions'; import {inferAliases} from './InferAlias'; +import {inferAliasForFunctionCaptureEffects} from './InferAliasesForFunctionCaptureEffects'; import {inferAliasForPhis} from './InferAliasForPhis'; import {inferAliasForStores} from './InferAliasForStores'; import {inferMutableLifetimes} from './InferMutableLifetimes'; import {inferMutableRangesForAlias} from './InferMutableRangesForAlias'; +import {inferMutableRangesForComutation} from './InferMutableRangesForComutation'; import {inferTryCatchAliases} from './InferTryCatchAliases'; +import {printIdentifier, printMutableRange} from '../HIR/PrintHIR'; -export function inferMutableRanges(ir: HIRFunction): void { +export function inferMutableRanges(ir: HIRFunction): DisjointSet { // Infer mutable ranges for non fields inferMutableLifetimes(ir, false); @@ -30,18 +35,22 @@ export function inferMutableRanges(ir: HIRFunction): void { * Eagerly canonicalize so that if nothing changes we can bail out * after a single iteration */ - let prevAliases: Map = aliases.canonicalize(); + let prevAliases: Map = canonicalize(aliases); while (true) { // Infer mutable ranges for aliases that are not fields inferMutableRangesForAlias(ir, aliases); + inferMutableRangesForComutation(ir); + // Update aliasing information of fields inferAliasForStores(ir, aliases); + inferAliasForFunctionCaptureEffects(ir, aliases); + // Update aliasing information of phis inferAliasForPhis(ir, aliases); - const nextAliases = aliases.canonicalize(); + const nextAliases = canonicalize(aliases); if (areEqualMaps(prevAliases, nextAliases)) { break; } @@ -73,20 +82,57 @@ export function inferMutableRanges(ir: HIRFunction): void { * but does not modify values that `y` "contains" such as the * object literal or `z`. */ - prevAliases = aliases.canonicalize(); + prevAliases = canonicalize(aliases); while (true) { inferMutableRangesForAlias(ir, aliases); + inferMutableRangesForComutation(ir); inferAliasForPhis(ir, aliases); inferAliasForUncalledFunctions(ir, aliases); - const nextAliases = aliases.canonicalize(); + const nextAliases = canonicalize(aliases); if (areEqualMaps(prevAliases, nextAliases)) { break; } prevAliases = nextAliases; } + + return aliases; +} + +export function debugAliases(aliases: DisjointSet): void { + console.log( + prettyFormat( + aliases + .buildSets() + .map(set => + [...set].map( + ident => printIdentifier(ident) + printMutableRange(ident), + ), + ), + ), + ); +} + +/** + * Canonicalizes the alias set and mutable range information calculated at the current time. + * The returned value maps each identifier in the program to the root identifier of its alias + * set and the the mutable range of that set. + * + * This ensures that we fixpoint the mutable ranges themselves and not just the alias sets. + */ +function canonicalize( + aliases: DisjointSet, +): Map { + const entries = new Map(); + aliases.forEach((item, root) => { + entries.set( + item, + `${root.id}:${root.mutableRange.start}:${root.mutableRange.end}`, + ); + }); + return entries; } -function areEqualMaps(a: Map, b: Map): boolean { +function areEqualMaps(a: Map, b: Map): boolean { if (a.size !== b.size) { return false; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForComutation.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForComutation.ts new file mode 100644 index 0000000000000..09e347671427b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForComutation.ts @@ -0,0 +1,91 @@ +/** + * 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 { + HIRFunction, + Identifier, + isRefOrRefValue, + makeInstructionId, +} from '../HIR'; +import {eachInstructionOperand} from '../HIR/visitors'; +import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables'; + +/** + * Finds instructions with operands that co-mutate and extends all their mutable ranges + * to end at the same point (the highest `end` value of the group). Note that the + * alias sets used in InferMutableRanges are meant for values that strictly alias: + * a mutation of one value in the set would directly modify the same object as some + * other value in the set. + * + * However, co-mutation can cause an alias to one object to be stored within another object, + * for example: + * + * ``` + * const a = {}; + * const b = {}; + * const f = () => b.c; // + * setProperty(a, 'b', b); // equiv to a.b = b + * + * a.b.c = 'c'; // this mutates b! + * ``` + * + * Here, the co-mutation in `setProperty(a, 'b', b)` means that a reference to b may be stored + * in a, vice-versa, or both. We need to extend the mutable range of both a and b to reflect + * the fact the values may mutate together. + * + * Previously this was implemented in InferReactiveScopeVariables, but that is too late: + * we need this to be part of the InferMutableRanges fixpoint iteration to account for functions + * like `f` in the example, which capture a reference to a value that may change later. `f` + * cannot be independently memoized from the `setProperty()` call due to the co-mutation. + * + * See aliased-capture-mutate and aliased-capture-aliased-mutate fixtures for examples. + */ +export function inferMutableRangesForComutation(fn: HIRFunction): void { + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + let operands: Array | null = null; + for (const operand of eachInstructionOperand(instr)) { + if ( + isMutable(instr, operand) && + operand.identifier.mutableRange.start > 0 + ) { + if ( + instr.value.kind === 'FunctionExpression' || + instr.value.kind === 'ObjectMethod' + ) { + if (operand.identifier.type.kind === 'Primitive') { + continue; + } + } + operands ??= []; + operands.push(operand.identifier); + } + } + if (operands != null) { + // Find the last instruction which mutates any of the mutable operands + let lastMutatingInstructionId = makeInstructionId(0); + for (const id of operands) { + if (id.mutableRange.end > lastMutatingInstructionId) { + lastMutatingInstructionId = id.mutableRange.end; + } + } + + /** + * Update all mutable operands's mutable ranges to end at the same point + */ + for (const id of operands) { + if ( + id.mutableRange.end < lastMutatingInstructionId && + !isRefOrRefValue(id) + ) { + id.mutableRange.end = lastMutatingInstructionId; + } + } + } + } + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts index d1546038edcbe..2de141bbd3c11 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts @@ -31,13 +31,13 @@ import { isArrayType, isMapType, isMutableEffect, - isObjectType, isSetType, + isObjectType, } from '../HIR/HIR'; import {FunctionSignature} from '../HIR/ObjectShape'; import { printIdentifier, - printMixedHIR, + printInstructionValue, printPlace, printSourceLocation, } from '../HIR/PrintHIR'; @@ -48,7 +48,7 @@ import { eachTerminalOperand, eachTerminalSuccessor, } from '../HIR/visitors'; -import {assertExhaustive} from '../Utils/utils'; +import {assertExhaustive, retainWhere} from '../Utils/utils'; import { inferTerminalFunctionEffects, inferInstructionFunctionEffects, @@ -521,7 +521,7 @@ class InferenceState { * `expected valueKind to be 'Mutable' but found to be \`${valueKind}\`` * ); */ - effect = isObjectType(place.identifier) ? Effect.Store : Effect.Mutate; + effect = Effect.Store; break; } case Effect.Capture: { @@ -669,7 +669,10 @@ class InferenceState { } for (const [value, kind] of this.#values) { const id = identify(value); - result.values[id] = {kind, value: printMixedHIR(value)}; + result.values[id] = { + abstract: this.debugAbstractValue(kind), + value: printInstructionValue(value), + }; } for (const [variable, values] of this.#variables) { result.variables[`$${variable}`] = [...values].map(identify); @@ -677,6 +680,14 @@ class InferenceState { return result; } + debugAbstractValue(value: AbstractValue): any { + return { + kind: value.kind, + context: [...value.context].map(printPlace), + reason: [...value.reason], + }; + } + inferPhi(phi: Phi): void { const values: Set = new Set(); for (const [_, operand] of phi.operands) { @@ -902,19 +913,11 @@ function inferBlock( break; } case 'ArrayExpression': { - const contextRefOperands = getContextRefOperand(state, instrValue); - const valueKind: AbstractValue = - contextRefOperands.length > 0 - ? { - kind: ValueKind.Context, - reason: new Set([ValueReason.Other]), - context: new Set(contextRefOperands), - } - : { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }; + const valueKind: AbstractValue = { + kind: ValueKind.Mutable, + reason: new Set([ValueReason.Other]), + context: new Set(), + }; for (const element of instrValue.elements) { if (element.kind === 'Spread') { @@ -935,6 +938,7 @@ function inferBlock( let _: 'Hole' = element.kind; } } + state.initialize(instrValue, valueKind); state.define(instr.lvalue, instrValue); instr.lvalue.effect = Effect.Store; @@ -954,19 +958,11 @@ function inferBlock( break; } case 'ObjectExpression': { - const contextRefOperands = getContextRefOperand(state, instrValue); - const valueKind: AbstractValue = - contextRefOperands.length > 0 - ? { - kind: ValueKind.Context, - reason: new Set([ValueReason.Other]), - context: new Set(contextRefOperands), - } - : { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }; + const valueKind: AbstractValue = { + kind: ValueKind.Mutable, + reason: new Set([ValueReason.Other]), + context: new Set(), + }; for (const property of instrValue.properties) { switch (property.kind) { @@ -1190,6 +1186,35 @@ function inferBlock( ); hasMutableOperand ||= isMutableEffect(operand.effect, operand.loc); } + + /* + * Filter CaptureEffects to remove values that are immutable and don't + * need to be tracked for aliasing + */ + const effects = instrValue.loweredFunc.func.effects; + if (effects != null && effects.length !== 0) { + retainWhere(effects, effect => { + if (effect.kind !== 'CaptureEffect') { + return true; + } + const places: Set = new Set(); + for (const place of effect.places) { + const kind = state.kind(place); + if ( + kind.kind === ValueKind.Context || + kind.kind === ValueKind.MaybeFrozen || + kind.kind === ValueKind.Mutable + ) { + places.add(place); + } + } + if (places.size === 0) { + return false; + } + effect.places = places; + return true; + }); + } /* * If a closure did not capture any mutable values, then we can consider it to be * frozen, which allows it to be independently memoized. @@ -1280,20 +1305,18 @@ function inferBlock( break; } case 'PropertyStore': { - const effect = - state.kind(instrValue.object).kind === ValueKind.Context - ? Effect.ConditionallyMutate - : Effect.Capture; state.referenceAndRecordEffects( freezeActions, instrValue.value, - effect, + Effect.Capture, ValueReason.Other, ); state.referenceAndRecordEffects( freezeActions, instrValue.object, - Effect.Store, + isObjectType(instrValue.object.identifier) + ? Effect.Store + : Effect.Mutate, ValueReason.Other, ); @@ -1320,25 +1343,21 @@ function inferBlock( state.referenceAndRecordEffects( freezeActions, instrValue.object, - Effect.Read, + Effect.Capture, ValueReason.Other, ); const lvalue = instr.lvalue; - lvalue.effect = Effect.ConditionallyMutate; + lvalue.effect = Effect.Store; state.initialize(instrValue, state.kind(instrValue.object)); state.define(lvalue, instrValue); continuation = {kind: 'funeffects'}; break; } case 'ComputedStore': { - const effect = - state.kind(instrValue.object).kind === ValueKind.Context - ? Effect.ConditionallyMutate - : Effect.Capture; state.referenceAndRecordEffects( freezeActions, instrValue.value, - effect, + Effect.Capture, ValueReason.Other, ); state.referenceAndRecordEffects( @@ -1350,7 +1369,9 @@ function inferBlock( state.referenceAndRecordEffects( freezeActions, instrValue.object, - Effect.Store, + isObjectType(instrValue.object.identifier) + ? Effect.Store + : Effect.Mutate, ValueReason.Other, ); @@ -1387,7 +1408,7 @@ function inferBlock( state.referenceAndRecordEffects( freezeActions, instrValue.object, - Effect.Read, + Effect.Capture, ValueReason.Other, ); state.referenceAndRecordEffects( @@ -1397,7 +1418,7 @@ function inferBlock( ValueReason.Other, ); const lvalue = instr.lvalue; - lvalue.effect = Effect.ConditionallyMutate; + lvalue.effect = Effect.Store; state.initialize(instrValue, state.kind(instrValue.object)); state.define(lvalue, instrValue); continuation = {kind: 'funeffects'}; @@ -1811,7 +1832,9 @@ function inferBlock( state.isDefined(operand) && ((operand.identifier.type.kind === 'Function' && state.isFunctionExpression) || - state.kind(operand).kind === ValueKind.Context) + state.kind(operand).kind === ValueKind.Context || + (state.kind(operand).kind === ValueKind.Mutable && + state.isFunctionExpression)) ) { /** * Returned values should only be typed as 'frozen' if they are both (1) @@ -1838,22 +1861,6 @@ function inferBlock( ); } -function getContextRefOperand( - state: InferenceState, - instrValue: InstructionValue, -): Array { - const result = []; - for (const place of eachInstructionValueOperand(instrValue)) { - if ( - state.isDefined(place) && - state.kind(place).kind === ValueKind.Context - ) { - result.push(place); - } - } - return result; -} - export function getFunctionCallSignature( env: Environment, type: Type, diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 33a124dcec6e2..39b2f1489809f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -1183,7 +1183,7 @@ function codegenTerminal( ? codegenPlaceToExpression(cx, case_.test) : null; const block = codegenBlock(cx, case_.block!); - return t.switchCase(test, [block]); + return t.switchCase(test, block.body.length === 0 ? [] : [block]); }), ); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.expect.md new file mode 100644 index 0000000000000..5b9900a120fd2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.expect.md @@ -0,0 +1,71 @@ + +## Input + +```javascript +// @flow @enableTransitivelyFreezeFunctionExpressions:false +import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime'; + +function useFoo({a, b}: {a: number, b: number}) { + const x = []; + const y = {value: a}; + + arrayPush(x, y); // x and y co-mutate + const y_alias = y; + const cb = () => y_alias.value; + setPropertyByKey(x[0], 'value', b); // might overwrite y.value + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{a: 2, b: 10}], + sequentialRenders: [ + {a: 2, b: 10}, + {a: 2, b: 11}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { arrayPush, setPropertyByKey, Stringify } from "shared-runtime"; + +function useFoo(t0) { + const $ = _c(3); + const { a, b } = t0; + let t1; + if ($[0] !== a || $[1] !== b) { + const x = []; + const y = { value: a }; + + arrayPush(x, y); + const y_alias = y; + const cb = () => y_alias.value; + setPropertyByKey(x[0], "value", b); + t1 = ; + $[0] = a; + $[1] = b; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ a: 2, b: 10 }], + sequentialRenders: [ + { a: 2, b: 10 }, + { a: 2, b: 11 }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
+
{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.js new file mode 100644 index 0000000000000..d1f92144d4c3d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.js @@ -0,0 +1,22 @@ +// @flow @enableTransitivelyFreezeFunctionExpressions:false +import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime'; + +function useFoo({a, b}: {a: number, b: number}) { + const x = []; + const y = {value: a}; + + arrayPush(x, y); // x and y co-mutate + const y_alias = y; + const cb = () => y_alias.value; + setPropertyByKey(x[0], 'value', b); // might overwrite y.value + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{a: 2, b: 10}], + sequentialRenders: [ + {a: 2, b: 10}, + {a: 2, b: 11}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.expect.md similarity index 62% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.expect.md index c35efe6a16bb5..0bd15a268ee36 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.expect.md @@ -5,19 +5,6 @@ // @flow @enableTransitivelyFreezeFunctionExpressions:false import {setPropertyByKey, Stringify} from 'shared-runtime'; -/** - * Variation of bug in `bug-aliased-capture-aliased-mutate` - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- */ - function useFoo({a}: {a: number, b: number}) { const arr = []; const obj = {value: a}; @@ -46,7 +33,7 @@ import { c as _c } from "react/compiler-runtime"; import { setPropertyByKey, Stringify } from "shared-runtime"; function useFoo(t0) { - const $ = _c(4); + const $ = _c(2); const { a } = t0; let t1; if ($[0] !== a) { @@ -55,15 +42,7 @@ function useFoo(t0) { setPropertyByKey(obj, "arr", arr); const obj_alias = obj; - let t2; - if ($[2] !== obj_alias.arr.length) { - t2 = () => obj_alias.arr.length; - $[2] = obj_alias.arr.length; - $[3] = t2; - } else { - t2 = $[3]; - } - const cb = t2; + const cb = () => obj_alias.arr.length; for (let i = 0; i < a; i++) { arr.push(i); } @@ -84,4 +63,7 @@ export const FIXTURE_ENTRYPOINT = { }; ``` - \ No newline at end of file + +### Eval output +(kind: ok)
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
+
{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.js similarity index 52% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.js index a7e57672665f6..be12a7c9be8e7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.js @@ -1,19 +1,6 @@ // @flow @enableTransitivelyFreezeFunctionExpressions:false import {setPropertyByKey, Stringify} from 'shared-runtime'; -/** - * Variation of bug in `bug-aliased-capture-aliased-mutate` - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- */ - function useFoo({a}: {a: number, b: number}) { const arr = []; const obj = {value: a}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-map-captures-receiver-noAlias.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-map-captures-receiver-noAlias.expect.md index 1680386c741a3..efd094c1a5360 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-map-captures-receiver-noAlias.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-map-captures-receiver-noAlias.expect.md @@ -23,34 +23,18 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(6); + const $ = _c(2); let t0; if ($[0] !== props.a) { - t0 = { a: props.a }; + const item = { a: props.a }; + const items = [item]; + t0 = items.map(_temp); $[0] = props.a; $[1] = t0; } else { t0 = $[1]; } - const item = t0; - let t1; - if ($[2] !== item) { - t1 = [item]; - $[2] = item; - $[3] = t1; - } else { - t1 = $[3]; - } - const items = t1; - let t2; - if ($[4] !== items) { - t2 = items.map(_temp); - $[4] = items; - $[5] = t2; - } else { - t2 = $[5]; - } - const mapped = t2; + const mapped = t0; return mapped; } function _temp(item_0) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md index d5bf094ef4ff9..a225812dbd306 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/block-scoping-switch-variable-scoping.expect.md @@ -50,8 +50,7 @@ function Component(props) { console.log(handlers.value); break bb0; } - default: { - } + default: } t0 = handlers; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md deleted file mode 100644 index d0ad9e2f9dbe5..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md +++ /dev/null @@ -1,107 +0,0 @@ - -## Input - -```javascript -// @flow @enableTransitivelyFreezeFunctionExpressions:false -import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime'; - -/** - * 1. `InferMutableRanges` derives the mutable range of identifiers and their - * aliases from `LoadLocal`, `PropertyLoad`, etc - * - After this pass, y's mutable range only extends to `arrayPush(x, y)` - * - We avoid assigning mutable ranges to loads after y's mutable range, as - * these are working with an immutable value. As a result, `LoadLocal y` and - * `PropertyLoad y` do not get mutable ranges - * 2. `InferReactiveScopeVariables` extends mutable ranges and creates scopes, - * as according to the 'co-mutation' of different values - * - Here, we infer that - * - `arrayPush(y, x)` might alias `x` and `y` to each other - * - `setPropertyKey(x, ...)` may mutate both `x` and `y` - * - This pass correctly extends the mutable range of `y` - * - Since we didn't run `InferMutableRange` logic again, the LoadLocal / - * PropertyLoads still don't have a mutable range - * - * Note that the this bug is an edge case. Compiler output is only invalid for: - * - function expressions with - * `enableTransitivelyFreezeFunctionExpressions:false` - * - functions that throw and get retried without clearing the memocache - * - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- */ -function useFoo({a, b}: {a: number, b: number}) { - const x = []; - const y = {value: a}; - - arrayPush(x, y); // x and y co-mutate - const y_alias = y; - const cb = () => y_alias.value; - setPropertyByKey(x[0], 'value', b); // might overwrite y.value - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{a: 2, b: 10}], - sequentialRenders: [ - {a: 2, b: 10}, - {a: 2, b: 11}, - ], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -import { arrayPush, setPropertyByKey, Stringify } from "shared-runtime"; - -function useFoo(t0) { - const $ = _c(5); - const { a, b } = t0; - let t1; - if ($[0] !== a || $[1] !== b) { - const x = []; - const y = { value: a }; - - arrayPush(x, y); - const y_alias = y; - let t2; - if ($[3] !== y_alias.value) { - t2 = () => y_alias.value; - $[3] = y_alias.value; - $[4] = t2; - } else { - t2 = $[4]; - } - const cb = t2; - setPropertyByKey(x[0], "value", b); - t1 = ; - $[0] = a; - $[1] = b; - $[2] = t1; - } else { - t1 = $[2]; - } - return t1; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{ a: 2, b: 10 }], - sequentialRenders: [ - { a: 2, b: 10 }, - { a: 2, b: 11 }, - ], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js deleted file mode 100644 index c46ecd6250b42..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js +++ /dev/null @@ -1,53 +0,0 @@ -// @flow @enableTransitivelyFreezeFunctionExpressions:false -import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime'; - -/** - * 1. `InferMutableRanges` derives the mutable range of identifiers and their - * aliases from `LoadLocal`, `PropertyLoad`, etc - * - After this pass, y's mutable range only extends to `arrayPush(x, y)` - * - We avoid assigning mutable ranges to loads after y's mutable range, as - * these are working with an immutable value. As a result, `LoadLocal y` and - * `PropertyLoad y` do not get mutable ranges - * 2. `InferReactiveScopeVariables` extends mutable ranges and creates scopes, - * as according to the 'co-mutation' of different values - * - Here, we infer that - * - `arrayPush(y, x)` might alias `x` and `y` to each other - * - `setPropertyKey(x, ...)` may mutate both `x` and `y` - * - This pass correctly extends the mutable range of `y` - * - Since we didn't run `InferMutableRange` logic again, the LoadLocal / - * PropertyLoads still don't have a mutable range - * - * Note that the this bug is an edge case. Compiler output is only invalid for: - * - function expressions with - * `enableTransitivelyFreezeFunctionExpressions:false` - * - functions that throw and get retried without clearing the memocache - * - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- */ -function useFoo({a, b}: {a: number, b: number}) { - const x = []; - const y = {value: a}; - - arrayPush(x, y); // x and y co-mutate - const y_alias = y; - const cb = () => y_alias.value; - setPropertyByKey(x[0], 'value', b); // might overwrite y.value - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{a: 2, b: 10}], - sequentialRenders: [ - {a: 2, b: 10}, - {a: 2, b: 11}, - ], -}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dominator.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dominator.expect.md index e878d4fb7f825..508a7b62581d7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dominator.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dominator.expect.md @@ -67,8 +67,7 @@ function Component(props) { case "b": { break bb1; } - case "c": { - } + case "c": default: { x = 6; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-access-assignment.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-access-assignment.expect.md new file mode 100644 index 0000000000000..8b4dbc8f863d6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-access-assignment.expect.md @@ -0,0 +1,83 @@ + +## Input + +```javascript +function Component({a, b, c}) { + // This is an object version of array-access-assignment.js + // Meant to confirm that object expressions and PropertyStore/PropertyLoad with strings + // works equivalently to array expressions and property accesses with numeric indices + const x = {zero: a}; + const y = {zero: null, one: b}; + const z = {zero: {}, one: {}, two: {zero: c}}; + x.zero = y.one; + z.zero.zero = x.zero; + return {zero: x, one: z}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 1, b: 20, c: 300}], + sequentialRenders: [ + {a: 2, b: 20, c: 300}, + {a: 3, b: 20, c: 300}, + {a: 3, b: 21, c: 300}, + {a: 3, b: 22, c: 300}, + {a: 3, b: 22, c: 301}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component(t0) { + const $ = _c(6); + const { a, b, c } = t0; + let t1; + if ($[0] !== a || $[1] !== b || $[2] !== c) { + const x = { zero: a }; + let t2; + if ($[4] !== b) { + t2 = { zero: null, one: b }; + $[4] = b; + $[5] = t2; + } else { + t2 = $[5]; + } + const y = t2; + const z = { zero: {}, one: {}, two: { zero: c } }; + x.zero = y.one; + z.zero.zero = x.zero; + t1 = { zero: x, one: z }; + $[0] = a; + $[1] = b; + $[2] = c; + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: 1, b: 20, c: 300 }], + sequentialRenders: [ + { a: 2, b: 20, c: 300 }, + { a: 3, b: 20, c: 300 }, + { a: 3, b: 21, c: 300 }, + { a: 3, b: 22, c: 300 }, + { a: 3, b: 22, c: 301 }, + ], +}; + +``` + +### Eval output +(kind: ok) {"zero":{"zero":20},"one":{"zero":{"zero":20},"one":{},"two":{"zero":300}}} +{"zero":{"zero":20},"one":{"zero":{"zero":20},"one":{},"two":{"zero":300}}} +{"zero":{"zero":21},"one":{"zero":{"zero":21},"one":{},"two":{"zero":300}}} +{"zero":{"zero":22},"one":{"zero":{"zero":22},"one":{},"two":{"zero":300}}} +{"zero":{"zero":22},"one":{"zero":{"zero":22},"one":{},"two":{"zero":301}}} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-access-assignment.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-access-assignment.js new file mode 100644 index 0000000000000..ef047238e7f84 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-access-assignment.js @@ -0,0 +1,23 @@ +function Component({a, b, c}) { + // This is an object version of array-access-assignment.js + // Meant to confirm that object expressions and PropertyStore/PropertyLoad with strings + // works equivalently to array expressions and property accesses with numeric indices + const x = {zero: a}; + const y = {zero: null, one: b}; + const z = {zero: {}, one: {}, two: {zero: c}}; + x.zero = y.one; + z.zero.zero = x.zero; + return {zero: x, one: z}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 1, b: 20, c: 300}], + sequentialRenders: [ + {a: 2, b: 20, c: 300}, + {a: 3, b: 20, c: 300}, + {a: 3, b: 21, c: 300}, + {a: 3, b: 22, c: 300}, + {a: 3, b: 22, c: 301}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.expect.md new file mode 100644 index 0000000000000..cc3e8107994a5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.expect.md @@ -0,0 +1,89 @@ + +## Input + +```javascript +// @validateNoFreezingKnownMutableFunctions + +import {useCallback, useEffect, useRef} from 'react'; +import {useHook} from 'shared-runtime'; + +function Component() { + const params = useHook(); + const update = useCallback( + partialParams => { + const nextParams = { + ...params, + ...partialParams, + }; + // Due to how we previously represented ObjectExpressions in InferReferenceEffects, + // this was recorded as a mutation of a context value (`params`) which then made + // the function appear ineligible for freezing when passing to useEffect below. + nextParams.param = 'value'; + console.log(nextParams); + }, + [params] + ); + const ref = useRef(null); + useEffect(() => { + if (ref.current === null) { + update(); + } + }, [update]); + + return 'ok'; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoFreezingKnownMutableFunctions + +import { useCallback, useEffect, useRef } from "react"; +import { useHook } from "shared-runtime"; + +function Component() { + const $ = _c(5); + const params = useHook(); + let t0; + if ($[0] !== params) { + t0 = (partialParams) => { + const nextParams = { ...params, ...partialParams }; + + nextParams.param = "value"; + console.log(nextParams); + }; + $[0] = params; + $[1] = t0; + } else { + t0 = $[1]; + } + const update = t0; + + const ref = useRef(null); + let t1; + let t2; + if ($[2] !== update) { + t1 = () => { + if (ref.current === null) { + update(); + } + }; + + t2 = [update]; + $[2] = update; + $[3] = t1; + $[4] = t2; + } else { + t1 = $[3]; + t2 = $[4]; + } + useEffect(t1, t2); + return "ok"; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.js new file mode 100644 index 0000000000000..5ba1eb1f648d1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-incorrectly-flagged-as-context-mutation.js @@ -0,0 +1,30 @@ +// @validateNoFreezingKnownMutableFunctions + +import {useCallback, useEffect, useRef} from 'react'; +import {useHook} from 'shared-runtime'; + +function Component() { + const params = useHook(); + const update = useCallback( + partialParams => { + const nextParams = { + ...params, + ...partialParams, + }; + // Due to how we previously represented ObjectExpressions in InferReferenceEffects, + // this was recorded as a mutation of a context value (`params`) which then made + // the function appear ineligible for freezing when passing to useEffect below. + nextParams.param = 'value'; + console.log(nextParams); + }, + [params] + ); + const ref = useRef(null); + useEffect(() => { + if (ref.current === null) { + update(); + } + }, [update]); + + return 'ok'; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reverse-postorder.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reverse-postorder.expect.md index c0ff0f7e7db78..98f2cd2190c20 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reverse-postorder.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reverse-postorder.expect.md @@ -50,10 +50,8 @@ function Component(props) { case 1: { break bb0; } - case 2: { - } - default: { - } + case 2: + default: } } else { if (props.cond2) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-switch.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-switch.expect.md index 4796fbdcc2bcf..48a765d3f3631 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-switch.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-switch.expect.md @@ -41,8 +41,7 @@ function foo() { case 2: { break bb0; } - default: { - } + default: } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-with-fallthrough.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-with-fallthrough.expect.md index c54631092c0ac..6fd911c432716 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-with-fallthrough.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/switch-with-fallthrough.expect.md @@ -43,22 +43,17 @@ export const FIXTURE_ENTRYPOINT = { ```javascript function foo(x) { bb0: switch (x) { - case 0: { - } - case 1: { - } + case 0: + case 1: case 2: { break bb0; } case 3: { break bb0; } - case 4: { - } - case 5: { - } - default: { - } + case 4: + case 5: + default: } } diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 62b8a7703fddb..217b4a968964b 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -453,8 +453,6 @@ const skipFilter = new Set([ 'inner-function/nullable-objects/bug-invalid-array-map-manual', 'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr', `bug-capturing-func-maybealias-captured-mutate`, - 'bug-aliased-capture-aliased-mutate', - 'bug-aliased-capture-mutate', 'bug-functiondecl-hoisting', 'bug-type-inference-control-flow', 'fbt/bug-fbt-plural-multiple-function-calls',