Add Effect.Mutate for known mutation
Adds back `Effect.Mutate`, and changes so that `Effect.ConditionallyMutate` never rejects frozen/immutable values, while `Effect.Mutate` _always_ rejects frozen/immutable values.
Joe Savona committed
May 25, 2023 at 13:54 UTC
9766e1980523e3ee214aca72dad23177d4250f84
15 files changed
+181
-117
compiler/forget/src/HIR/HIR.ts
+20
-4
@@ -7,6 +7,8 @@
7
8
import * as t from "@babel/types";
9
import invariant from "invariant";
10
+import { CompilerError } from "../CompilerError";
11
+import { assertExhaustive } from "../Utils/utils";
12
import { Environment } from "./Environment";
13
import { HookKind } from "./ObjectShape";
14
import { Type } from "./Types";
@@ -856,7 +858,7 @@ export enum Effect {
858
ConditionallyMutate = "mutate?",
859
// This reference *does* write to (mutate) the value. It is an error (invalid input)
860
// if an immutable value flows into a location with this effect.
859
- // DefiniitelyMutate = "mutate",
861
+ Mutate = "mutate",
862
// This reference may alias to (mutate) the value
863
Store = "store",
864
}
@@ -864,13 +866,27 @@ export enum Effect {
866
export function isMutableEffect(effect: Effect): boolean {
867
switch (effect) {
868
case Effect.Capture:
867
- case Effect.ConditionallyMutate:
868
- case Effect.Store: {
869
+ case Effect.Store:
870
+ case Effect.Mutate: {
871
return true;
872
}
871
- default: {
873
+ case Effect.ConditionallyMutate: {
874
+ // All conditional mutations should be resolved into some other effect after InferReferenceEffects
875
+ CompilerError.invariant(
876
+ "Unexpected conditional mutation effect",
877
+ GeneratedSource
878
+ );
879
+ }
880
+ case Effect.Unknown: {
881
+ CompilerError.invariant("Unexpected unknown effect", GeneratedSource);
882
+ }
883
+ case Effect.Read:
884
+ case Effect.Freeze: {
885
return false;
886
}
887
+ default: {
888
+ assertExhaustive(effect, `Unexpected effect '${effect}'`);
889
+ }
890
}
891
}
892
compiler/forget/src/Inference/AnalyseFunctions.ts
+1
-1
@@ -94,7 +94,7 @@ function lower(func: HIRFunction): void {
94
constantPropagation(func);
95
inferTypes(func);
96
analyseFunctions(func);
97
- inferReferenceEffects(func);
97
+ inferReferenceEffects(func, { isFunctionExpression: true });
98
inferMutableRanges(func);
99
logHIRFunction("AnalyseFunction (inner)", func);
100
}
compiler/forget/src/Inference/InferMutableLifetimes.ts
+2
-1
@@ -86,7 +86,8 @@ function inferPlace(
86
infer(place, instrId);
87
}
88
return;
89
- case Effect.ConditionallyMutate: {
89
+ case Effect.ConditionallyMutate:
90
+ case Effect.Mutate: {
91
infer(place, instrId);
92
return;
93
}
compiler/forget/src/Inference/InferReferenceEffects.ts
+39
-61
@@ -24,11 +24,7 @@ import {
24
Type,
25
ValueKind,
26
} from "../HIR/HIR";
27
-import {
28
- DefaultMutatingHook,
29
- DefaultNonmutatingHook,
30
- FunctionSignature,
31
-} from "../HIR/ObjectShape";
27
+import { FunctionSignature } from "../HIR/ObjectShape";
28
import {
29
printIdentifier,
30
printMixedHIR,
@@ -87,7 +83,10 @@ import { assertExhaustive } from "../Utils/utils";
83
* When control flow paths converge the types of values are merged together, with the value
84
* types forming a lattice to ensure convergence.
85
*/
90
-export default function inferReferenceEffects(fn: HIRFunction): void {
86
+export default function inferReferenceEffects(
87
+ fn: HIRFunction,
88
+ options: { isFunctionExpression: boolean } = { isFunctionExpression: false }
89
+): void {
90
// Initial state contains function params
91
// TODO: include module declarations here as well
92
const initialState = InferenceState.empty();
@@ -118,13 +117,16 @@ export default function inferReferenceEffects(fn: HIRFunction): void {
117
initialState.define(ref, value);
118
}
119
120
+ const paramKind = options.isFunctionExpression
121
+ ? ValueKind.Mutable
122
+ : ValueKind.Frozen;
123
for (const param of fn.params) {
124
const value: InstructionValue = {
125
kind: "Primitive",
126
loc: param.loc,
127
value: undefined,
128
};
127
- initialState.initialize(value, ValueKind.Frozen);
129
+ initialState.initialize(value, paramKind);
130
initialState.define(param, value);
131
}
132
@@ -273,18 +275,6 @@ class InferenceState {
275
* value is already frozen or is immutable.
276
*/
277
reference(place: Place, effectKind: Effect): void {
276
- this.#referenceImpl(place, effectKind, false);
277
- }
278
-
279
- /**
280
- * Throwing version of {@link reference}, which throws with an error
281
- * if we record a mutate effect on an immutable value.
282
- */
283
- referenceAndCheckError(place: Place, effectKind: Effect): void {
284
- this.#referenceImpl(place, effectKind, true);
285
- }
286
-
287
- #referenceImpl(place: Place, effectKind: Effect, shouldError: boolean): void {
278
const values = this.#variables.get(place.identifier.id);
279
if (values === undefined) {
280
if (effectKind === Effect.Store) {
@@ -321,26 +311,33 @@ class InferenceState {
311
valueKind === ValueKind.Mutable ||
312
valueKind === ValueKind.Context
313
) {
324
- effect = Effect.ConditionallyMutate;
314
+ effect = Effect.Mutate;
315
} else {
326
- if (shouldError) {
327
- CompilerError.invalidInput(
328
- `InferReferenceEffects: inferred mutation of known immutable value`,
329
- place.loc,
330
- `Found mutation of ${printIdentifier(
331
- place.identifier
332
- )}${printType(place.identifier.type)} (${valueKind})`
333
- );
334
- }
316
effect = Effect.Read;
317
}
318
break;
319
}
320
+ case Effect.Mutate: {
321
+ if (
322
+ valueKind === ValueKind.Mutable ||
323
+ valueKind === ValueKind.Context
324
+ ) {
325
+ effect = Effect.Mutate;
326
+ } else {
327
+ CompilerError.invalidInput(
328
+ `InferReferenceEffects: inferred mutation of known immutable value`,
329
+ place.loc,
330
+ `Found mutation of ${printIdentifier(place.identifier)}${printType(
331
+ place.identifier.type
332
+ )} (${valueKind})`
333
+ );
334
+ }
335
+ break;
336
+ }
337
case Effect.Store: {
338
if (
339
valueKind !== ValueKind.Mutable &&
342
- valueKind !== ValueKind.Context &&
343
- shouldError
340
+ valueKind !== ValueKind.Context
341
) {
342
CompilerError.invalidInput(
343
`InferReferenceEffects: inferred mutation of known immutable value`,
@@ -357,9 +354,7 @@ class InferenceState {
354
// valueKind === ValueKind.Mutable,
355
// `expected valueKind to be 'Mutable' but found to be '${valueKind}'`
356
// );
360
- effect = isObjectType(place.identifier)
361
- ? Effect.Store
362
- : Effect.ConditionallyMutate;
357
+ effect = isObjectType(place.identifier) ? Effect.Store : Effect.Mutate;
358
break;
359
}
360
case Effect.Capture: {
@@ -737,13 +732,6 @@ function inferBlock(
732
break;
733
}
734
740
- // We currently always check reference effects of typed functions
741
- // (i.e. call `referenceAndCheckError`). However, default custom hooks
742
- // should not assert reference effects, since their signatures are only
743
- // assumptions / defaults.
744
- const isDefaultCustomHook =
745
- instrValue.callee.identifier.type === DefaultMutatingHook ||
746
- instrValue.callee.identifier.type === DefaultNonmutatingHook;
735
const effects =
736
signature !== null ? getFunctionEffects(instrValue, signature) : null;
737
const returnValueKind =
@@ -752,22 +740,13 @@ function inferBlock(
740
const arg = instrValue.args[i];
741
const place = arg.kind === "Identifier" ? arg : arg.place;
742
if (effects !== null) {
755
- if (isDefaultCustomHook) {
756
- state.reference(place, effects[i]);
757
- } else {
758
- // If effects are inferred for an argument, we should fail invalid
759
- // mutating effects
760
- state.referenceAndCheckError(place, effects[i]);
761
- }
743
+ state.reference(place, effects[i]);
744
} else {
745
state.reference(place, Effect.ConditionallyMutate);
746
}
747
}
748
if (signature !== null) {
767
- state.referenceAndCheckError(
768
- instrValue.callee,
769
- signature.calleeEffect
770
- );
749
+ state.reference(instrValue.callee, signature.calleeEffect);
750
} else {
751
state.reference(instrValue.callee, Effect.ConditionallyMutate);
752
}
@@ -797,16 +776,13 @@ function inferBlock(
776
if (effects !== null) {
777
// If effects are inferred for an argument, we should fail invalid
778
// mutating effects
800
- state.referenceAndCheckError(place, effects[i]);
779
+ state.reference(place, effects[i]);
780
} else {
781
state.reference(place, Effect.ConditionallyMutate);
782
}
783
}
784
if (signature !== null) {
806
- state.referenceAndCheckError(
807
- instrValue.receiver,
808
- signature.calleeEffect
809
- );
785
+ state.reference(instrValue.receiver, signature.calleeEffect);
786
} else {
787
state.reference(instrValue.receiver, Effect.ConditionallyMutate);
788
}
@@ -858,7 +834,7 @@ function inferBlock(
834
continue;
835
}
836
case "ComputedDelete": {
861
- state.reference(instrValue.object, Effect.ConditionallyMutate);
837
+ state.reference(instrValue.object, Effect.Mutate);
838
state.reference(instrValue.property, Effect.Read);
839
state.initialize(instrValue, ValueKind.Immutable);
840
state.reference(instr.lvalue, Effect.ConditionallyMutate);
@@ -951,12 +927,13 @@ function inferBlock(
927
state.alias(lvalue, instrValue.value);
928
lvalue.effect = Effect.Store;
929
state.alias(instrValue.lvalue.place, instrValue.value);
954
- state.reference(instrValue.lvalue.place, Effect.Store);
930
+ // state.reference(instrValue.lvalue.place, Effect.Store);
931
+ instrValue.lvalue.place.effect = Effect.Store;
932
continue;
933
}
934
case "StoreContext": {
935
state.reference(instrValue.value, Effect.ConditionallyMutate);
959
- state.reference(instrValue.lvalue.place, Effect.ConditionallyMutate);
936
+ state.reference(instrValue.lvalue.place, Effect.Mutate);
937
938
const lvalue = instr.lvalue;
939
state.alias(lvalue, instrValue.value);
@@ -981,7 +958,8 @@ function inferBlock(
958
lvalue.effect = Effect.Store;
959
for (const place of eachPatternOperand(instrValue.lvalue.pattern)) {
960
state.alias(place, instrValue.value);
984
- state.reference(place, Effect.Store);
961
+ // state.reference(place, Effect.Store);
962
+ place.effect = Effect.Store;
963
}
964
continue;
965
}
compiler/forget/src/ReactiveScopes/InferReactiveIdentifiers.ts
+2
-1
@@ -91,7 +91,8 @@ class Visitor extends ReactiveFunctionVisitor<State> {
91
switch (operand.effect) {
92
case Effect.Capture:
93
case Effect.Store:
94
- case Effect.ConditionallyMutate: {
94
+ case Effect.ConditionallyMutate:
95
+ case Effect.Mutate: {
96
const resolvedId: IdentifierId =
97
state.temporaries.get(operand.identifier.id) ??
98
operand.identifier.id;
compiler/forget/src/__tests__/fixtures/compiler/_bug.computed-call-evaluation-order.expect.md
+20
-15
@@ -3,12 +3,11 @@
3
4
```javascript
5
// Should print A, B, arg, original
6
-function changeF(o) {
7
- o.f = () => console.log("new");
8
-}
9
-
6
function Component() {
11
- let x = {
7
+ const changeF = (o) => {
8
+ o.f = () => console.log("new");
9
+ };
10
+ const x = {
11
f: () => console.log("original"),
12
};
13
@@ -24,31 +23,37 @@ function Component() {
23
24
```javascript
25
import { unstable_useMemoCache as useMemoCache } from "react"; // Should print A, B, arg, original
27
-function changeF(o) {
28
- o.f = () => console.log("new");
29
-}
30
-
26
function Component() {
32
- const $ = useMemoCache(2);
27
+ const $ = useMemoCache(3);
28
let t0;
29
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
35
- t0 = () => console.log("original");
30
+ t0 = (o) => {
31
+ o.f = () => console.log("new");
32
+ };
33
$[0] = t0;
34
} else {
35
t0 = $[0];
36
}
40
- let x;
37
+ const changeF = t0;
38
+ let t1;
39
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
42
- x = { f: t0 };
40
+ t1 = () => console.log("original");
41
+ $[1] = t1;
42
+ } else {
43
+ t1 = $[1];
44
+ }
45
+ let x;
46
+ if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
47
+ x = { f: t1 };
48
49
console.log("A");
50
console.log("B");
51
changeF(x);
52
console.log("arg");
53
x.f(1);
49
- $[1] = x;
54
+ $[2] = x;
55
} else {
51
- x = $[1];
56
+ x = $[2];
57
}
58
return x;
59
}
compiler/forget/src/__tests__/fixtures/compiler/_bug.computed-call-evaluation-order.js
+4
-5
@@ -1,10 +1,9 @@
1
// Should print A, B, arg, original
2
-function changeF(o) {
3
- o.f = () => console.log("new");
4
-}
5
-
2
function Component() {
7
- let x = {
3
+ const changeF = (o) => {
4
+ o.f = () => console.log("new");
5
+ };
6
+ const x = {
7
f: () => console.log("original"),
8
};
9
compiler/forget/src/__tests__/fixtures/compiler/_bug.property-call-evaluation-order.expect.md
+20
-15
@@ -4,12 +4,11 @@
4
```javascript
5
// Should print A, arg, original
6
7
-function changeF(o) {
8
- o.f = () => console.log("new");
9
-}
10
-
7
function Component() {
12
- let x = {
8
+ const changeF = (o) => {
9
+ o.f = () => console.log("new");
10
+ };
11
+ const x = {
12
f: () => console.log("original"),
13
};
14
@@ -24,30 +23,36 @@ function Component() {
23
```javascript
24
import { unstable_useMemoCache as useMemoCache } from "react"; // Should print A, arg, original
25
27
-function changeF(o) {
28
- o.f = () => console.log("new");
29
-}
30
-
26
function Component() {
32
- const $ = useMemoCache(2);
27
+ const $ = useMemoCache(3);
28
let t0;
29
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
35
- t0 = () => console.log("original");
30
+ t0 = (o) => {
31
+ o.f = () => console.log("new");
32
+ };
33
$[0] = t0;
34
} else {
35
t0 = $[0];
36
}
40
- let x;
37
+ const changeF = t0;
38
+ let t1;
39
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
42
- x = { f: t0 };
40
+ t1 = () => console.log("original");
41
+ $[1] = t1;
42
+ } else {
43
+ t1 = $[1];
44
+ }
45
+ let x;
46
+ if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
47
+ x = { f: t1 };
48
49
console.log("A");
50
changeF(x);
51
console.log("arg");
52
x.f(1);
48
- $[1] = x;
53
+ $[2] = x;
54
} else {
50
- x = $[1];
55
+ x = $[2];
56
}
57
return x;
58
}
compiler/forget/src/__tests__/fixtures/compiler/_bug.property-call-evaluation-order.js
+4
-5
@@ -1,11 +1,10 @@
1
// Should print A, arg, original
2
3
-function changeF(o) {
4
- o.f = () => console.log("new");
5
-}
6
-
3
function Component() {
8
- let x = {
4
+ const changeF = (o) => {
5
+ o.f = () => console.log("new");
6
+ };
7
+ const x = {
8
f: () => console.log("original"),
9
};
10
compiler/forget/src/__tests__/fixtures/compiler/function-expression-with-store-to-parameter.expect.md
new
+48
@@ -0,0 +1,48 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const mutate = (object, key, value) => {
7
+ object.updated = true;
8
+ object[key] = value;
9
+ };
10
+ const x = makeObject(props);
11
+ mutate(x);
12
+ return x;
13
+}
14
+
15
+```
16
+
17
+## Code
18
+
19
+```javascript
20
+import { unstable_useMemoCache as useMemoCache } from "react";
21
+function Component(props) {
22
+ const $ = useMemoCache(3);
23
+ let t0;
24
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
25
+ t0 = (object, key, value) => {
26
+ object.updated = true;
27
+ object[key] = value;
28
+ };
29
+ $[0] = t0;
30
+ } else {
31
+ t0 = $[0];
32
+ }
33
+ const mutate = t0;
34
+ const c_1 = $[1] !== props;
35
+ let x;
36
+ if (c_1) {
37
+ x = makeObject(props);
38
+ mutate(x);
39
+ $[1] = props;
40
+ $[2] = x;
41
+ } else {
42
+ x = $[2];
43
+ }
44
+ return x;
45
+}
46
+
47
+```
48
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/function-expression-with-store-to-parameter.js
new
+9
@@ -0,0 +1,9 @@
1
+function Component(props) {
2
+ const mutate = (object, key, value) => {
3
+ object.updated = true;
4
+ object[key] = value;
5
+ };
6
+ const x = makeObject(props);
7
+ mutate(x);
8
+ return x;
9
+}
compiler/forget/src/__tests__/fixtures/compiler/object-computed-access-assignment.expect.md
+6
-4
@@ -3,8 +3,9 @@
3
4
```javascript
5
function foo(a, b, c) {
6
- a[b] = c[b];
7
- a[1 + 2] = c[b * 4];
6
+ const x = { ...a };
7
+ x[b] = c[b];
8
+ x[1 + 2] = c[b * 4];
9
}
10
11
```
@@ -13,8 +14,9 @@ function foo(a, b, c) {
14
15
```javascript
16
function foo(a, b, c) {
16
- a[b] = c[b];
17
- a[3] = c[b * 4];
17
+ const x = { ...a };
18
+ x[b] = c[b];
19
+ x[3] = c[b * 4];
20
}
21
22
```
compiler/forget/src/__tests__/fixtures/compiler/object-computed-access-assignment.js
+3
-2
@@ -1,4 +1,5 @@
1
function foo(a, b, c) {
2
- a[b] = c[b];
3
- a[1 + 2] = c[b * 4];
2
+ const x = { ...a };
3
+ x[b] = c[b];
4
+ x[1 + 2] = c[b * 4];
5
}
compiler/forget/src/__tests__/fixtures/compiler/object-properties.expect.md
+2
-2
@@ -4,7 +4,7 @@
4
```javascript
5
function foo(a, b, c) {
6
const x = a.x;
7
- const y = b.c.d;
7
+ const y = { ...b.c.d };
8
y.z = c.d.e;
9
foo(a.b.c);
10
[a.b.c];
@@ -16,7 +16,7 @@ function foo(a, b, c) {
16
17
```javascript
18
function foo(a, b, c) {
19
- const y = b.c.d;
19
+ const y = { ...b.c.d };
20
y.z = c.d.e;
21
foo(a.b.c);
22
}
compiler/forget/src/__tests__/fixtures/compiler/object-properties.js
+1
-1
@@ -1,6 +1,6 @@
1
function foo(a, b, c) {
2
const x = a.x;
3
- const y = b.c.d;
3
+ const y = { ...b.c.d };
4
y.z = c.d.e;
5
foo(a.b.c);
6
[a.b.c];