diff --git a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReactivePlaces.ts b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReactivePlaces.ts index ea950d1062..161647ddfb 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReactivePlaces.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReactivePlaces.ts @@ -97,10 +97,48 @@ export function inferReactivePlaces(fn: HIRFunction): void { includeThrowsAsExitNode: false, }); const postDominatorFrontierCache = new Map>(); + + function isReactiveControlledBlock(id: BlockId): boolean { + let controlBlocks = postDominatorFrontierCache.get(id); + if (controlBlocks === undefined) { + controlBlocks = postDominatorFrontier(fn, postDominators, id); + postDominatorFrontierCache.set(id, controlBlocks); + } + for (const blockId of controlBlocks) { + const controlBlock = fn.body.blocks.get(blockId)!; + switch (controlBlock.terminal.kind) { + case "if": + case "branch": { + if (reactiveIdentifiers.isReactive(controlBlock.terminal.test)) { + return true; + } + break; + } + case "switch": { + if (reactiveIdentifiers.isReactive(controlBlock.terminal.test)) { + return true; + } + for (const case_ of controlBlock.terminal.cases) { + if ( + case_.test !== null && + reactiveIdentifiers.isReactive(case_.test) + ) { + return true; + } + } + break; + } + } + } + return false; + } + const hasLoop = hasBackEdge(fn); do { const identifierMapping = new Map(); for (const [, block] of fn.body.blocks) { + let hasReactiveControl = isReactiveControlledBlock(block.id); + for (const phi of block.phis) { if (reactiveIdentifiers.isReactiveIdentifier(phi.id)) { // Already marked reactive on a previous pass @@ -116,48 +154,10 @@ export function inferReactivePlaces(fn: HIRFunction): void { if (isPhiReactive) { reactiveIdentifiers.markReactiveIdentifier(phi.id); } else { - // check to see if it has a reactive control dependency - for (const [pred, _operand] of phi.operands) { - let controlBlocks = postDominatorFrontierCache.get(pred); - if (controlBlocks === undefined) { - controlBlocks = postDominatorFrontier(fn, postDominators, pred); - postDominatorFrontierCache.set(pred, controlBlocks); - } - control: for (const blockId of controlBlocks) { - const controlBlock = fn.body.blocks.get(blockId)!; - switch (controlBlock.terminal.kind) { - case "if": - case "branch": { - if ( - reactiveIdentifiers.isReactive(controlBlock.terminal.test) - ) { - // control dependency is reactive - reactiveIdentifiers.markReactiveIdentifier(phi.id); - break control; - } - break; - } - case "switch": { - if ( - reactiveIdentifiers.isReactive(controlBlock.terminal.test) - ) { - // control dependency is reactive - reactiveIdentifiers.markReactiveIdentifier(phi.id); - break control; - } - for (const case_ of controlBlock.terminal.cases) { - if ( - case_.test !== null && - reactiveIdentifiers.isReactive(case_.test) - ) { - // control dependency is reactive - reactiveIdentifiers.markReactiveIdentifier(phi.id); - break control; - } - } - break; - } - } + for (const [pred] of phi.operands) { + if (isReactiveControlledBlock(pred)) { + reactiveIdentifiers.markReactiveIdentifier(phi.id); + break; } } } @@ -197,7 +197,8 @@ export function inferReactivePlaces(fn: HIRFunction): void { } reactiveIdentifiers.markReactive(lvalue); } - + } + if (hasReactiveInput || hasReactiveControl) { for (const operand of eachInstructionValueOperand(value)) { switch (operand.effect) { case Effect.Capture: diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-if.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-if.expect.md new file mode 100644 index 0000000000..93c9aeb826 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-if.expect.md @@ -0,0 +1,63 @@ + +## Input + +```javascript +function Component(props) { + // x is mutated conditionally based on a reactive value, + // so it needs to be considered reactive + let x = []; + if (props.cond) { + x.push(1); + } + // Since x is reactive, y is now reactively controlled too: + let y = false; + if (x[0]) { + y = true; + } + // Thus this value should be reactive on `y`: + return [y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true }], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function Component(props) { + const $ = useMemoCache(2); + + const x = []; + if (props.cond) { + x.push(1); + } + + let y = false; + if (x[0]) { + y = true; + } + let t0; + if ($[0] !== y) { + t0 = [y]; + $[0] = y; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true }], +}; + +``` + +### Eval output +(kind: ok) [true] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-if.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-if.js new file mode 100644 index 0000000000..4fc1a4d67d --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-if.js @@ -0,0 +1,20 @@ +function Component(props) { + // x is mutated conditionally based on a reactive value, + // so it needs to be considered reactive + let x = []; + if (props.cond) { + x.push(1); + } + // Since x is reactive, y is now reactively controlled too: + let y = false; + if (x[0]) { + y = true; + } + // Thus this value should be reactive on `y`: + return [y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true }], +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-switch.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-switch.expect.md new file mode 100644 index 0000000000..9f67305fbb --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-switch.expect.md @@ -0,0 +1,68 @@ + +## Input + +```javascript +function Component(props) { + // x is mutated conditionally based on a reactive value, + // so it needs to be considered reactive + let x = []; + if (props.cond) { + x.push(1); + } + // Since x is reactive, y is now reactively controlled too: + let y = false; + switch (x[0]) { + case 1: { + y = true; + break; + } + } + // Thus this value should be reactive on `y`: + return [y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true }], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function Component(props) { + const $ = useMemoCache(2); + + const x = []; + if (props.cond) { + x.push(1); + } + + let y = false; + switch (x[0]) { + case 1: { + y = true; + } + } + let t0; + if ($[0] !== y) { + t0 = [y]; + $[0] = y; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true }], +}; + +``` + +### Eval output +(kind: ok) [true] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-switch.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-switch.js new file mode 100644 index 0000000000..d0c932cc1c --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-switch.js @@ -0,0 +1,23 @@ +function Component(props) { + // x is mutated conditionally based on a reactive value, + // so it needs to be considered reactive + let x = []; + if (props.cond) { + x.push(1); + } + // Since x is reactive, y is now reactively controlled too: + let y = false; + switch (x[0]) { + case 1: { + y = true; + break; + } + } + // Thus this value should be reactive on `y`: + return [y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true }], +};