Rename Effect.Mutate -> ConditionallyMutate
We currently use `Effect.Mutate` both for places that _may_ mutate (ie untyped function calls) and for places that have known mutation (typed function calls, or operations like `delete x.y`). We then use a separate mechanism to decide whether to reject the input, with some call paths checking the effect and others not. This stack refactors this logic in InferReferenceEffects per our discussion, so that `Effect.ConditionallyMutate` is for "may or may not mutate" either because we're not 100% sure (untyped function) or because the mutation depends on the operand (ie, a callback arg that will be invoked and thus will mutate if the lambda is mutable, not mutate if the lambda is immutable). Later diffs add back `Effect.Mutate` as "definitely 100% mutating".
Joe Savona committed
May 25, 2023 at 13:41 UTC
40cf400f7215b344da3658104cfc3b9e3ce1dd66
7 files changed
+55
-42
compiler/forget/src/HIR/HIR.ts
+11
-3
@@ -847,8 +847,16 @@ export enum Effect {
847
Read = "read",
848
// This reference reads and stores the value
849
Capture = "capture",
850
- // This reference may write to (mutate) the value
851
- Mutate = "mutate",
850
+ // This reference *may* write to (mutate) the value. This covers two similar cases:
851
+ // - The compiler is being conservative and assuming that a value *may* be mutated
852
+ // - The effect is polymorphic: mutable values may be mutated, non-mutable values
853
+ // will not be mutated.
854
+ // In both cases, we conservatively assume that mutable values will be mutated.
855
+ // But we do not error if the value is known to be immutable.
856
+ ConditionallyMutate = "mutate?",
857
+ // This reference *does* write to (mutate) the value. It is an error (invalid input)
858
+ // if an immutable value flows into a location with this effect.
859
+ // DefiniitelyMutate = "mutate",
860
// This reference may alias to (mutate) the value
861
Store = "store",
862
}
@@ -856,7 +864,7 @@ export enum Effect {
864
export function isMutableEffect(effect: Effect): boolean {
865
switch (effect) {
866
case Effect.Capture:
859
- case Effect.Mutate:
867
+ case Effect.ConditionallyMutate:
868
case Effect.Store: {
869
return true;
870
}
compiler/forget/src/HIR/MergeConsecutiveBlocks.ts
+1
-1
@@ -63,7 +63,7 @@ export function mergeConsecutiveBlocks(fn: HIRFunction): void {
63
lvalue: {
64
kind: "Identifier",
65
identifier: phi.id,
66
- effect: Effect.Mutate,
66
+ effect: Effect.ConditionallyMutate,
67
loc: GeneratedSource,
68
},
69
value: {
compiler/forget/src/HIR/ObjectShape.ts
+1
-1
@@ -250,7 +250,7 @@ addObject(BUILTIN_SHAPES, BuiltInRefValueId, []);
250
251
export const DefaultMutatingHook = addHook(BUILTIN_SHAPES, [], {
252
positionalParams: [],
253
- restParam: Effect.Mutate,
253
+ restParam: Effect.ConditionallyMutate,
254
returnType: { kind: "Poly" },
255
calleeEffect: Effect.Read,
256
hookKind: "Custom",
compiler/forget/src/Inference/InferMutableLifetimes.ts
+1
-1
@@ -86,7 +86,7 @@ function inferPlace(
86
infer(place, instrId);
87
}
88
return;
89
- case Effect.Mutate: {
89
+ case Effect.ConditionallyMutate: {
90
infer(place, instrId);
91
return;
92
}
compiler/forget/src/Inference/InferReferenceEffects.ts
+37
-32
@@ -293,7 +293,10 @@ class InferenceState {
293
place.loc
294
);
295
}
296
- place.effect = effectKind === Effect.Mutate ? Effect.Mutate : Effect.Read;
296
+ place.effect =
297
+ effectKind === Effect.ConditionallyMutate
298
+ ? Effect.ConditionallyMutate
299
+ : Effect.Read;
300
return;
301
}
302
let valueKind: ValueKind | null = this.kind(place);
@@ -313,12 +316,12 @@ class InferenceState {
316
}
317
break;
318
}
316
- case Effect.Mutate: {
319
+ case Effect.ConditionallyMutate: {
320
if (
321
valueKind === ValueKind.Mutable ||
322
valueKind === ValueKind.Context
323
) {
321
- effect = Effect.Mutate;
324
+ effect = Effect.ConditionallyMutate;
325
} else {
326
if (shouldError) {
327
CompilerError.invalidInput(
@@ -354,7 +357,9 @@ class InferenceState {
357
// valueKind === ValueKind.Mutable,
358
// `expected valueKind to be 'Mutable' but found to be '${valueKind}'`
359
// );
357
- effect = isObjectType(place.identifier) ? Effect.Store : Effect.Mutate;
360
+ effect = isObjectType(place.identifier)
361
+ ? Effect.Store
362
+ : Effect.ConditionallyMutate;
363
break;
364
}
365
case Effect.Capture: {
@@ -616,7 +621,7 @@ function inferBlock(
621
for (const instr of block.instructions) {
622
const instrValue = instr.value;
623
let effectKind: Effect | null = null;
619
- let lvalueEffect = Effect.Mutate;
624
+ let lvalueEffect = Effect.ConditionallyMutate;
625
let valueKind: ValueKind;
626
switch (instrValue.kind) {
627
case "BinaryExpression": {
@@ -634,7 +639,7 @@ function inferBlock(
639
}
640
case "NewExpression": {
641
valueKind = ValueKind.Mutable;
637
- effectKind = Effect.Mutate;
642
+ effectKind = Effect.ConditionallyMutate;
643
break;
644
}
645
case "ObjectExpression": {
@@ -669,7 +674,7 @@ function inferBlock(
674
}
675
case "TaggedTemplateExpression": {
676
valueKind = ValueKind.Mutable;
672
- effectKind = Effect.Mutate;
677
+ effectKind = Effect.ConditionallyMutate;
678
break;
679
}
680
case "TemplateLiteral": {
@@ -682,7 +687,7 @@ function inferBlock(
687
case "RegExpLiteral": {
688
// RegExp instances are mutable objects
689
valueKind = ValueKind.Mutable;
685
- effectKind = Effect.Mutate;
690
+ effectKind = Effect.ConditionallyMutate;
691
break;
692
}
693
case "Debugger":
@@ -755,7 +760,7 @@ function inferBlock(
760
state.referenceAndCheckError(place, effects[i]);
761
}
762
} else {
758
- state.reference(place, Effect.Mutate);
763
+ state.reference(place, Effect.ConditionallyMutate);
764
}
765
}
766
if (signature !== null) {
@@ -764,12 +769,12 @@ function inferBlock(
769
signature.calleeEffect
770
);
771
} else {
767
- state.reference(instrValue.callee, Effect.Mutate);
772
+ state.reference(instrValue.callee, Effect.ConditionallyMutate);
773
}
774
775
state.initialize(instrValue, returnValueKind);
776
state.define(instr.lvalue, instrValue);
772
- instr.lvalue.effect = Effect.Mutate;
777
+ instr.lvalue.effect = Effect.ConditionallyMutate;
778
continue;
779
}
780
case "MethodCall": {
@@ -794,7 +799,7 @@ function inferBlock(
799
// mutating effects
800
state.referenceAndCheckError(place, effects[i]);
801
} else {
797
- state.reference(place, Effect.Mutate);
802
+ state.reference(place, Effect.ConditionallyMutate);
803
}
804
}
805
if (signature !== null) {
@@ -803,18 +808,18 @@ function inferBlock(
808
signature.calleeEffect
809
);
810
} else {
806
- state.reference(instrValue.receiver, Effect.Mutate);
811
+ state.reference(instrValue.receiver, Effect.ConditionallyMutate);
812
}
813
814
state.initialize(instrValue, ValueKind.Mutable);
815
state.define(instr.lvalue, instrValue);
811
- instr.lvalue.effect = Effect.Mutate;
816
+ instr.lvalue.effect = Effect.ConditionallyMutate;
817
continue;
818
}
819
case "PropertyStore": {
820
const effect =
821
state.kind(instrValue.object) === ValueKind.Context
817
- ? Effect.Mutate
822
+ ? Effect.ConditionallyMutate
823
: Effect.Capture;
824
state.reference(instrValue.value, effect);
825
state.reference(instrValue.object, Effect.Store);
@@ -827,13 +832,13 @@ function inferBlock(
832
case "PropertyDelete": {
833
// `delete` returns a boolean (immutable) and modifies the object
834
valueKind = ValueKind.Immutable;
830
- effectKind = Effect.Mutate;
835
+ effectKind = Effect.ConditionallyMutate;
836
break;
837
}
838
case "PropertyLoad": {
839
state.reference(instrValue.object, Effect.Read);
840
const lvalue = instr.lvalue;
836
- lvalue.effect = Effect.Mutate;
841
+ lvalue.effect = Effect.ConditionallyMutate;
842
state.initialize(instrValue, state.kind(instrValue.object));
843
state.define(lvalue, instrValue);
844
continue;
@@ -841,7 +846,7 @@ function inferBlock(
846
case "ComputedStore": {
847
const effect =
848
state.kind(instrValue.object) === ValueKind.Context
844
- ? Effect.Mutate
849
+ ? Effect.ConditionallyMutate
850
: Effect.Capture;
851
state.reference(instrValue.value, effect);
852
state.reference(instrValue.property, Effect.Capture);
@@ -853,17 +858,17 @@ function inferBlock(
858
continue;
859
}
860
case "ComputedDelete": {
856
- state.reference(instrValue.object, Effect.Mutate);
861
+ state.reference(instrValue.object, Effect.ConditionallyMutate);
862
state.reference(instrValue.property, Effect.Read);
863
state.initialize(instrValue, ValueKind.Immutable);
859
- state.reference(instr.lvalue, Effect.Mutate);
864
+ state.reference(instr.lvalue, Effect.ConditionallyMutate);
865
continue;
866
}
867
case "ComputedLoad": {
868
state.reference(instrValue.object, Effect.Read);
869
state.reference(instrValue.property, Effect.Read);
870
const lvalue = instr.lvalue;
866
- lvalue.effect = Effect.Mutate;
871
+ lvalue.effect = Effect.ConditionallyMutate;
872
state.initialize(instrValue, state.kind(instrValue.object));
873
state.define(lvalue, instrValue);
874
continue;
@@ -873,9 +878,9 @@ function inferBlock(
878
// Awaiting a value causes it to change state (go from unresolved to resolved or error)
879
// It also means that any side-effects which would occur as part of the promise evaluation
880
// will occur.
876
- state.reference(instrValue.value, Effect.Mutate);
881
+ state.reference(instrValue.value, Effect.ConditionallyMutate);
882
const lvalue = instr.lvalue;
878
- lvalue.effect = Effect.Mutate;
883
+ lvalue.effect = Effect.ConditionallyMutate;
884
state.alias(lvalue, instrValue.value);
885
continue;
886
}
@@ -890,7 +895,7 @@ function inferBlock(
895
state.initialize(instrValue, state.kind(instrValue.value));
896
state.reference(instrValue.value, Effect.Read);
897
const lvalue = instr.lvalue;
893
- lvalue.effect = Effect.Mutate;
898
+ lvalue.effect = Effect.ConditionallyMutate;
899
state.alias(lvalue, instrValue.value);
900
continue;
901
}
@@ -898,10 +903,10 @@ function inferBlock(
903
const lvalue = instr.lvalue;
904
const effect =
905
state.isDefined(lvalue) && state.kind(lvalue) === ValueKind.Context
901
- ? Effect.Mutate
906
+ ? Effect.ConditionallyMutate
907
: Effect.Capture;
908
state.reference(instrValue.place, effect);
904
- lvalue.effect = Effect.Mutate;
909
+ lvalue.effect = Effect.ConditionallyMutate;
910
// direct aliasing: `a = b`;
911
state.alias(lvalue, instrValue.place);
912
continue;
@@ -909,7 +914,7 @@ function inferBlock(
914
case "LoadContext": {
915
state.reference(instrValue.place, Effect.Capture);
916
const lvalue = instr.lvalue;
912
- lvalue.effect = Effect.Mutate;
917
+ lvalue.effect = Effect.ConditionallyMutate;
918
const valueKind = state.kind(instrValue.place);
919
invariant(
920
valueKind === ValueKind.Mutable || valueKind === ValueKind.Context,
@@ -938,7 +943,7 @@ function inferBlock(
943
const effect =
944
state.isDefined(instrValue.lvalue.place) &&
945
state.kind(instrValue.lvalue.place) === ValueKind.Context
941
- ? Effect.Mutate
946
+ ? Effect.ConditionallyMutate
947
: Effect.Capture;
948
state.reference(instrValue.value, effect);
949
@@ -950,8 +955,8 @@ function inferBlock(
955
continue;
956
}
957
case "StoreContext": {
953
- state.reference(instrValue.value, Effect.Mutate);
954
- state.reference(instrValue.lvalue.place, Effect.Mutate);
958
+ state.reference(instrValue.value, Effect.ConditionallyMutate);
959
+ state.reference(instrValue.lvalue.place, Effect.ConditionallyMutate);
960
961
const lvalue = instr.lvalue;
962
state.alias(lvalue, instrValue.value);
@@ -965,7 +970,7 @@ function inferBlock(
970
state.isDefined(place) &&
971
state.kind(place) === ValueKind.Context
972
) {
968
- effect = Effect.Mutate;
973
+ effect = Effect.ConditionallyMutate;
974
break;
975
}
976
}
@@ -1012,7 +1017,7 @@ function inferBlock(
1017
state.isDefined(operand) &&
1018
state.kind(operand) === ValueKind.Context
1019
) {
1015
- effect = Effect.Mutate;
1020
+ effect = Effect.ConditionallyMutate;
1021
} else {
1022
effect = Effect.Freeze;
1023
}
compiler/forget/src/ReactiveScopes/InferReactiveIdentifiers.ts
+1
-1
@@ -91,7 +91,7 @@ class Visitor extends ReactiveFunctionVisitor<State> {
91
switch (operand.effect) {
92
case Effect.Capture:
93
case Effect.Store:
94
- case Effect.Mutate: {
94
+ case Effect.ConditionallyMutate: {
95
const resolvedId: IdentifierId =
96
state.temporaries.get(operand.identifier.id) ??
97
operand.identifier.id;
compiler/forget/src/SSA/LeaveSSA.ts
+3
-3
@@ -390,7 +390,7 @@ export function leaveSSA(fn: HIRFunction): void {
390
};
391
block.instructions.push({
392
id: block.terminal.id,
393
- lvalue: { ...initValue, effect: Effect.Mutate },
393
+ lvalue: { ...initValue, effect: Effect.ConditionallyMutate },
394
value: {
395
kind: "Primitive",
396
// TODO: consider leaving the variable uninitialized rather than explicitly undefined.
@@ -411,7 +411,7 @@ export function leaveSSA(fn: HIRFunction): void {
411
place: {
412
kind: "Identifier",
413
identifier: phi.id,
414
- effect: Effect.Mutate,
414
+ effect: Effect.ConditionallyMutate,
415
loc: GeneratedSource,
416
},
417
kind: InstructionKind.Let,
@@ -433,7 +433,7 @@ export function leaveSSA(fn: HIRFunction): void {
433
scope: null,
434
type: phi.id.type,
435
},
436
- effect: Effect.Mutate,
436
+ effect: Effect.ConditionallyMutate,
437
loc: GeneratedSource,
438
},
439
value: {