Skip to content

Commit cc3ca6b

Browse files
committed
fix: prevent catch parameter aliases from breaking scope boundaries in noBackwardRangeExtension heuristic
Track catch handler bindings in AliasingState so the backward-alias walk in mutate() stops at catch parameters. Without this, catch parameters aliased to call results in try blocks incorrectly trigger the effectiveSkipAliases heuristic, preventing backward range extension and causing 'Expected a break target' crashes when try-catch blocks get split across reactive scopes. Fixes 4 compiler crashes (try-catch-try-value-modified-in-catch and variants) and 1 snapshot regression (try-catch-alias-try-values). Updates 8 snapshot expectations for tests where the noBackwardRangeExtension change produces different but functionally correct memoization output. Closes #35902
1 parent f4cfb20 commit cc3ca6b

9 files changed

Lines changed: 191 additions & 89 deletions

compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,9 @@ export function inferMutationAliasingRanges(
231231
for (const effect of block.terminal.effects) {
232232
if (effect.kind === 'Alias') {
233233
state.assign(index++, effect.from, effect.into);
234+
if (block.terminal.kind === 'maybe-throw') {
235+
state.catchParams.add(effect.into.identifier);
236+
}
234237
} else {
235238
CompilerError.invariant(effect.kind === 'Freeze', {
236239
reason: `Unexpected '${effect.kind}' effect for MaybeThrow terminal`,
@@ -600,6 +603,13 @@ type Node = {
600603
};
601604
class AliasingState {
602605
nodes: Map<Identifier, Node> = new Map();
606+
/*
607+
* Identifiers that are catch handler bindings. These are tracked
608+
* so the backward-alias heuristic in mutate() can stop walking
609+
* through catch parameter aliases, which would otherwise incorrectly
610+
* match the "function call result" pattern and skip range extension.
611+
*/
612+
catchParams: Set<Identifier> = new Set();
603613

604614
create(place: Place, value: Node['value']): void {
605615
this.nodes.set(place.identifier, {
@@ -740,8 +750,10 @@ class AliasingState {
740750
* Apply results are identifiable by having maybeAliases, since the
741751
* default Apply handler records MaybeAlias from each argument to
742752
* the result. Only walk through alias edges (not maybeAliases or
743-
* captures) to avoid matching catch parameters and similar patterns
744-
* where range extension is required for correctness.
753+
* captures). Additionally, stop walking when a catch parameter is
754+
* reached, since catch parameters are aliased to call results via
755+
* terminal Alias effects and would otherwise incorrectly trigger
756+
* this heuristic.
745757
*/
746758
const visited = new Set<Identifier>();
747759
const aliasQueue: Array<Identifier> = [start];
@@ -755,6 +767,18 @@ class AliasingState {
755767
if (node == null) {
756768
continue;
757769
}
770+
/*
771+
* Stop walking through catch parameter aliases. Catch bindings
772+
* are aliased to call results in the try block, and those call
773+
* results have maybeAliases from the Apply handler. Without
774+
* this check, the heuristic would incorrectly treat catch
775+
* parameters as independently-memoizable function call results,
776+
* preventing backward range extension and causing scope boundary
777+
* issues (e.g. "Expected a break target" in try-catch blocks).
778+
*/
779+
if (this.catchParams.has(id)) {
780+
continue;
781+
}
758782
if (node.maybeAliases.size > 0) {
759783
effectiveSkipAliases = true;
760784
break;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/align-scopes-within-nested-valueblock-in-array.expect.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ import { Stringify, identity, makeArray, mutate } from "shared-runtime";
5151
function Foo(t0) {
5252
const $ = _c(3);
5353
const { cond1, cond2 } = t0;
54+
const arr = makeArray({ a: 2 }, 2, []);
5455
let t1;
5556
if ($[0] !== cond1 || $[1] !== cond2) {
56-
const arr = makeArray({ a: 2 }, 2, []);
5757
t1 = cond1 ? (
5858
<>
5959
<div>{identity("foo")}</div>

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/global-types/set-add-mutate.expect.md

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,24 +33,32 @@ import { c as _c } from "react/compiler-runtime";
3333
import { makeArray } from "shared-runtime";
3434

3535
function useHook(t0) {
36-
const $ = _c(5);
36+
const $ = _c(7);
3737
const { el1, el2 } = t0;
3838
let s;
3939
if ($[0] !== el1 || $[1] !== el2) {
4040
s = new Set();
41-
const arr = makeArray(el1);
42-
s.add(arr);
43-
44-
arr.push(el2);
4541
let t1;
46-
if ($[3] !== el2) {
47-
t1 = makeArray(el2);
48-
$[3] = el2;
42+
if ($[3] !== el1) {
43+
t1 = makeArray(el1);
44+
$[3] = el1;
4945
$[4] = t1;
5046
} else {
5147
t1 = $[4];
5248
}
53-
s.add(t1);
49+
const arr = t1;
50+
s.add(arr);
51+
52+
arr.push(el2);
53+
let t2;
54+
if ($[5] !== el2) {
55+
t2 = makeArray(el2);
56+
$[5] = el2;
57+
$[6] = t2;
58+
} else {
59+
t2 = $[6];
60+
}
61+
s.add(t2);
5462
$[0] = el1;
5563
$[1] = el2;
5664
$[2] = s;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hooks-with-prefix.expect.md

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ const React$useMemo = React.useMemo;
4646
const Internal$Reassigned$useHook = useHook;
4747

4848
function Component() {
49-
const $ = _c(7);
49+
const $ = _c(9);
5050
const [state] = React$useState(0);
5151
const object = Internal$Reassigned$useHook();
5252
let t0;
@@ -60,28 +60,36 @@ function Component() {
6060
const json = t0;
6161
let t1;
6262
if ($[2] !== state) {
63-
const doubledArray = makeArray(state);
64-
t1 = doubledArray.join("");
63+
t1 = makeArray(state);
6564
$[2] = state;
6665
$[3] = t1;
6766
} else {
6867
t1 = $[3];
6968
}
69+
const doubledArray = t1;
7070
let t2;
71-
if ($[4] !== json || $[5] !== t1) {
72-
t2 = (
71+
if ($[4] !== doubledArray) {
72+
t2 = doubledArray.join("");
73+
$[4] = doubledArray;
74+
$[5] = t2;
75+
} else {
76+
t2 = $[5];
77+
}
78+
let t3;
79+
if ($[6] !== json || $[7] !== t2) {
80+
t3 = (
7381
<div>
74-
{t1}
82+
{t2}
7583
{json}
7684
</div>
7785
);
78-
$[4] = json;
79-
$[5] = t1;
80-
$[6] = t2;
86+
$[6] = json;
87+
$[7] = t2;
88+
$[8] = t3;
8189
} else {
82-
t2 = $[6];
90+
t3 = $[8];
8391
}
84-
return t2;
92+
return t3;
8593
}
8694

8795
export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/set-add-mutate.expect.md

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,32 @@ function useHook({el1, el2}) {
2121
```javascript
2222
import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel
2323
function useHook(t0) {
24-
const $ = _c(5);
24+
const $ = _c(7);
2525
const { el1, el2 } = t0;
2626
let s;
2727
if ($[0] !== el1 || $[1] !== el2) {
2828
s = new Set();
29-
const arr = makeArray(el1);
30-
s.add(arr);
31-
32-
arr.push(el2);
3329
let t1;
34-
if ($[3] !== el2) {
35-
t1 = makeArray(el2);
36-
$[3] = el2;
30+
if ($[3] !== el1) {
31+
t1 = makeArray(el1);
32+
$[3] = el1;
3733
$[4] = t1;
3834
} else {
3935
t1 = $[4];
4036
}
41-
s.add(t1);
37+
const arr = t1;
38+
s.add(arr);
39+
40+
arr.push(el2);
41+
let t2;
42+
if ($[5] !== el2) {
43+
t2 = makeArray(el2);
44+
$[5] = el2;
45+
$[6] = t2;
46+
} else {
47+
t2 = $[6];
48+
}
49+
s.add(t2);
4250
$[0] = el1;
4351
$[1] = el2;
4452
$[2] = s;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-independently-memoized-property-load-for-method-call.expect.md

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -54,40 +54,54 @@ export const FIXTURE_ENTRYPOINT = {
5454
```javascript
5555
import { c as _c } from "react/compiler-runtime";
5656
function Component(t0) {
57-
const $ = _c(8);
57+
const $ = _c(12);
5858
const { label, highlightedItem } = t0;
5959
const serverTime = useServerTime();
6060
let t1;
61+
if ($[0] !== highlightedItem) {
62+
t1 = new Highlight(highlightedItem);
63+
$[0] = highlightedItem;
64+
$[1] = t1;
65+
} else {
66+
t1 = $[1];
67+
}
68+
let t2;
6169
let timestampLabel;
62-
if ($[0] !== highlightedItem || $[1] !== label || $[2] !== serverTime) {
63-
const highlight = new Highlight(highlightedItem);
70+
if ($[2] !== label || $[3] !== serverTime || $[4] !== t1) {
71+
const highlight = t1;
6472
const time = serverTime.get();
6573
timestampLabel = time / 1000 || label;
66-
t1 = highlight.render();
67-
$[0] = highlightedItem;
68-
$[1] = label;
69-
$[2] = serverTime;
70-
$[3] = t1;
71-
$[4] = timestampLabel;
74+
if ($[7] !== highlight) {
75+
t2 = highlight.render();
76+
$[7] = highlight;
77+
$[8] = t2;
78+
} else {
79+
t2 = $[8];
80+
}
81+
$[2] = label;
82+
$[3] = serverTime;
83+
$[4] = t1;
84+
$[5] = t2;
85+
$[6] = timestampLabel;
7286
} else {
73-
t1 = $[3];
74-
timestampLabel = $[4];
87+
t2 = $[5];
88+
timestampLabel = $[6];
7589
}
76-
let t2;
77-
if ($[5] !== t1 || $[6] !== timestampLabel) {
78-
t2 = (
90+
let t3;
91+
if ($[9] !== t2 || $[10] !== timestampLabel) {
92+
t3 = (
7993
<>
80-
{t1}
94+
{t2}
8195
{timestampLabel}
8296
</>
8397
);
84-
$[5] = t1;
85-
$[6] = timestampLabel;
86-
$[7] = t2;
98+
$[9] = t2;
99+
$[10] = timestampLabel;
100+
$[11] = t3;
87101
} else {
88-
t2 = $[7];
102+
t3 = $[11];
89103
}
90-
return t2;
104+
return t3;
91105
}
92106

93107
function useServerTime() {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-unmerged-fbt-call-merge-overlapping-reactive-scopes.expect.md

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,27 +36,35 @@ import fbt from "fbt";
3636
import { Stringify } from "shared-runtime";
3737

3838
function Component(props) {
39-
const $ = _c(3);
39+
const $ = _c(5);
4040
let t0;
41-
if ($[0] !== props.cond || $[1] !== props.value.length) {
42-
const label = fbt._(
41+
if ($[0] !== props.value.length) {
42+
t0 = fbt._(
4343
{ "*": "{number} bars", _1: "1 bar" },
4444
[fbt._plural(props.value.length, "number")],
4545
{ hk: "4mUen7" },
4646
);
47-
t0 = props.cond ? (
47+
$[0] = props.value.length;
48+
$[1] = t0;
49+
} else {
50+
t0 = $[1];
51+
}
52+
const label = t0;
53+
let t1;
54+
if ($[2] !== label || $[3] !== props.cond) {
55+
t1 = props.cond ? (
4856
<Stringify
4957
description={fbt._("Text here", null, { hk: "21YpZs" })}
5058
label={label.toString()}
5159
/>
5260
) : null;
53-
$[0] = props.cond;
54-
$[1] = props.value.length;
55-
$[2] = t0;
61+
$[2] = label;
62+
$[3] = props.cond;
63+
$[4] = t1;
5664
} else {
57-
t0 = $[2];
65+
t1 = $[4];
5866
}
59-
return t0;
67+
return t1;
6068
}
6169

6270
export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-global-load-cached.expect.md

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,31 +35,47 @@ import { makeArray } from "shared-runtime";
3535
* and avoid adding them to `temporariesUsedOutsideDefiningScope`.
3636
*/
3737
function Component(t0) {
38-
const $ = _c(6);
38+
const $ = _c(12);
3939
const { num } = t0;
40-
let T0;
4140
let t1;
4241
if ($[0] !== num) {
43-
const arr = makeArray(num);
44-
T0 = Stringify;
45-
t1 = arr.push(num);
42+
t1 = makeArray(num);
4643
$[0] = num;
47-
$[1] = T0;
48-
$[2] = t1;
44+
$[1] = t1;
4945
} else {
50-
T0 = $[1];
51-
t1 = $[2];
46+
t1 = $[1];
5247
}
48+
let T0;
5349
let t2;
54-
if ($[3] !== T0 || $[4] !== t1) {
55-
t2 = <T0 value={t1} />;
56-
$[3] = T0;
57-
$[4] = t1;
50+
if ($[2] !== num || $[3] !== t1) {
51+
const arr = t1;
52+
T0 = Stringify;
53+
if ($[6] !== arr || $[7] !== num) {
54+
t2 = arr.push(num);
55+
$[6] = arr;
56+
$[7] = num;
57+
$[8] = t2;
58+
} else {
59+
t2 = $[8];
60+
}
61+
$[2] = num;
62+
$[3] = t1;
63+
$[4] = T0;
5864
$[5] = t2;
5965
} else {
66+
T0 = $[4];
6067
t2 = $[5];
6168
}
62-
return t2;
69+
let t3;
70+
if ($[9] !== T0 || $[10] !== t2) {
71+
t3 = <T0 value={t2} />;
72+
$[9] = T0;
73+
$[10] = t2;
74+
$[11] = t3;
75+
} else {
76+
t3 = $[11];
77+
}
78+
return t3;
6379
}
6480

6581
export const FIXTURE_ENTRYPOINT = {

0 commit comments

Comments
 (0)