From 501fbd8ed8f036c6e0ec29290a391c5a3b375b5f Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Wed, 22 Mar 2023 15:58:06 -0400 Subject: [PATCH] [hir] infer reference effects for property call --- This PR does not add inference for normal `CallExpression`s, since built-in functions for `Array` and `Object` are usually only valid if called with a correctly-typed `this`. If we want codegen to preserve source code semantics, Forget should only add inferred types it is confident about. This PR also adds `returnEffect` to FunctionSignature. `returnEffect = Store` if this function is known to always return a captured value from `receiver` or `args`. --- compiler/forget/src/CompilerError.ts | 13 ++ compiler/forget/src/HIR/HIR.ts | 2 +- compiler/forget/src/HIR/PrintHIR.ts | 2 +- .../src/Inference/InferReferenceEffects.ts | 131 +++++++++++++++--- .../compiler/array-at-closure.expect.md | 52 +++++++ .../fixtures/compiler/array-at-closure.js | 9 ++ .../compiler/array-at-effect.expect.md | 70 ++++++++++ .../fixtures/compiler/array-at-effect.js | 9 ++ .../array-at-mutate-after-capture.expect.md | 47 +++++++ .../compiler/array-at-mutate-after-capture.js | 10 ++ .../compiler/array-property-call.expect.md | 56 ++++---- .../compiler/array-push-effect.expect.md | 71 ++++++++++ .../fixtures/compiler/array-push-effect.js | 11 ++ ...-variations-complex-lvalue-array.expect.md | 22 ++- ...rror.mutate-after-aliased-freeze.expect.md | 31 +++++ .../error.mutate-after-aliased-freeze.js | 16 +++ .../error.mutate-after-freeze.expect.md | 25 ++++ .../compiler/error.mutate-after-freeze.js | 10 ++ .../overlapping-scopes-within-block.expect.md | 18 ++- .../reactive-scope-grouping.expect.md | 11 +- .../reassignment-conditional.expect.md | 58 ++++---- .../compiler/reassignment-conditional.js | 1 - .../fixtures/hir/array-at-closure.expect.md | 47 +++++++ .../fixtures/hir/array-at-closure.js | 9 ++ .../fixtures/hir/array-at-effect.expect.md | 38 +++++ .../__tests__/fixtures/hir/array-at-effect.js | 9 ++ .../hir/array-property-call.expect.md | 18 +-- .../fixtures/hir/array-push-effect.expect.md | 46 ++++++ .../fixtures/hir/array-push-effect.js | 11 ++ 29 files changed, 760 insertions(+), 93 deletions(-) create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/array-at-closure.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/array-at-closure.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/array-at-effect.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/array-at-effect.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/array-at-mutate-after-capture.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/array-at-mutate-after-capture.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/array-push-effect.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/array-push-effect.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-aliased-freeze.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-aliased-freeze.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-freeze.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-freeze.js create mode 100644 compiler/forget/src/__tests__/fixtures/hir/array-at-closure.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/array-at-closure.js create mode 100644 compiler/forget/src/__tests__/fixtures/hir/array-at-effect.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/array-at-effect.js create mode 100644 compiler/forget/src/__tests__/fixtures/hir/array-push-effect.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/array-push-effect.js diff --git a/compiler/forget/src/CompilerError.ts b/compiler/forget/src/CompilerError.ts index c725ae20bd..ef1675f3e2 100644 --- a/compiler/forget/src/CompilerError.ts +++ b/compiler/forget/src/CompilerError.ts @@ -128,6 +128,19 @@ export class CompilerError extends Error { throw errors; } + static invalidInput(reason: string, loc: SourceLocation): never { + const errors = new CompilerError(); + errors.pushErrorDetail( + new CompilerErrorDetail({ + codeframe: null, + loc: typeof loc === "symbol" ? null : loc, + reason, + severity: ErrorSeverity.InvalidInput, + }) + ); + throw errors; + } + constructor(...args: any[]) { super(...args); } diff --git a/compiler/forget/src/HIR/HIR.ts b/compiler/forget/src/HIR/HIR.ts index 9a1e1e374d..34c5c6a8d7 100644 --- a/compiler/forget/src/HIR/HIR.ts +++ b/compiler/forget/src/HIR/HIR.ts @@ -449,7 +449,7 @@ export type Phi = { * Type inference does not currently guarantee that {@link PropertyCall.property} * is a FunctionType. */ -type PropertyCall = { +export type PropertyCall = { kind: "PropertyCall"; receiver: Place; property: Place; diff --git a/compiler/forget/src/HIR/PrintHIR.ts b/compiler/forget/src/HIR/PrintHIR.ts index 9139f7e885..e582c8be54 100644 --- a/compiler/forget/src/HIR/PrintHIR.ts +++ b/compiler/forget/src/HIR/PrintHIR.ts @@ -564,7 +564,7 @@ function printScope(scope: ReactiveScope | null): string { return `${scope !== null ? `_@${scope.id}` : ""}`; } -function printType(type: Type): string { +export function printType(type: Type): string { if (type.kind === "Type") return ""; // TODO(mofeiZ): add debugName for generated ids if (type.kind === "Object" && type.shapeId != null) { diff --git a/compiler/forget/src/Inference/InferReferenceEffects.ts b/compiler/forget/src/Inference/InferReferenceEffects.ts index 9c0d356f47..9dbd0253f7 100644 --- a/compiler/forget/src/Inference/InferReferenceEffects.ts +++ b/compiler/forget/src/Inference/InferReferenceEffects.ts @@ -18,12 +18,17 @@ import { isObjectType, Phi, Place, + PropertyCall, + Type, ValueKind, } from "../HIR/HIR"; +import { FunctionSignature } from "../HIR/ObjectShape"; import { + printIdentifier, printMixedHIR, printPlace, printSourceLocation, + printType, } from "../HIR/PrintHIR"; import { eachInstructionOperand, @@ -263,6 +268,18 @@ class InferenceState { * value is already frozen or is immutable. */ reference(place: Place, effectKind: Effect): void { + this.#referenceImpl(place, effectKind, false); + } + + /** + * Throwing version of {@link reference}, which throws with an error + * if we record a mutate effect on an immutable value. + */ + referenceAndCheckError(place: Place, effectKind: Effect): void { + this.#referenceImpl(place, effectKind, true); + } + + #referenceImpl(place: Place, effectKind: Effect, shouldError: boolean): void { const values = this.#variables.get(place.identifier.id); if (values === undefined) { place.effect = effectKind === Effect.Mutate ? Effect.Mutate : Effect.Read; @@ -292,6 +309,14 @@ class InferenceState { ) { effect = Effect.Mutate; } else { + if (shouldError) { + CompilerError.invalidInput( + `InferReferenceEffects: inferred mutation of known immutable value ${printIdentifier( + place.identifier + )}${printType(place.identifier.type)} (${valueKind})`, + place.loc + ); + } effect = Effect.Read; } break; @@ -554,7 +579,7 @@ function mergeValues(a: ValueKind, b: ValueKind): ValueKind { * recording references on the @param state according to JS semantics. */ function inferBlock( - _env: Environment, + env: Environment, state: InferenceState, block: BasicBlock ): void { @@ -660,25 +685,39 @@ function inferBlock( continue; } case "PropertyCall": { - if (!state.isDefined(instrValue.receiver)) { - // TODO @josephsavona: improve handling of globals - const value: InstructionValue = { - kind: "Primitive", - loc: instrValue.loc, - value: undefined, - }; - state.initialize(value, ValueKind.Frozen); - state.define(instrValue.receiver, value); - } + invariant( + state.isDefined(instrValue.receiver), + "[InferReferenceEffects] Internal error: receiver of PropertyCall should have been defined by corresponding PropertyLoad" + ); - state.reference(instrValue.receiver, Effect.Mutate); state.reference(instrValue.property, Effect.Read); - for (const arg of instrValue.args) { - if (arg.kind === "Identifier") { - state.reference(arg, Effect.Mutate); - } else { - state.reference(arg.place, Effect.Mutate); + + const signature = getFunctionCallSignature( + env, + instrValue.property.identifier.type + ); + if (signature !== null) { + const effects = getFunctionCallEffects( + instrValue, + signature, + Effect.Mutate + ); + for (const [place, effect] of effects) { + state.referenceAndCheckError(place, effect); } + state.referenceAndCheckError( + instrValue.receiver, + signature.calleeEffect + ); + } else { + for (const arg of instrValue.args) { + if (arg.kind === "Identifier") { + state.reference(arg, Effect.Mutate); + } else { + state.reference(arg.place, Effect.Mutate); + } + } + state.reference(instrValue.receiver, Effect.Mutate); } state.initialize(instrValue, ValueKind.Mutable); state.define(instr.lvalue, instrValue); @@ -904,3 +943,61 @@ function hasContextRefOperand( } return false; } + +function getFunctionCallSignature( + env: Environment, + type: Type +): FunctionSignature | null { + if (type.kind !== "Function") { + return null; + } + return env.getFunctionSignature(type); +} + +/** + * Make a best attempt at matching arguments of a PropertyCall to its FunctionSignature, + * calling back to `defaultEffect` when we are unable to. + * + * @param fn + * @param sig + * @param defaultEffect In the case that inference fails, all arguments will be inferred + * as defaultEffect + * @returns Inferred effects of function arguments + */ +function getFunctionCallEffects( + fn: PropertyCall, + sig: FunctionSignature, + defaultEffect: Effect +): Array<[Place, Effect]> { + const inferredEffects: Array<[Place, Effect | null]> = fn.args.map( + (arg, idx) => { + const argPlace = arg.kind === "Identifier" ? arg : arg.place; + if (idx < sig.positionalParams.length) { + // Only infer effects when there is a direct mapping positional arg --> positional param + // Otherwise, return null to indicate inference failed + if (arg.kind === "Identifier") { + return [argPlace, sig.positionalParams[idx]]; + } else { + return [argPlace, null]; + } + } else if (sig.restParam !== null) { + return [argPlace, sig.restParam]; + } else { + // If there are more arguments than positional arguments, we'll also assume + // that inference failed + return [argPlace, null]; + } + } + ); + + let results: Array<[Place, Effect]>; + if (inferredEffects.some(([_, effect]) => effect === null)) { + // If inference failed for any argument, give up on inference for all arguments + results = inferredEffects.map(([arg, _]) => { + return [arg, defaultEffect]; + }); + } else { + results = inferredEffects as Array<[Place, Effect]>; + } + return results; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/array-at-closure.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/array-at-closure.expect.md new file mode 100644 index 0000000000..8e418fad9b --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/array-at-closure.expect.md @@ -0,0 +1,52 @@ + +## Input + +```javascript +function Component(props) { + const x = foo(props.x); + const fn = function () { + const arr = [...bar(props)]; + return arr.at(x); + }; + const fnResult = fn(); + return fnResult; +} + +``` + +## Code + +```javascript +function Component(props) { + const $ = React.unstable_useMemoCache(5); + const c_0 = $[0] !== props.x; + let t0; + if (c_0) { + t0 = foo(props.x); + $[0] = props.x; + $[1] = t0; + } else { + t0 = $[1]; + } + const x = t0; + const c_2 = $[2] !== props; + const c_3 = $[3] !== x; + let t1; + if (c_2 || c_3) { + const fn = function () { + const arr = [...bar(props)]; + return arr.at(x); + }; + t1 = fn(); + $[2] = props; + $[3] = x; + $[4] = t1; + } else { + t1 = $[4]; + } + const fnResult = t1; + return fnResult; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/array-at-closure.js b/compiler/forget/src/__tests__/fixtures/compiler/array-at-closure.js new file mode 100644 index 0000000000..244b834756 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/array-at-closure.js @@ -0,0 +1,9 @@ +function Component(props) { + const x = foo(props.x); + const fn = function () { + const arr = [...bar(props)]; + return arr.at(x); + }; + const fnResult = fn(); + return fnResult; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/array-at-effect.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/array-at-effect.expect.md new file mode 100644 index 0000000000..e8ee5dad01 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/array-at-effect.expect.md @@ -0,0 +1,70 @@ + +## Input + +```javascript +// arrayInstance.at should have the following effects: +// - read on arg0 +// - read on receiver +// - mutate on lvalue +function ArrayAtTest(props) { + const arr = [foo(props.x)]; + const result = arr.at(bar(props.y)); + return result; +} + +``` + +## Code + +```javascript +// arrayInstance.at should have the following effects: +// - read on arg0 +// - read on receiver +// - mutate on lvalue +function ArrayAtTest(props) { + const $ = React.unstable_useMemoCache(9); + const c_0 = $[0] !== props.x; + let t0; + if (c_0) { + t0 = foo(props.x); + $[0] = props.x; + $[1] = t0; + } else { + t0 = $[1]; + } + const c_2 = $[2] !== t0; + let t1; + if (c_2) { + t1 = [t0]; + $[2] = t0; + $[3] = t1; + } else { + t1 = $[3]; + } + const arr = t1; + const c_4 = $[4] !== props.y; + let t2; + if (c_4) { + t2 = bar(props.y); + $[4] = props.y; + $[5] = t2; + } else { + t2 = $[5]; + } + const c_6 = $[6] !== arr; + const c_7 = $[7] !== t2; + let t3; + if (c_6 || c_7) { + t3 = arr.at(t2); + $[6] = arr; + $[7] = t2; + $[8] = t3; + } else { + t3 = $[8]; + } + const result = t3; + return result; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/array-at-effect.js b/compiler/forget/src/__tests__/fixtures/compiler/array-at-effect.js new file mode 100644 index 0000000000..2e2e78e50e --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/array-at-effect.js @@ -0,0 +1,9 @@ +// arrayInstance.at should have the following effects: +// - read on arg0 +// - read on receiver +// - mutate on lvalue +function ArrayAtTest(props) { + const arr = [foo(props.x)]; + const result = arr.at(bar(props.y)); + return result; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/array-at-mutate-after-capture.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/array-at-mutate-after-capture.expect.md new file mode 100644 index 0000000000..6500c946c3 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/array-at-mutate-after-capture.expect.md @@ -0,0 +1,47 @@ + +## Input + +```javascript +// x's mutable range should extend to `mutate(y)` + +function Component(props) { + let x = [42, {}]; + const idx = foo(props.b); + let y = x.at(idx); + mutate(y); + + return x; +} + +``` + +## Code + +```javascript +// x's mutable range should extend to `mutate(y)` + +function Component(props) { + const $ = React.unstable_useMemoCache(2); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = {}; + $[0] = t0; + } else { + t0 = $[0]; + } + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = [42, t0]; + $[1] = t1; + } else { + t1 = $[1]; + } + const x = t1; + const idx = foo(props.b); + const y = x.at(idx); + mutate(y); + return x; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/array-at-mutate-after-capture.js b/compiler/forget/src/__tests__/fixtures/compiler/array-at-mutate-after-capture.js new file mode 100644 index 0000000000..9553132e1a --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/array-at-mutate-after-capture.js @@ -0,0 +1,10 @@ +// x's mutable range should extend to `mutate(y)` + +function Component(props) { + let x = [42, {}]; + const idx = foo(props.b); + let y = x.at(idx); + mutate(y); + + return x; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/array-property-call.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/array-property-call.expect.md index 8f897264d4..fa0d1c60f2 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/array-property-call.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/array-property-call.expect.md @@ -16,43 +16,49 @@ function Component(props) { ```javascript function Component(props) { - const $ = React.unstable_useMemoCache(10); + const $ = React.unstable_useMemoCache(11); const c_0 = $[0] !== props.a; const c_1 = $[1] !== props.b; - const c_2 = $[2] !== props.c; let t0; let a; - let x; - if (c_0 || c_1 || c_2) { + if (c_0 || c_1) { a = [props.a, props.b, "hello"]; - x = a.push(42); - t0 = a.at(props.c); + t0 = a.push(42); $[0] = props.a; $[1] = props.b; - $[2] = props.c; - $[3] = t0; - $[4] = a; - $[5] = x; + $[2] = t0; + $[3] = a; } else { - t0 = $[3]; - a = $[4]; - x = $[5]; + t0 = $[2]; + a = $[3]; } - const y = t0; - const c_6 = $[6] !== a; - const c_7 = $[7] !== x; - const c_8 = $[8] !== y; + const x = t0; + const c_4 = $[4] !== a; + const c_5 = $[5] !== props.c; let t1; - if (c_6 || c_7 || c_8) { - t1 = { a, x, y }; - $[6] = a; - $[7] = x; - $[8] = y; - $[9] = t1; + if (c_4 || c_5) { + t1 = a.at(props.c); + $[4] = a; + $[5] = props.c; + $[6] = t1; } else { - t1 = $[9]; + t1 = $[6]; } - return t1; + const y = t1; + const c_7 = $[7] !== a; + const c_8 = $[8] !== x; + const c_9 = $[9] !== y; + let t2; + if (c_7 || c_8 || c_9) { + t2 = { a, x, y }; + $[7] = a; + $[8] = x; + $[9] = y; + $[10] = t2; + } else { + t2 = $[10]; + } + return t2; } ``` diff --git a/compiler/forget/src/__tests__/fixtures/compiler/array-push-effect.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/array-push-effect.expect.md new file mode 100644 index 0000000000..15c6128ebb --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/array-push-effect.expect.md @@ -0,0 +1,71 @@ + +## Input + +```javascript +// arrayInstance.push should have the following effects: +// - read on all args (rest parameter) +// - mutate on receiver +function Component(props) { + const x = foo(props.x); + const y = { y: props.y }; + const arr = []; + arr.push({}); + arr.push(x, y); + return arr; +} + +``` + +## Code + +```javascript +// arrayInstance.push should have the following effects: +// - read on all args (rest parameter) +// - mutate on receiver +function Component(props) { + const $ = React.unstable_useMemoCache(8); + const c_0 = $[0] !== props.x; + let t0; + if (c_0) { + t0 = foo(props.x); + $[0] = props.x; + $[1] = t0; + } else { + t0 = $[1]; + } + const x = t0; + const c_2 = $[2] !== props.y; + let t1; + if (c_2) { + t1 = { y: props.y }; + $[2] = props.y; + $[3] = t1; + } else { + t1 = $[3]; + } + const y = t1; + const c_4 = $[4] !== x; + const c_5 = $[5] !== y; + let arr; + if (c_4 || c_5) { + arr = []; + let t2; + if ($[7] === Symbol.for("react.memo_cache_sentinel")) { + t2 = {}; + $[7] = t2; + } else { + t2 = $[7]; + } + arr.push(t2); + arr.push(x, y); + $[4] = x; + $[5] = y; + $[6] = arr; + } else { + arr = $[6]; + } + return arr; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/array-push-effect.js b/compiler/forget/src/__tests__/fixtures/compiler/array-push-effect.js new file mode 100644 index 0000000000..b91eff8642 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/array-push-effect.js @@ -0,0 +1,11 @@ +// arrayInstance.push should have the following effects: +// - read on all args (rest parameter) +// - mutate on receiver +function Component(props) { + const x = foo(props.x); + const y = { y: props.y }; + const arr = []; + arr.push({}); + arr.push(x, y); + return arr; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/assignment-variations-complex-lvalue-array.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/assignment-variations-complex-lvalue-array.expect.md index 8e4ab75469..fd20e1d99c 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/assignment-variations-complex-lvalue-array.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/assignment-variations-complex-lvalue-array.expect.md @@ -15,16 +15,24 @@ function foo() { ```javascript function foo() { - const $ = React.unstable_useMemoCache(1); - let a; + const $ = React.unstable_useMemoCache(2); + let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - a = [[1]]; - const first = a.at(0); - first.set(0, 2); - $[0] = a; + t0 = [1]; + $[0] = t0; } else { - a = $[0]; + t0 = $[0]; } + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = [t0]; + $[1] = t1; + } else { + t1 = $[1]; + } + const a = t1; + const first = a.at(0); + first.set(0, 2); return a; } diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-aliased-freeze.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-aliased-freeze.expect.md new file mode 100644 index 0000000000..65f5d9a57d --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-aliased-freeze.expect.md @@ -0,0 +1,31 @@ + +## Input + +```javascript +function Component(props) { + let x = []; + let y = x; + + if (props.p1) { + x = []; + } + + let _ = ; + + // y is MaybeFrozen at this point, since it may alias to x + // (which is the above line freezes) + y.push(props.p2); + + return ; +} + +``` + + +## Error + +``` +[ReactForget] InvalidInput: InferReferenceEffects: inferred mutation of known immutable value $42:TObject (frozen) (13:13) +``` + + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-aliased-freeze.js b/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-aliased-freeze.js new file mode 100644 index 0000000000..8b08aaa215 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-aliased-freeze.js @@ -0,0 +1,16 @@ +function Component(props) { + let x = []; + let y = x; + + if (props.p1) { + x = []; + } + + let _ = ; + + // y is MaybeFrozen at this point, since it may alias to x + // (which is the above line freezes) + y.push(props.p2); + + return ; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-freeze.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-freeze.expect.md new file mode 100644 index 0000000000..491ed4e8fe --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-freeze.expect.md @@ -0,0 +1,25 @@ + +## Input + +```javascript +function Component(props) { + let x = []; + + let _ = ; + + // x is Frozen at this point + x.push(props.p2); + + return
{_}
; +} + +``` + + +## Error + +``` +[ReactForget] InvalidInput: InferReferenceEffects: inferred mutation of known immutable value $25:TObject (frozen) (7:7) +``` + + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-freeze.js b/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-freeze.js new file mode 100644 index 0000000000..a40fbcb31e --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.mutate-after-freeze.js @@ -0,0 +1,10 @@ +function Component(props) { + let x = []; + + let _ = ; + + // x is Frozen at this point + x.push(props.p2); + + return
{_}
; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/overlapping-scopes-within-block.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/overlapping-scopes-within-block.expect.md index cb78891139..0618885d00 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/overlapping-scopes-within-block.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/overlapping-scopes-within-block.expect.md @@ -21,7 +21,7 @@ function foo(a, b, c) { ```javascript function foo(a, b, c) { - const $ = React.unstable_useMemoCache(4); + const $ = React.unstable_useMemoCache(7); const c_0 = $[0] !== a; const c_1 = $[1] !== b; const c_2 = $[2] !== c; @@ -29,9 +29,19 @@ function foo(a, b, c) { if (c_0 || c_1 || c_2) { x = []; if (a) { - const y = []; - if (b) { - y.push(c); + const c_4 = $[4] !== b; + const c_5 = $[5] !== c; + let y; + if (c_4 || c_5) { + y = []; + if (b) { + y.push(c); + } + $[4] = b; + $[5] = c; + $[6] = y; + } else { + y = $[6]; } x.push(y); diff --git a/compiler/forget/src/__tests__/fixtures/compiler/reactive-scope-grouping.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/reactive-scope-grouping.expect.md index 170f98b03b..136c5a00ff 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/reactive-scope-grouping.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/reactive-scope-grouping.expect.md @@ -18,14 +18,21 @@ function foo() { ```javascript function foo() { - const $ = React.unstable_useMemoCache(2); + const $ = React.unstable_useMemoCache(3); let x; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { x = {}; let y; if ($[1] === Symbol.for("react.memo_cache_sentinel")) { y = []; - const z = {}; + let t0; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t0 = {}; + $[2] = t0; + } else { + t0 = $[2]; + } + const z = t0; y.push(z); $[1] = y; } else { diff --git a/compiler/forget/src/__tests__/fixtures/compiler/reassignment-conditional.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/reassignment-conditional.expect.md index 264c35876e..a2d86d537a 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/reassignment-conditional.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/reassignment-conditional.expect.md @@ -11,7 +11,6 @@ function Component(props) { x = []; } - let _ = ; y.push(props.p2); return ; @@ -23,40 +22,47 @@ function Component(props) { ```javascript function Component(props) { - const $ = React.unstable_useMemoCache(6); + const $ = React.unstable_useMemoCache(9); const c_0 = $[0] !== props.p0; + const c_1 = $[1] !== props.p1; + const c_2 = $[2] !== props.p2; let x; - if (c_0) { + let y; + if (c_0 || c_1 || c_2) { x = []; x.push(props.p0); - $[0] = props.p0; - $[1] = x; - } else { - x = $[1]; - } - const y = x; - if (props.p1) { - let t0; - if ($[2] === Symbol.for("react.memo_cache_sentinel")) { - t0 = []; - $[2] = t0; - } else { - t0 = $[2]; + y = x; + if (props.p1) { + let t0; + if ($[5] === Symbol.for("react.memo_cache_sentinel")) { + t0 = []; + $[5] = t0; + } else { + t0 = $[5]; + } + x = t0; } - x = t0; - } - y.push(props.p2); - const c_3 = $[3] !== x; - const c_4 = $[4] !== y; - let t1; - if (c_3 || c_4) { - t1 = ; + y.push(props.p2); + $[0] = props.p0; + $[1] = props.p1; + $[2] = props.p2; $[3] = x; $[4] = y; - $[5] = t1; } else { - t1 = $[5]; + x = $[3]; + y = $[4]; + } + const c_6 = $[6] !== x; + const c_7 = $[7] !== y; + let t1; + if (c_6 || c_7) { + t1 = ; + $[6] = x; + $[7] = y; + $[8] = t1; + } else { + t1 = $[8]; } return t1; } diff --git a/compiler/forget/src/__tests__/fixtures/compiler/reassignment-conditional.js b/compiler/forget/src/__tests__/fixtures/compiler/reassignment-conditional.js index 3c45161aa9..4e30a5a390 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/reassignment-conditional.js +++ b/compiler/forget/src/__tests__/fixtures/compiler/reassignment-conditional.js @@ -7,7 +7,6 @@ function Component(props) { x = []; } - let _ = ; y.push(props.p2); return ; diff --git a/compiler/forget/src/__tests__/fixtures/hir/array-at-closure.expect.md b/compiler/forget/src/__tests__/fixtures/hir/array-at-closure.expect.md new file mode 100644 index 0000000000..77f0a86519 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/array-at-closure.expect.md @@ -0,0 +1,47 @@ + +## Input + +```javascript +function Component(props) { + const x = foo(props.x); + const fn = function () { + const arr = [...bar(props)]; + return arr.at(x); + }; + const fnResult = fn(); + return fnResult; +} + +``` + +## HIR + +```javascript +bb0 (block): + [1] mutate $31:TFunction = Global foo + [2] mutate $32 = LoadLocal read props$30 + [3] mutate $33 = PropertyLoad read $32.x + [4] mutate $34 = Call read $31:TFunction(read $33) + [5] store $36 = StoreLocal Const mutate x$35 = capture $34 + [6] mutate $37 = LoadLocal read props$30 + [7] mutate $38 = LoadLocal capture x$35 + [8] store $39[8:12]:TFunction = Function @deps[read $37,read $38]: + bb0 (block): + [1] mutate $49:TFunction = Global bar + [2] mutate $50[2:4] = LoadLocal capture props$47[0:4] + [3] mutate $51 = Call read $49:TFunction(mutate $50[2:4]) + [4] store $52:TObject = Array [...capture $51] + [5] store $54:TObject = StoreLocal Const store arr$53:TObject = capture $52:TObject + [6] mutate $55:TObject = LoadLocal capture arr$53:TObject + [7] mutate $56 = LoadLocal capture x$48 + [8] mutate $57:TFunction<> = PropertyLoad read $55:TObject.at + [9] mutate $58 = PropertyCall read $55:TObject.read $57:TFunction<>(read $56) + [10] Return freeze $58 + [9] store $41[9:12]:TFunction = StoreLocal Const mutate fn$40[9:12]:TFunction = capture $39[8:12]:TFunction + [10] mutate $42[10:12]:TFunction = LoadLocal capture fn$40[9:12]:TFunction + [11] mutate $43 = Call mutate $42[10:12]:TFunction() + [12] store $45 = StoreLocal Const mutate fnResult$44 = capture $43 + [13] mutate $46 = LoadLocal capture fnResult$44 + [14] Return freeze $46 +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/array-at-closure.js b/compiler/forget/src/__tests__/fixtures/hir/array-at-closure.js new file mode 100644 index 0000000000..244b834756 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/array-at-closure.js @@ -0,0 +1,9 @@ +function Component(props) { + const x = foo(props.x); + const fn = function () { + const arr = [...bar(props)]; + return arr.at(x); + }; + const fnResult = fn(); + return fnResult; +} diff --git a/compiler/forget/src/__tests__/fixtures/hir/array-at-effect.expect.md b/compiler/forget/src/__tests__/fixtures/hir/array-at-effect.expect.md new file mode 100644 index 0000000000..b9119689c8 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/array-at-effect.expect.md @@ -0,0 +1,38 @@ + +## Input + +```javascript +// arrayInstance.at should have the following effects: +// - read on arg0 +// - read on receiver +// - mutate on lvalue +function ArrayAtTest(props) { + const arr = [foo(props.x)]; + const result = arr.at(bar(props.y)); + return result; +} + +``` + +## HIR + +```javascript +bb0 (block): + [1] mutate $20:TFunction = Global foo + [2] mutate $21 = LoadLocal read props$19 + [3] mutate $22 = PropertyLoad read $21.x + [4] mutate $23 = Call read $20:TFunction(read $22) + [5] store $24:TObject = Array [capture $23] + [6] store $26:TObject = StoreLocal Const store arr$25:TObject = capture $24:TObject + [7] mutate $27:TObject = LoadLocal capture arr$25:TObject + [8] mutate $28:TFunction = Global bar + [9] mutate $29 = LoadLocal read props$19 + [10] mutate $30 = PropertyLoad read $29.y + [11] mutate $31 = Call read $28:TFunction(read $30) + [12] mutate $32:TFunction<> = PropertyLoad read $27:TObject.at + [13] mutate $33 = PropertyCall read $27:TObject.read $32:TFunction<>(read $31) + [14] store $35 = StoreLocal Const mutate result$34 = capture $33 + [15] mutate $36 = LoadLocal capture result$34 + [16] Return freeze $36 +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/array-at-effect.js b/compiler/forget/src/__tests__/fixtures/hir/array-at-effect.js new file mode 100644 index 0000000000..2e2e78e50e --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/array-at-effect.js @@ -0,0 +1,9 @@ +// arrayInstance.at should have the following effects: +// - read on arg0 +// - read on receiver +// - mutate on lvalue +function ArrayAtTest(props) { + const arr = [foo(props.x)]; + const result = arr.at(bar(props.y)); + return result; +} diff --git a/compiler/forget/src/__tests__/fixtures/hir/array-property-call.expect.md b/compiler/forget/src/__tests__/fixtures/hir/array-property-call.expect.md index bad8635364..92d83e67a2 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/array-property-call.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/array-property-call.expect.md @@ -21,20 +21,20 @@ bb0 (block): [3] mutate $30 = LoadLocal read props$27 [4] mutate $31 = PropertyLoad read $30.b [5] mutate $32:TPrimitive = "hello" - [6] store $33[6:18]:TObject = Array [read $29, read $31, read $32:TPrimitive] - [7] store $35[7:18]:TObject = StoreLocal Const store a$34[7:18]:TObject = capture $33[6:18]:TObject - [8] mutate $36[8:18]:TObject = LoadLocal capture a$34[7:18]:TObject + [6] store $33[6:12]:TObject = Array [read $29, read $31, read $32:TPrimitive] + [7] store $35[7:12]:TObject = StoreLocal Const store a$34[7:12]:TObject = capture $33[6:12]:TObject + [8] mutate $36[8:12]:TObject = LoadLocal capture a$34[7:12]:TObject [9] mutate $37:TPrimitive = 42 - [10] mutate $38[10:18]:TFunction<> = PropertyLoad read $36[8:18]:TObject.push - [11] mutate $39:TPrimitive = PropertyCall mutate $36[8:18]:TObject.read $38[10:18]:TFunction<>(read $37:TPrimitive) + [10] mutate $38[10:12]:TFunction<> = PropertyLoad read $36[8:12]:TObject.push + [11] mutate $39:TPrimitive = PropertyCall mutate $36[8:12]:TObject.read $38[10:12]:TFunction<>(read $37:TPrimitive) [12] store $41:TPrimitive = StoreLocal Const mutate x$40:TPrimitive = capture $39:TPrimitive - [13] mutate $42[13:18]:TObject = LoadLocal capture a$34[7:18]:TObject + [13] mutate $42:TObject = LoadLocal capture a$34[7:12]:TObject [14] mutate $43 = LoadLocal read props$27 [15] mutate $44 = PropertyLoad read $43.c - [16] mutate $45[16:18]:TFunction<> = PropertyLoad read $42[13:18]:TObject.at - [17] mutate $46 = PropertyCall mutate $42[13:18]:TObject.read $45[16:18]:TFunction<>(read $44) + [16] mutate $45:TFunction<> = PropertyLoad read $42:TObject.at + [17] mutate $46 = PropertyCall read $42:TObject.read $45:TFunction<>(read $44) [18] store $48 = StoreLocal Const mutate y$47 = capture $46 - [19] mutate $49:TObject = LoadLocal capture a$34[7:18]:TObject + [19] mutate $49:TObject = LoadLocal capture a$34[7:12]:TObject [20] mutate $50:TPrimitive = LoadLocal capture x$40:TPrimitive [21] mutate $51 = LoadLocal capture y$47 [22] store $52:TObject = Object { a: capture $49:TObject, x: capture $50:TPrimitive, y: capture $51 } diff --git a/compiler/forget/src/__tests__/fixtures/hir/array-push-effect.expect.md b/compiler/forget/src/__tests__/fixtures/hir/array-push-effect.expect.md new file mode 100644 index 0000000000..8f90200f08 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/array-push-effect.expect.md @@ -0,0 +1,46 @@ + +## Input + +```javascript +// arrayInstance.push should have the following effects: +// - read on all args (rest parameter) +// - mutate on receiver +function Component(props) { + const x = foo(props.x); + const y = { y: props.y }; + const arr = []; + arr.push({}); + arr.push(x, y); + return arr; +} + +``` + +## HIR + +```javascript +bb0 (block): + [1] mutate $27:TFunction = Global foo + [2] mutate $28 = LoadLocal read props$26 + [3] mutate $29 = PropertyLoad read $28.x + [4] mutate $30 = Call read $27:TFunction(read $29) + [5] store $32 = StoreLocal Const mutate x$31 = capture $30 + [6] mutate $33 = LoadLocal read props$26 + [7] mutate $34 = PropertyLoad read $33.y + [8] store $35:TObject = Object { y: read $34 } + [9] store $37:TObject = StoreLocal Const store y$36:TObject = capture $35:TObject + [10] store $38[10:21]:TObject = Array [] + [11] store $40[11:21]:TObject = StoreLocal Const store arr$39[11:21]:TObject = capture $38[10:21]:TObject + [12] mutate $41[12:21]:TObject = LoadLocal capture arr$39[11:21]:TObject + [13] store $42:TObject = Object { } + [14] mutate $43[14:21]:TFunction<> = PropertyLoad read $41[12:21]:TObject.push + [15] mutate $44:TPrimitive = PropertyCall mutate $41[12:21]:TObject.read $43[14:21]:TFunction<>(capture $42:TObject) + [16] mutate $45[16:21]:TObject = LoadLocal capture arr$39[11:21]:TObject + [17] mutate $46 = LoadLocal capture x$31 + [18] mutate $47:TObject = LoadLocal capture y$36:TObject + [19] mutate $48[19:21]:TFunction<> = PropertyLoad read $45[16:21]:TObject.push + [20] mutate $49:TPrimitive = PropertyCall mutate $45[16:21]:TObject.read $48[19:21]:TFunction<>(capture $46, capture $47:TObject) + [21] mutate $50:TObject = LoadLocal capture arr$39[11:21]:TObject + [22] Return freeze $50:TObject +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/array-push-effect.js b/compiler/forget/src/__tests__/fixtures/hir/array-push-effect.js new file mode 100644 index 0000000000..b91eff8642 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/array-push-effect.js @@ -0,0 +1,11 @@ +// arrayInstance.push should have the following effects: +// - read on all args (rest parameter) +// - mutate on receiver +function Component(props) { + const x = foo(props.x); + const y = { y: props.y }; + const arr = []; + arr.push({}); + arr.push(x, y); + return arr; +}