Skip to content

Commit 99d6d02

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 9f2b4d7 commit 99d6d02

8 files changed

Lines changed: 172 additions & 78 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
@@ -234,6 +234,9 @@ export function inferMutationAliasingRanges(
234234
for (const effect of block.terminal.effects) {
235235
if (effect.kind === 'Alias') {
236236
state.assign(index++, effect.from, effect.into);
237+
if (block.terminal.kind === 'maybe-throw') {
238+
state.catchParams.add(effect.into.identifier);
239+
}
237240
} else {
238241
CompilerError.invariant(effect.kind === 'Freeze', {
239242
reason: `Unexpected '${effect.kind}' effect for MaybeThrow terminal`,
@@ -604,6 +607,13 @@ type Node = {
604607
};
605608
class AliasingState {
606609
nodes: Map<Identifier, Node> = new Map();
610+
/*
611+
* Identifiers that are catch handler bindings. These are tracked
612+
* so the backward-alias heuristic in mutate() can stop walking
613+
* through catch parameter aliases, which would otherwise incorrectly
614+
* match the "function call result" pattern and skip range extension.
615+
*/
616+
catchParams: Set<Identifier> = new Set();
607617

608618
create(place: Place, value: Node['value']): void {
609619
this.nodes.set(place.identifier, {
@@ -744,8 +754,10 @@ class AliasingState {
744754
* Apply results are identifiable by having maybeAliases, since the
745755
* default Apply handler records MaybeAlias from each argument to
746756
* the result. Only walk through alias edges (not maybeAliases or
747-
* captures) to avoid matching catch parameters and similar patterns
748-
* where range extension is required for correctness.
757+
* captures). Additionally, stop walking when a catch parameter is
758+
* reached, since catch parameters are aliased to call results via
759+
* terminal Alias effects and would otherwise incorrectly trigger
760+
* this heuristic.
749761
*/
750762
const visited = new Set<Identifier>();
751763
const aliasQueue: Array<Identifier> = [start];
@@ -759,6 +771,18 @@ class AliasingState {
759771
if (node == null) {
760772
continue;
761773
}
774+
/*
775+
* Stop walking through catch parameter aliases. Catch bindings
776+
* are aliased to call results in the try block, and those call
777+
* results have maybeAliases from the Apply handler. Without
778+
* this check, the heuristic would incorrectly treat catch
779+
* parameters as independently-memoizable function call results,
780+
* preventing backward range extension and causing scope boundary
781+
* issues (e.g. "Expected a break target" in try-catch blocks).
782+
*/
783+
if (this.catchParams.has(id)) {
784+
continue;
785+
}
762786
if (node.maybeAliases.size > 0) {
763787
effectiveSkipAliases = true;
764788
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/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 = {

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

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,31 +39,47 @@ import { makeArray } from "shared-runtime";
3939
* and avoid adding them to `temporariesUsedOutsideDefiningScope`.
4040
*/
4141
function Component(t0) {
42-
const $ = _c(6);
42+
const $ = _c(12);
4343
const { num } = t0;
44-
let T0;
4544
let t1;
4645
if ($[0] !== num) {
47-
const arr = makeArray(num);
48-
T0 = SharedRuntime.Stringify;
49-
t1 = arr.push(num);
46+
t1 = makeArray(num);
5047
$[0] = num;
51-
$[1] = T0;
52-
$[2] = t1;
48+
$[1] = t1;
5349
} else {
54-
T0 = $[1];
55-
t1 = $[2];
50+
t1 = $[1];
5651
}
52+
let T0;
5753
let t2;
58-
if ($[3] !== T0 || $[4] !== t1) {
59-
t2 = <T0 value={t1} />;
60-
$[3] = T0;
61-
$[4] = t1;
54+
if ($[2] !== num || $[3] !== t1) {
55+
const arr = t1;
56+
T0 = SharedRuntime.Stringify;
57+
if ($[6] !== arr || $[7] !== num) {
58+
t2 = arr.push(num);
59+
$[6] = arr;
60+
$[7] = num;
61+
$[8] = t2;
62+
} else {
63+
t2 = $[8];
64+
}
65+
$[2] = num;
66+
$[3] = t1;
67+
$[4] = T0;
6268
$[5] = t2;
6369
} else {
70+
T0 = $[4];
6471
t2 = $[5];
6572
}
66-
return t2;
73+
let t3;
74+
if ($[9] !== T0 || $[10] !== t2) {
75+
t3 = <T0 value={t2} />;
76+
$[9] = T0;
77+
$[10] = t2;
78+
$[11] = t3;
79+
} else {
80+
t3 = $[11];
81+
}
82+
return t3;
6783
}
6884

6985
export const FIXTURE_ENTRYPOINT = {

0 commit comments

Comments
 (0)