Fix Array#at and similar cases to capture if receiver is mutable
The previous PR helped me realize we weren't handling Array#at correctly. If the receiver is a mutable value its effect should be Capture and the lvalue effect needs to be Store. This PR updates the definition for Array#at to make the receiver Capture, and then updates inference to automatically set the lvalue effect to Store if _any_ argument (or the receiver) was Capture.
Joe Savona committed
Nov 29, 2023 at 12:05 UTC
7da906d64802bd33f8c004df5f67504b7635e972
4 files changed
+29
-20
compiler/packages/babel-plugin-react-forget/src/HIR/ObjectShape.ts
+1
-1
@@ -203,7 +203,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [
203
positionalParams: [Effect.Read],
204
restParam: null,
205
returnType: { kind: "Poly" },
206
- calleeEffect: Effect.Read,
206
+ calleeEffect: Effect.Capture,
207
returnValueKind: ValueKind.Mutable,
208
}),
209
],
compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts
+12
-2
@@ -809,6 +809,7 @@ function inferBlock(
809
signature !== null ? getFunctionEffects(instrValue, signature) : null;
810
const returnValueKind =
811
signature !== null ? signature.returnValueKind : ValueKind.Mutable;
812
+ let hasCaptureArgument = false;
813
for (let i = 0; i < instrValue.args.length; i++) {
814
const arg = instrValue.args[i];
815
const place = arg.kind === "Identifier" ? arg : arg.place;
@@ -817,16 +818,20 @@ function inferBlock(
818
} else {
819
state.reference(place, Effect.ConditionallyMutate);
820
}
821
+ hasCaptureArgument ||= place.effect === Effect.Capture;
822
}
823
if (signature !== null) {
824
state.reference(instrValue.callee, signature.calleeEffect);
825
} else {
826
state.reference(instrValue.callee, Effect.ConditionallyMutate);
827
}
828
+ hasCaptureArgument ||= instrValue.callee.effect === Effect.Capture;
829
830
state.initialize(instrValue, returnValueKind);
831
state.define(instr.lvalue, instrValue);
829
- instr.lvalue.effect = Effect.ConditionallyMutate;
832
+ instr.lvalue.effect = hasCaptureArgument
833
+ ? Effect.Store
834
+ : Effect.ConditionallyMutate;
835
continue;
836
}
837
case "MethodCall": {
@@ -871,6 +876,7 @@ function inferBlock(
876
signature !== null ? getFunctionEffects(instrValue, signature) : null;
877
const returnValueKind =
878
signature !== null ? signature.returnValueKind : ValueKind.Mutable;
879
+ let hasCaptureArgument = false;
880
for (let i = 0; i < instrValue.args.length; i++) {
881
const arg = instrValue.args[i];
882
const place = arg.kind === "Identifier" ? arg : arg.place;
@@ -883,16 +889,20 @@ function inferBlock(
889
} else {
890
state.reference(place, Effect.ConditionallyMutate);
891
}
892
+ hasCaptureArgument ||= place.effect === Effect.Capture;
893
}
894
if (signature !== null) {
895
state.reference(instrValue.receiver, signature.calleeEffect);
896
} else {
897
state.reference(instrValue.receiver, Effect.ConditionallyMutate);
898
}
899
+ hasCaptureArgument ||= instrValue.receiver.effect === Effect.Capture;
900
901
state.initialize(instrValue, returnValueKind);
902
state.define(instr.lvalue, instrValue);
895
- instr.lvalue.effect = Effect.ConditionallyMutate;
903
+ instr.lvalue.effect = hasCaptureArgument
904
+ ? Effect.Store
905
+ : Effect.ConditionallyMutate;
906
continue;
907
}
908
case "PropertyStore": {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/array-at-mutate-after-capture.expect.md
+10
-10
@@ -21,18 +21,18 @@ function Component(props) {
21
import { unstable_useMemoCache as useMemoCache } from "react"; // x's mutable range should extend to `mutate(y)`
22
23
function Component(props) {
24
- const $ = useMemoCache(1);
25
- let t0;
26
- if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
27
- t0 = [42, {}];
28
- $[0] = t0;
24
+ const $ = useMemoCache(2);
25
+ let x;
26
+ if ($[0] !== props.b) {
27
+ x = [42, {}];
28
+ const idx = foo(props.b);
29
+ const y = x.at(idx);
30
+ mutate(y);
31
+ $[0] = props.b;
32
+ $[1] = x;
33
} else {
30
- t0 = $[0];
34
+ x = $[1];
35
}
32
- const x = t0;
33
- const idx = foo(props.b);
34
- const y = x.at(idx);
35
- mutate(y);
36
return x;
37
}
38
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/assignment-variations-complex-lvalue-array.expect.md
+6
-7
@@ -23,16 +23,15 @@ export const FIXTURE_ENTRYPOINT = {
23
import { unstable_useMemoCache as useMemoCache } from "react";
24
function foo() {
25
const $ = useMemoCache(1);
26
- let t0;
26
+ let a;
27
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
28
- t0 = [[1]];
29
- $[0] = t0;
28
+ a = [[1]];
29
+ const first = a.at(0);
30
+ first.set(0, 2);
31
+ $[0] = a;
32
} else {
31
- t0 = $[0];
33
+ a = $[0];
34
}
33
- const a = t0;
34
- const first = a.at(0);
35
- first.set(0, 2);
35
return a;
36
}
37