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 2cd2cfe633054..3bedd76ba8d15 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -949,10 +949,13 @@ function getFunctionName( export function printAliasingEffect(effect: AliasingEffect): string { switch (effect.kind) { case 'Alias': { - return `Alias ${printPlaceForAliasEffect(effect.from)} -> ${printPlaceForAliasEffect(effect.into)}`; + return `Alias ${printPlaceForAliasEffect(effect.into)} = ${printPlaceForAliasEffect(effect.from)}`; } case 'Capture': { - return `Capture ${printPlaceForAliasEffect(effect.from)} -> ${printPlaceForAliasEffect(effect.into)}`; + return `Capture ${printPlaceForAliasEffect(effect.into)} <- ${printPlaceForAliasEffect(effect.from)}`; + } + case 'ImmutableCapture': { + return `ImmutableCapture ${printPlaceForAliasEffect(effect.into)} <- ${printPlaceForAliasEffect(effect.from)}`; } case 'Create': { return `Create ${printPlaceForAliasEffect(effect.into)} = ${effect.value}`; 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 23cbf27a8d833..12b99b8b157b8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts @@ -14,6 +14,7 @@ import { Place, isRefOrRefValue, makeInstructionId, + printFunction, } from '../HIR'; import {deadCodeElimination} from '../Optimization'; import {inferReactiveScopeVariables} from '../ReactiveScopes'; @@ -26,10 +27,7 @@ import { eachInstructionValueOperand, } from '../HIR/visitors'; import {Iterable_some} from '../Utils/utils'; -import { - AliasingEffect, - inferMutationAliasingEffects, -} from './InferMutationAliasingEffects'; +import {inferMutationAliasingEffects} from './InferMutationAliasingEffects'; import {inferMutationAliasingFunctionEffects} from './InferMutationAliasingFunctionEffects'; import {inferMutationAliasingRanges} from './InferMutationAliasingRanges'; @@ -44,7 +42,6 @@ export default function analyseFunctions(func: HIRFunction): void { infer(instr.value.loweredFunc, aliases); } else { lowerWithMutationAliasing(instr.value.loweredFunc.func); - infer(instr.value.loweredFunc, new DisjointSet()); } /** diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index a222dd59b121b..ee19dbc03b3bf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -38,7 +38,9 @@ import { Set_isSuperset, } from '../Utils/utils'; import { + printAliasingEffect, printIdentifier, + printInstruction, printInstructionValue, printPlace, printSourceLocation, @@ -258,6 +260,23 @@ function applySignature( state.define(effect.into, value); break; } + case 'ImmutableCapture': { + let value = effectInstructionValueCache.get(effect); + if (value == null) { + value = { + kind: 'ObjectExpression', + properties: [], + loc: effect.into.loc, + }; + effectInstructionValueCache.set(effect, value); + } + state.initialize(value, { + kind: ValueKind.Frozen, + reason: new Set([ValueReason.Other]), + }); + state.define(effect.into, value); + break; + } case 'CreateFrom': { const kind = state.kind(effect.from).kind; let value = effectInstructionValueCache.get(effect); @@ -274,6 +293,29 @@ function applySignature( reason: new Set([ValueReason.Other]), }); state.define(effect.into, value); + switch (kind) { + case ValueKind.Primitive: + case ValueKind.Global: { + // no need to track this data flow + break; + } + case ValueKind.Frozen: { + effects.push({ + kind: 'ImmutableCapture', + from: effect.from, + into: effect.into, + }); + break; + } + default: { + // TODO: should this be Alias?? + effects.push({ + kind: 'Capture', + from: effect.from, + into: effect.into, + }); + } + } break; } case 'Capture': { @@ -297,27 +339,28 @@ function applySignature( } } const fromKind = state.kind(effect.from).kind; - let isCopyByReferenceValue: boolean; + let isMutableReferenceType: boolean; switch (fromKind) { case ValueKind.Global: case ValueKind.Primitive: { - isCopyByReferenceValue = false; + isMutableReferenceType = false; break; } case ValueKind.Frozen: { - /* - * TODO: add a separate "ImmutableAlias" effect to downgrade to, that doesn't impact mutable ranges - * We want to remember that the data flow occurred for PruneNonEscapingScopes - */ - isCopyByReferenceValue = false; + isMutableReferenceType = false; + effects.push({ + kind: 'ImmutableCapture', + from: effect.from, + into: effect.into, + }); break; } default: { - isCopyByReferenceValue = true; + isMutableReferenceType = true; break; } } - if (isMutableDesination && isCopyByReferenceValue) { + if (isMutableDesination && isMutableReferenceType) { effects.push(effect); } break; @@ -329,12 +372,25 @@ function applySignature( */ const fromKind = state.kind(effect.from).kind; switch (fromKind) { - /* - * TODO: add a separate "ImmutableAlias" effect to downgrade to, that doesn't impact mutable ranges - * We want to remember that the data flow occurred for PruneNonEscapingScopes - * (use this to replace the ValueKind.Frozen case) - */ - case ValueKind.Frozen: + case ValueKind.Frozen: { + effects.push({ + kind: 'ImmutableCapture', + from: effect.from, + into: effect.into, + }); + let value = effectInstructionValueCache.get(effect); + if (value == null) { + value = { + kind: 'Primitive', + value: undefined, + loc: effect.from.loc, + }; + effectInstructionValueCache.set(effect, value); + } + state.initialize(value, {kind: fromKind, reason: new Set([])}); + state.define(effect.into, value); + break; + } case ValueKind.Global: case ValueKind.Primitive: { let value = effectInstructionValueCache.get(effect); @@ -385,24 +441,7 @@ function applySignature( case 'MutateTransitiveConditionally': { const didMutate = state.mutate(effect.kind, effect.value); if (didMutate) { - switch (effect.kind) { - case 'Mutate': { - effects.push(effect); - break; - } - case 'MutateConditionally': { - effects.push({kind: 'Mutate', value: effect.value}); - break; - } - case 'MutateTransitive': { - effects.push(effect); - break; - } - case 'MutateTransitiveConditionally': { - effects.push({kind: 'MutateTransitive', value: effect.value}); - break; - } - } + effects.push(effect); } break; } @@ -414,13 +453,19 @@ function applySignature( } } } - CompilerError.invariant( - state.isDefined(instruction.lvalue) && state.kind(instruction.lvalue), - { + if ( + !(state.isDefined(instruction.lvalue) && state.kind(instruction.lvalue)) + ) { + console.log(printInstruction(instruction)); + console.log('input effects'); + console.log(signature.effects.map(printAliasingEffect).join('\n')); + console.log('refined effects'); + console.log(effects.map(printAliasingEffect).join('\n')); + CompilerError.invariant(false, { reason: `Expected instruction lvalue to be initialized`, loc: instruction.loc, - }, - ); + }); + } return effects.length !== 0 ? effects : null; } @@ -961,11 +1006,6 @@ function computeSignatureForInstruction( from: value.object, into: lvalue, }); - effects.push({ - kind: 'Capture', - from: value.object, - into: lvalue, - }); break; } case 'PropertyStore': @@ -1106,11 +1146,6 @@ function computeSignatureForInstruction( from: value.value, into: patternLValue, }); - effects.push({ - kind: 'Capture', - from: value.value, - into: patternLValue, - }); } effects.push({kind: 'Alias', from: value.value, into: lvalue}); break; @@ -1267,6 +1302,7 @@ function computeEffectsForSignature( } break; } + case 'ImmutableCapture': case 'CreateFrom': case 'Apply': case 'Mutate': @@ -1413,6 +1449,10 @@ export type AliasingEffect = * Creates a new value with the same kind as the starting value. */ | {kind: 'CreateFrom'; from: Place; into: Place} + /** + * Immutable data flow, used for escape analysis. Does not influence mutable range analysis: + */ + | {kind: 'ImmutableCapture'; from: Place; into: Place} /** * Calls the function at the given place with the given arguments either captured or aliased, * and captures/aliases the result into the given place. diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts index e9f2bce050ae6..e7224be8066b1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts @@ -54,7 +54,10 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void { effect.kind === 'Capture' ) { captures.union([effect.from.identifier, effect.into.identifier]); - } else if (effect.kind === 'MutateTransitive') { + } else if ( + effect.kind === 'MutateTransitive' || + effect.kind === 'MutateTransitiveConditionally' + ) { const value = effect.value; value.identifier.mutableRange.end = makeInstructionId(instr.id + 1); } @@ -83,7 +86,10 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void { for (const effect of instr.effects) { if (effect.kind === 'Alias' || effect.kind === 'CreateFrom') { aliases.union([effect.from.identifier, effect.into.identifier]); - } else if (effect.kind === 'Mutate') { + } else if ( + effect.kind === 'Mutate' || + effect.kind === 'MutateConditionally' + ) { const value = effect.value; value.identifier.mutableRange.end = makeInstructionId(instr.id + 1); } @@ -94,7 +100,10 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void { /** * Part 3 - * Add legacy operand-specific effects based on instruction effects and mutable ranges + * Add legacy operand-specific effects based on instruction effects and mutable ranges. + * Also fixes up operand mutable ranges, making sure that start is non-zero if the value + * is mutated (depended on by later passes like InferReactiveScopeVariables which uses this + * to filter spurious mutations of globals, which we now guard against more precisely) */ for (const block of fn.body.blocks.values()) { for (const phi of block.phis) { @@ -136,6 +145,10 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void { } break; } + case 'ImmutableCapture': { + operandEffects.set(effect.from.identifier.id, Effect.Read); + break; + } case 'Create': { break; } @@ -178,6 +191,12 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void { lvalue.effect = effect; } for (const operand of eachInstructionValueOperand(instr.value)) { + if ( + operand.identifier.mutableRange.end > instr.id && + operand.identifier.mutableRange.start === 0 + ) { + operand.identifier.mutableRange.start = instr.id; + } const effect = operandEffects.get(operand.identifier.id) ?? Effect.Read; operand.effect = effect; } 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 82fd4bdece6e0..0c1fd759bd5b8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -376,12 +376,12 @@ export function findDisjointMutableValues( } else { for (const operand of eachInstructionOperand(instr)) { if ( - isMutable(instr, operand) + isMutable(instr, operand) && /* * exclude global variables from being added to scopes, we can't recreate them! * TODO: improve handling of module-scoped variables and globals */ - // && operand.identifier.mutableRange.start > 0 + operand.identifier.mutableRange.start > 0 ) { if ( instr.value.kind === 'FunctionExpression' || diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-named-function-with-shadowed-local-same-name.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-named-function-with-shadowed-local-same-name.expect.md index f66b970f00dd4..2a935256d7a0d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-named-function-with-shadowed-local-same-name.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-named-function-with-shadowed-local-same-name.expect.md @@ -22,7 +22,7 @@ function Component(props) { 7 | return hasErrors; 8 | } > 9 | return hasErrors(); - | ^^^^^^^^^ Invariant: [hoisting] Expected value for identifier to be initialized. hasErrors_0$14 (9:9) + | ^^^^^^^^^ Invariant: [hoisting] Expected value for identifier to be initialized. hasErrors_0$15 (9:9) 10 | } 11 | ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation-via-function-expression.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation-via-function-expression.expect.md new file mode 100644 index 0000000000000..85ebf65a1fed4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation-via-function-expression.expect.md @@ -0,0 +1,58 @@ + +## Input + +```javascript +// @enableNewMutationAliasingModel +function Component({a, b}) { + const x = {a}; + const y = [b]; + const f = () => { + y.x = x; + mutate(y); + }; + return
{x}
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel +function Component(t0) { + const $ = _c(7); + const { a, b } = t0; + let t1; + let x; + if ($[0] !== a || $[1] !== b) { + x = { a }; + const y = [b]; + t1 = () => { + y.x = x; + mutate(y); + }; + $[0] = a; + $[1] = b; + $[2] = t1; + $[3] = x; + } else { + t1 = $[2]; + x = $[3]; + } + const f = t1; + let t2; + if ($[4] !== f || $[5] !== x) { + t2 =
{x}
; + $[4] = f; + $[5] = x; + $[6] = t2; + } else { + t2 = $[6]; + } + return t2; +} + +``` + +### 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/new-mutability/basic-mutation-via-function-expression.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation-via-function-expression.js new file mode 100644 index 0000000000000..8d4bb23742b2b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation-via-function-expression.js @@ -0,0 +1,11 @@ +// @enableNewMutationAliasingModel +function Component({a, b}) { + const x = {a}; + const y = [b]; + const f = () => { + y.x = x; + mutate(y); + }; + f(); + return
{x}
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation.expect.md new file mode 100644 index 0000000000000..0753f007b7b32 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation.expect.md @@ -0,0 +1,42 @@ + +## Input + +```javascript +// @enableNewMutationAliasingModel +function Component({a, b}) { + const x = {a}; + const y = [b]; + y.x = x; + mutate(y); + return
{x}
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel +function Component(t0) { + const $ = _c(3); + const { a, b } = t0; + let t1; + if ($[0] !== a || $[1] !== b) { + const x = { a }; + const y = [b]; + y.x = x; + mutate(y); + t1 =
{x}
; + $[0] = a; + $[1] = b; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +``` + +### 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/new-mutability/basic-mutation.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation.js new file mode 100644 index 0000000000000..480221fef4dab --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation.js @@ -0,0 +1,8 @@ +// @enableNewMutationAliasingModel +function Component({a, b}) { + const x = {a}; + const y = [b]; + y.x = x; + mutate(y); + return
{x}
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/potential-mutation-in-function-expression.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/potential-mutation-in-function-expression.expect.md new file mode 100644 index 0000000000000..9e6fa024e3da3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/potential-mutation-in-function-expression.expect.md @@ -0,0 +1,59 @@ + +## Input + +```javascript +// @enableNewMutationAliasingModel +function Component({a, b, c}) { + const x = [a, b]; + const f = () => { + maybeMutate(x); + // different dependency to force this not to merge with x's scope + console.log(c); + }; + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel +function Component(t0) { + const $ = _c(8); + const { a, b, c } = t0; + let t1; + let x; + if ($[0] !== a || $[1] !== b || $[2] !== c) { + x = [a, b]; + t1 = () => { + maybeMutate(x); + + console.log(c); + }; + $[0] = a; + $[1] = b; + $[2] = c; + $[3] = t1; + $[4] = x; + } else { + t1 = $[3]; + x = $[4]; + } + const f = t1; + let t2; + if ($[5] !== f || $[6] !== x) { + t2 = ; + $[5] = f; + $[6] = x; + $[7] = t2; + } else { + t2 = $[7]; + } + return t2; +} + +``` + +### 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/new-mutability/potential-mutation-in-function-expression.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/potential-mutation-in-function-expression.js new file mode 100644 index 0000000000000..096f4f17ea545 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/potential-mutation-in-function-expression.js @@ -0,0 +1,10 @@ +// @enableNewMutationAliasingModel +function Component({a, b, c}) { + const x = [a, b]; + const f = () => { + maybeMutate(x); + // different dependency to force this not to merge with x's scope + console.log(c); + }; + return ; +}