From aa2403e06883f023c5031666d8970bef2fe0b013 Mon Sep 17 00:00:00 2001 From: Sathya Gunasekaran Date: Thu, 2 Feb 2023 14:05:35 +0000 Subject: [PATCH] =?UTF-8?q?[=CE=BB]=20Run=20analyseFunctions=20on=20inner?= =?UTF-8?q?=20func=20exprs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously we would skip out on calling analyseFunctions on inner function exprs so we missed out on catching aliased mutation in a different scope. --- compiler/forget/src/HIR/HIR.ts | 23 +++--- .../forget/src/Inference/AnalyseFunctions.ts | 78 ++++++++++--------- .../_bug_capture-mutate-across-fns.expect.md | 40 ++++++++++ .../hir/_bug_capture-mutate-across-fns.js | 9 +++ .../capture-indirect-mutate-alias.expect.md | 43 ++++++++++ .../hir/capture-indirect-mutate-alias.js | 11 +++ 6 files changed, 156 insertions(+), 48 deletions(-) create mode 100644 compiler/forget/src/__tests__/fixtures/hir/_bug_capture-mutate-across-fns.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/_bug_capture-mutate-across-fns.js create mode 100644 compiler/forget/src/__tests__/fixtures/hir/capture-indirect-mutate-alias.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/capture-indirect-mutate-alias.js diff --git a/compiler/forget/src/HIR/HIR.ts b/compiler/forget/src/HIR/HIR.ts index 04997055e3..e978407bfc 100644 --- a/compiler/forget/src/HIR/HIR.ts +++ b/compiler/forget/src/HIR/HIR.ts @@ -440,17 +440,7 @@ export type InstructionData = | { kind: "ComputedStore"; object: Place; property: Place; value: Place } // load `object[index]` - like PropertyLoad but with a dynamic property | { kind: "ComputedLoad"; object: Place; property: Place } - | { - kind: "FunctionExpression"; - name: string | null; - params: Array; - dependencies: Array; - // TODO(gsn): Remove this mutatedDeps array and use dependencies as single - // source of truth. - mutatedDeps: Array; - loweredFunc: HIRFunction; - expr: t.ArrowFunctionExpression | t.FunctionExpression; - } + | FunctionExpression | { kind: "TaggedTemplateExpression"; tag: Place; @@ -471,6 +461,17 @@ export type JsxAttribute = | { kind: "JsxSpreadAttribute"; argument: Place } | { kind: "JsxAttribute"; name: string; place: Place }; +export type FunctionExpression = { + kind: "FunctionExpression"; + name: string | null; + params: Array; + dependencies: Array; + // TODO(gsn): Remove this mutatedDeps array and use dependencies as single + // source of truth. + mutatedDeps: Array; + loweredFunc: HIRFunction; + expr: t.ArrowFunctionExpression | t.FunctionExpression; +}; /** * A place where data may be read from / written to: * - a variable (identifier) diff --git a/compiler/forget/src/Inference/AnalyseFunctions.ts b/compiler/forget/src/Inference/AnalyseFunctions.ts index 6ca02a107f..86a89c4382 100644 --- a/compiler/forget/src/Inference/AnalyseFunctions.ts +++ b/compiler/forget/src/Inference/AnalyseFunctions.ts @@ -1,4 +1,10 @@ -import { HIRFunction, Identifier, mergeConsecutiveBlocks, Place } from "../HIR"; +import { + HIRFunction, + FunctionExpression, + Identifier, + mergeConsecutiveBlocks, + Place, +} from "../HIR"; import { constantPropagation } from "../Optimization"; import { eliminateRedundantPhi, enterSSA } from "../SSA"; import { inferTypes } from "../TypeInference"; @@ -30,18 +36,15 @@ function declareProperty( properties.set(lvalue.identifier, nextDependency); } -export default function (func: HIRFunction) { +export default function analyseFunctions(func: HIRFunction) { const properties: Map = new Map(); for (const [_, block] of func.body.blocks) { for (const instr of block.instructions) { switch (instr.value.kind) { case "FunctionExpression": { - instr.value.mutatedDeps = buildMutatedDeps( - analyzeMutatedPlaces(instr.value.loweredFunc), - instr.value.dependencies, - properties - ); + lower(instr.value.loweredFunc); + infer(instr.value, properties); break; } case "PropertyLoad": { @@ -57,6 +60,37 @@ export default function (func: HIRFunction) { } } +function lower(func: HIRFunction) { + mergeConsecutiveBlocks(func); + enterSSA(func); + eliminateRedundantPhi(func); + constantPropagation(func); + inferTypes(func); + analyseFunctions(func); + inferReferenceEffects(func); + inferMutableRanges(func); + logHIRFunction("AnalyseFunction (inner)", func); +} + +function infer( + value: FunctionExpression, + properties: Map +) { + const func = value.loweredFunc; + const mutations: Array = func.context.filter((dep) => + isMutated(dep.identifier) + ); + value.mutatedDeps = buildMutatedDeps( + mutations, + value.dependencies, + properties + ); +} + +function isMutated(id: Identifier) { + return id.mutableRange.end - id.mutableRange.start > 1; +} + function buildMutatedDeps( mutations: Place[], capturedDeps: Place[], @@ -89,33 +123,3 @@ function buildMutatedDeps( return mutatedDeps; } - -function analyzeMutatedPlaces(func: HIRFunction): Array { - mergeConsecutiveBlocks(func); - enterSSA(func); - eliminateRedundantPhi(func); - constantPropagation(func); - inferTypes(func); - inferReferenceEffects(func); - inferMutableRanges(func); - logHIRFunction("AnalyseFunction (inner)", func); - - const mutations: Array = []; - for (const [_, block] of func.body.blocks) { - for (const instr of block.instructions) { - if ( - instr.value.kind === "FunctionExpression" && - instr.value.loweredFunc !== null - ) { - mutations.push(...analyzeMutatedPlaces(instr.value.loweredFunc)); - } - } - } - - mutations.push(...func.context.filter((dep) => isMutated(dep.identifier))); - return mutations; -} - -function isMutated(id: Identifier) { - return id.mutableRange.end - id.mutableRange.start > 1; -} diff --git a/compiler/forget/src/__tests__/fixtures/hir/_bug_capture-mutate-across-fns.expect.md b/compiler/forget/src/__tests__/fixtures/hir/_bug_capture-mutate-across-fns.expect.md new file mode 100644 index 0000000000..087a50b506 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/_bug_capture-mutate-across-fns.expect.md @@ -0,0 +1,40 @@ + +## Input + +```javascript +function component(a) { + let z = { a }; + (function () { + (function () { + z.b = 1; + })(); + })(); + return z; +} + +``` + +## Code + +```javascript +function component(a) { + const $ = React.useMemoCache(); + const c_0 = $[0] !== a; + let z; + if (c_0) { + z = { a: a }; + $[0] = a; + $[1] = z; + } else { + z = $[1]; + } + (function () { + (function () { + z.b = 1; + })(); + })(); + return z; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/_bug_capture-mutate-across-fns.js b/compiler/forget/src/__tests__/fixtures/hir/_bug_capture-mutate-across-fns.js new file mode 100644 index 0000000000..295d7434e8 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/_bug_capture-mutate-across-fns.js @@ -0,0 +1,9 @@ +function component(a) { + let z = { a }; + (function () { + (function () { + z.b = 1; + })(); + })(); + return z; +} diff --git a/compiler/forget/src/__tests__/fixtures/hir/capture-indirect-mutate-alias.expect.md b/compiler/forget/src/__tests__/fixtures/hir/capture-indirect-mutate-alias.expect.md new file mode 100644 index 0000000000..8ec975e059 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/capture-indirect-mutate-alias.expect.md @@ -0,0 +1,43 @@ + +## Input + +```javascript +function component(a) { + let x = { a }; + (function () { + let q = x; + (function () { + q.b = 1; + })(); + })(); + + return x; +} + +``` + +## Code + +```javascript +function component(a) { + const $ = React.useMemoCache(); + const c_0 = $[0] !== a; + let x; + if (c_0) { + x = { a: a }; + (function () { + let q = x; + (function () { + q.b = 1; + })(); + })(); + $[0] = a; + $[1] = x; + } else { + x = $[1]; + } + return x; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/capture-indirect-mutate-alias.js b/compiler/forget/src/__tests__/fixtures/hir/capture-indirect-mutate-alias.js new file mode 100644 index 0000000000..9123c960ad --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/capture-indirect-mutate-alias.js @@ -0,0 +1,11 @@ +function component(a) { + let x = { a }; + (function () { + let q = x; + (function () { + q.b = 1; + })(); + })(); + + return x; +}