diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/Globals.ts index ae70a49c63..931d315d30 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/Globals.ts @@ -89,7 +89,7 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ restParam: null, returnType: { kind: "Primitive" }, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], /* @@ -130,7 +130,7 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ restParam: Effect.Read, returnType: { kind: "Primitive" }, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], ]), @@ -147,7 +147,7 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ restParam: Effect.Read, returnType: { kind: "Primitive" }, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], [ @@ -157,7 +157,7 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ restParam: Effect.Read, returnType: { kind: "Primitive" }, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], [ @@ -167,7 +167,7 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ restParam: Effect.Read, returnType: { kind: "Primitive" }, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], [ @@ -177,7 +177,7 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ restParam: Effect.Read, returnType: { kind: "Primitive" }, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], [ @@ -187,7 +187,7 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ restParam: Effect.Read, returnType: { kind: "Primitive" }, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], [ @@ -197,7 +197,7 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ restParam: Effect.Read, returnType: { kind: "Primitive" }, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], ]), @@ -209,7 +209,7 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ restParam: Effect.Read, returnType: { kind: "Primitive" }, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], [ @@ -219,7 +219,7 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ restParam: Effect.Read, returnType: { kind: "Primitive" }, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], [ @@ -229,7 +229,7 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ restParam: Effect.Read, returnType: { kind: "Primitive" }, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], // TODO: rest of Global objects diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts index 6d3f4a1074..b4e659ec28 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts @@ -1244,7 +1244,8 @@ export enum ValueReason { export enum ValueKind { MaybeFrozen = "maybefrozen", Frozen = "frozen", - Immutable = "immutable", + Primitive = "primitive", + Global = "global", Mutable = "mutable", Context = "context", } diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/ObjectShape.ts index 22fd4ee3e2..33a0351ba1 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/ObjectShape.ts @@ -235,7 +235,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [ restParam: Effect.Capture, returnType: PRIMITIVE_TYPE, calleeEffect: Effect.Store, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], [ @@ -284,7 +284,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [ * the array object itself is not modified */ calleeEffect: Effect.ConditionallyMutate, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, noAlias: true, mutableOnlyIfOperandsAreMutable: true, }), @@ -301,7 +301,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [ * the array object itself is not modified */ calleeEffect: Effect.ConditionallyMutate, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, noAlias: true, mutableOnlyIfOperandsAreMutable: true, }), @@ -330,7 +330,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [ * the array object itself is not modified */ calleeEffect: Effect.ConditionallyMutate, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, noAlias: true, mutableOnlyIfOperandsAreMutable: true, }), @@ -342,7 +342,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [ restParam: Effect.ConditionallyMutate, returnType: PRIMITIVE_TYPE, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], // TODO: rest of Array properties @@ -357,7 +357,7 @@ addObject(BUILTIN_SHAPES, BuiltInObjectId, [ restParam: null, returnType: PRIMITIVE_TYPE, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], /* @@ -378,7 +378,7 @@ addObject(BUILTIN_SHAPES, BuiltInUseStateId, [ restParam: Effect.Freeze, returnType: PRIMITIVE_TYPE, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }, BuiltInSetStateId ), @@ -401,7 +401,7 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [ restParam: Effect.Read, returnType: PRIMITIVE_TYPE, calleeEffect: Effect.Read, - returnValueKind: ValueKind.Immutable, + returnValueKind: ValueKind.Primitive, }), ], [ diff --git a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts index 6b76c4aae7..24d72d1e34 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts @@ -531,7 +531,11 @@ class InferenceState { ? new Set([place]) : valueKind.context, }; - } else if (valueKind.kind !== ValueKind.Mutable) { + } else if ( + valueKind.kind !== ValueKind.Mutable && + // We ignore mutations of primitives since this is not a React-specific problem + valueKind.kind !== ValueKind.Primitive + ) { let reason = getWriteErrorReason(valueKind); functionEffect = { kind: @@ -566,7 +570,11 @@ class InferenceState { ? new Set([place]) : valueKind.context, }; - } else if (valueKind.kind !== ValueKind.Mutable) { + } else if ( + valueKind.kind !== ValueKind.Mutable && + // We ignore mutations of primitives since this is not a React-specific problem + valueKind.kind !== ValueKind.Primitive + ) { let reason = getWriteErrorReason(valueKind); functionEffect = { kind: @@ -601,7 +609,8 @@ class InferenceState { } case Effect.Capture: { if ( - valueKind.kind === ValueKind.Immutable || + valueKind.kind === ValueKind.Primitive || + valueKind.kind === ValueKind.Global || valueKind.kind === ValueKind.Frozen || valueKind.kind === ValueKind.MaybeFrozen ) { @@ -876,9 +885,20 @@ function mergeValues(a: ValueKind, b: ValueKind): ValueKind { // context | immutable return ValueKind.Context; } - } else { - // frozen | immutable + } else if (a === ValueKind.Frozen || b === ValueKind.Frozen) { return ValueKind.Frozen; + } else if (a === ValueKind.Global || b === ValueKind.Global) { + return ValueKind.Global; + } else { + CompilerError.invariant( + a === ValueKind.Primitive && b == ValueKind.Primitive, + { + reason: `Unexpected value kind in mergeValues()`, + description: `Found kinds ${a} and ${b}`, + loc: GeneratedSource, + } + ); + return ValueKind.Primitive; } } @@ -940,7 +960,7 @@ function inferBlock( switch (instrValue.kind) { case "BinaryExpression": { valueKind = { - kind: ValueKind.Immutable, + kind: ValueKind.Primitive, reason: new Set([ValueReason.Other]), context: new Set(), }; @@ -1066,7 +1086,7 @@ function inferBlock( } case "UnaryExpression": { valueKind = { - kind: ValueKind.Immutable, + kind: ValueKind.Primitive, reason: new Set([ValueReason.Other]), context: new Set(), }; @@ -1121,7 +1141,7 @@ function inferBlock( * an immutable string */ valueKind = { - kind: ValueKind.Immutable, + kind: ValueKind.Primitive, reason: new Set([ValueReason.Other]), context: new Set(), }; @@ -1143,7 +1163,7 @@ function inferBlock( } case "LoadGlobal": valueKind = { - kind: ValueKind.Immutable, + kind: ValueKind.Global, reason: new Set([ValueReason.Global]), context: new Set(), }; @@ -1152,7 +1172,7 @@ function inferBlock( case "JSXText": case "Primitive": { valueKind = { - kind: ValueKind.Immutable, + kind: ValueKind.Primitive, reason: new Set([ValueReason.Other]), context: new Set(), }; @@ -1454,7 +1474,7 @@ function inferBlock( case "PropertyDelete": { // `delete` returns a boolean (immutable) and modifies the object valueKind = { - kind: ValueKind.Immutable, + kind: ValueKind.Primitive, reason: new Set([ValueReason.Other]), context: new Set(), }; @@ -1517,7 +1537,7 @@ function inferBlock( functionEffects ); state.initialize(instrValue, { - kind: ValueKind.Immutable, + kind: ValueKind.Primitive, reason: new Set([ValueReason.Other]), context: new Set(), }); @@ -1605,7 +1625,7 @@ function inferBlock( const lvalue = instr.lvalue; lvalue.effect = Effect.ConditionallyMutate; state.initialize(instrValue, { - kind: ValueKind.Immutable, + kind: ValueKind.Frozen, reason: new Set([ValueReason.Other]), context: new Set(), }); @@ -1656,7 +1676,7 @@ function inferBlock( context: new Set(), } : { - kind: ValueKind.Immutable, + kind: ValueKind.Primitive, reason: new Set([ValueReason.Other]), context: new Set(), } @@ -1878,7 +1898,7 @@ function inferBlock( effect = { kind: Effect.Read, reason: ValueReason.Other }; lvalueEffect = Effect.Store; valueKind = { - kind: ValueKind.Immutable, + kind: ValueKind.Primitive, reason: new Set([ValueReason.Other]), context: new Set(), }; @@ -2009,7 +2029,8 @@ function areArgumentsImmutableAndNonMutating( const place = arg.kind === "Identifier" ? arg : arg.place; const kind = state.kind(place).kind; switch (kind) { - case ValueKind.Immutable: + case ValueKind.Global: + case ValueKind.Primitive: case ValueKind.Frozen: { /* * Only immutable values, or frozen lambdas are allowed. diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-props-in-effect-fixpoint.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-props-in-effect-fixpoint.expect.md index a30dd36cd2..19879150d0 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-props-in-effect-fixpoint.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-props-in-effect-fixpoint.expect.md @@ -28,7 +28,7 @@ function Component(props) { 8 | let y = x; 9 | let mutateProps = () => { > 10 | y.foo = true; - | ^ InvalidReact: This mutates a variable that React considers immutable. Found mutation of `y` (10:10) + | ^ InvalidReact: Mutating component props or hook arguments is not allowed. Consider using a local variable instead. Found mutation of `y` (10:10) 11 | }; 12 | let mutatePropsIndirect = () => { 13 | mutateProps(); diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-for-of-capture-item-of-local-collection-mutate-later-value-initially-null.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-for-of-capture-item-of-local-collection-mutate-later-value-initially-null.expect.md deleted file mode 100644 index 3c0eea1666..0000000000 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-for-of-capture-item-of-local-collection-mutate-later-value-initially-null.expect.md +++ /dev/null @@ -1,34 +0,0 @@ - -## Input - -```javascript -import { makeObject_Primitives } from "shared-runtime"; - -function Component(props) { - let lastItem = null; // we reject this code bc `lastItem` could be null and you can't mutate null - const items = [makeObject_Primitives(), makeObject_Primitives()]; - for (const x of items) { - lastItem = x; - } - if (lastItem != null) { - lastItem.mutated = true; - } - return items; -} - -``` - - -## Error - -``` - 8 | } - 9 | if (lastItem != null) { -> 10 | lastItem.mutated = true; - | ^^^^^^^^ InvalidReact: This mutates a variable that React considers immutable (10:10) - 11 | } - 12 | return items; - 13 | } -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-of-capture-item-of-local-collection-mutate-later-value-initially-null.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-of-capture-item-of-local-collection-mutate-later-value-initially-null.expect.md new file mode 100644 index 0000000000..e245f5f4e7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-of-capture-item-of-local-collection-mutate-later-value-initially-null.expect.md @@ -0,0 +1,63 @@ + +## Input + +```javascript +import { makeObject_Primitives } from "shared-runtime"; + +function Component(props) { + let lastItem = null; + const items = [makeObject_Primitives(), makeObject_Primitives()]; + for (const x of items) { + lastItem = x; + } + if (lastItem != null) { + lastItem.a += 1; + } + return items; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}, {}], +}; + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +import { makeObject_Primitives } from "shared-runtime"; + +function Component(props) { + const $ = useMemoCache(1); + let items; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + let lastItem = null; + items = [makeObject_Primitives(), makeObject_Primitives()]; + for (const x of items) { + lastItem = x; + } + if (lastItem != null) { + lastItem.a = lastItem.a + 1; + } + $[0] = items; + } else { + items = $[0]; + } + return items; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}, {}], +}; + +``` + +### Eval output +(kind: ok) [{"a":0,"b":"value1","c":true},{"a":1,"b":"value1","c":true}] +[{"a":0,"b":"value1","c":true},{"a":1,"b":"value1","c":true}] +[{"a":0,"b":"value1","c":true},{"a":1,"b":"value1","c":true}] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-for-of-capture-item-of-local-collection-mutate-later-value-initially-null.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-of-capture-item-of-local-collection-mutate-later-value-initially-null.js similarity index 62% rename from compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-for-of-capture-item-of-local-collection-mutate-later-value-initially-null.js rename to compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-of-capture-item-of-local-collection-mutate-later-value-initially-null.js index 3695bcdb2c..962ab8e42e 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-for-of-capture-item-of-local-collection-mutate-later-value-initially-null.js +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-of-capture-item-of-local-collection-mutate-later-value-initially-null.js @@ -1,13 +1,19 @@ import { makeObject_Primitives } from "shared-runtime"; function Component(props) { - let lastItem = null; // we reject this code bc `lastItem` could be null and you can't mutate null + let lastItem = null; const items = [makeObject_Primitives(), makeObject_Primitives()]; for (const x of items) { lastItem = x; } if (lastItem != null) { - lastItem.mutated = true; + lastItem.a += 1; } return items; } + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], + sequentialRenders: [{}, {}, {}], +};