@samitouri / QOS-React / commits / ee77d91ca2

TryStatement: handle catch clause params

It's possible that the value thrown during a `try` block actually is a reference to some value defined outside the scope of the try block. If the catch clause param is also mutated, that means the mutable range of the variable would have to include the entire try/catch. We handle this by emitting a DeclareLocal temporary for the catch param prior to the try/catch. If it is modified during the catch block, that will extend its mutable range to cover the full try/catch. If any values are mutated inside the try, their range will also (naturally) extend around the full try/catch block. These ranges will overlap and be merged, ensuring that we capture the possibility that the value is mutated via the catch param. See unit test.

Joe Savona committed Sep 7, 2023 at 16:32 UTC ee77d91ca2a3862cd60b8f59391a9a13241607ea
9 files changed +140 -10
compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts
+47 -8
@@ -941,7 +941,52 @@ function lowerStatement(
941 });
942 return;
943 }
944 + if (stmt.get("finalizer").node != null) {
945 + builder.errors.push({
946 + reason: `(BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause`,
947 + severity: ErrorSeverity.Todo,
948 + loc: stmt.node.loc ?? null,
949 + suggestions: null,
950 + });
951 + }
952 +
953 + const handlerBindingPath = handlerPath.get("param");
954 + let handlerBinding: {
955 + place: Place;
956 + path: NodePath<t.Identifier | t.ArrayPattern | t.ObjectPattern>;
957 + } | null = null;
958 + if (handlerBindingPath.node != null && handlerBindingPath.hasNode()) {
959 + const place: Place = {
960 + kind: "Identifier",
961 + identifier: builder.makeTemporary(),
962 + effect: Effect.Unknown,
963 + loc: handlerBindingPath.node.loc ?? GeneratedSource,
964 + };
965 + lowerValueToTemporary(builder, {
966 + kind: "DeclareLocal",
967 + lvalue: {
968 + kind: InstructionKind.Catch,
969 + place: { ...place },
970 + },
971 + loc: handlerBindingPath.node.loc ?? GeneratedSource,
972 + });
973 +
974 + handlerBinding = {
975 + path: handlerBindingPath,
976 + place,
977 + };
978 + }
979 +
980 const handler = builder.enter("block", (_blockId) => {
981 + if (handlerBinding !== null) {
982 + lowerAssignment(
983 + builder,
984 + handlerBinding.path.node.loc ?? GeneratedSource,
985 + InstructionKind.Catch,
986 + handlerBinding.path,
987 + { ...handlerBinding.place }
988 + );
989 + }
990 lowerStatement(builder, handlerPath.get("body"));
991 return {
992 kind: "goto",
@@ -951,14 +996,6 @@ function lowerStatement(
996 loc: handlerPath.node.loc ?? GeneratedSource,
997 };
998 });
954 - if (stmt.get("finalizer").node != null) {
955 - builder.errors.push({
956 - reason: `(BuildHIR::lowerStatement) Handle TryStatement with a finalizer ('finally') clause`,
957 - severity: ErrorSeverity.Todo,
958 - loc: stmt.node.loc ?? null,
959 - suggestions: null,
960 - });
961 - }
999
1000 const block = builder.enter("block", (_blockId) => {
1001 const block = stmt.get("block");
@@ -978,6 +1015,8 @@ function lowerStatement(
1015 {
1016 kind: "try",
1017 block,
1018 + handlerBinding:
1019 + handlerBinding !== null ? { ...handlerBinding.place } : null,
1020 handler,
1021 fallthrough: continuationBlock.id,
1022 id: makeInstructionId(0),
compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts
+6
@@ -213,6 +213,7 @@ export type ReactiveLabelTerminal = {
213 export type ReactiveTryTerminal = {
214 kind: "try";
215 block: ReactiveBlock;
216 + handlerBinding: Place | null;
217 handler: ReactiveBlock;
218 id: InstructionId;
219 };
@@ -457,6 +458,7 @@ export type SequenceTerminal = {
458 export type TryTerminal = {
459 kind: "try";
460 block: BlockId;
461 + handlerBinding: Place | null;
462 handler: BlockId;
463 // TODO: support `finally`
464 fallthrough: BlockId | null;
@@ -547,6 +549,10 @@ export enum InstructionKind {
549 * assing a new value to a let binding
550 */
551 Reassign = "Reassign",
552 + /**
553 + * catch clause binding
554 + */
555 + Catch = "Catch",
556 }
557
558 function _staticInvariantInstructionValueHasLocation(
compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts
+3
@@ -590,6 +590,9 @@ export function printLValue(lval: LValue): string {
590 case InstructionKind.Reassign: {
591 return `Reassign ${lvalue}`;
592 }
593 + case InstructionKind.Catch: {
594 + return `Catch ${lvalue}`;
595 + }
596 default: {
597 assertExhaustive(lval.kind, `Unexpected lvalue kind '${lval.kind}'`);
598 }
compiler/packages/babel-plugin-react-forget/src/HIR/visitors.ts
+9 -1
@@ -750,6 +750,7 @@ export function mapTerminalSuccessors(
750 return {
751 kind: "try",
752 block,
753 + handlerBinding: terminal.handlerBinding,
754 handler,
755 fallthrough,
756 id: makeInstructionId(0),
@@ -1001,7 +1002,14 @@ export function mapTerminalOperands(
1002 terminal.value = fn(terminal.value);
1003 break;
1004 }
1004 - case "try":
1005 + case "try": {
1006 + if (terminal.handlerBinding !== null) {
1007 + terminal.handlerBinding = fn(terminal.handlerBinding);
1008 + } else {
1009 + terminal.handlerBinding = null;
1010 + }
1011 + break;
1012 + }
1013 case "maybe-throw":
1014 case "sequence":
1015 case "label":
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/BuildReactiveFunction.ts
+1
@@ -676,6 +676,7 @@ class Driver {
676 terminal: {
677 kind: "try",
678 block,
679 + handlerBinding: terminal.handlerBinding,
680 handler,
681 id: terminal.id,
682 },
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+16 -1
@@ -454,6 +454,13 @@ function codegenTerminal(
454 loc: iterableItem.loc,
455 suggestions: null,
456 });
457 + case InstructionKind.Catch:
458 + CompilerError.invariant(false, {
459 + reason: "Unexpected catch variable as for-of collection",
460 + description: null,
461 + loc: iterableItem.loc,
462 + suggestions: null,
463 + });
464 default:
465 assertExhaustive(
466 iterableItem.value.lvalue.kind,
@@ -516,9 +523,14 @@ function codegenTerminal(
523 return codegenBlock(cx, terminal.block);
524 }
525 case "try": {
526 + let catchParam = null;
527 + if (terminal.handlerBinding !== null) {
528 + catchParam = convertIdentifier(terminal.handlerBinding.identifier);
529 + cx.temp.set(terminal.handlerBinding.identifier.id, null);
530 + }
531 return t.tryStatement(
532 codegenBlock(cx, terminal.block),
521 - t.catchClause(null, codegenBlock(cx, terminal.handler))
533 + t.catchClause(catchParam, codegenBlock(cx, terminal.handler))
534 );
535 }
536 default: {
@@ -638,6 +650,9 @@ function codegenInstructionNullable(
650 return createExpressionStatement(instr.loc, expr);
651 }
652 }
653 + case InstructionKind.Catch: {
654 + return t.emptyStatement();
655 + }
656 default: {
657 assertExhaustive(kind, `Unexpected instruction kind '${kind}'`);
658 }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-catch-param.expect.md new
+45
@@ -0,0 +1,45 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + let x = [];
7 + try {
8 + // foo could throw its argument...
9 + foo(x);
10 + } catch (e) {
11 + // ... in which case this could be mutating `x`!
12 + e.push(null);
13 + return e;
14 + }
15 + return x;
16 +}
17 +
18 +```
19 +
20 +## Code
21 +
22 +```javascript
23 +import { unstable_useMemoCache as useMemoCache } from "react";
24 +function Component(props) {
25 + const $ = useMemoCache(1);
26 + let x;
27 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
28 + x = [];
29 + try {
30 + foo(x);
31 + } catch (t22) {
32 + const e = t22;
33 +
34 + e.push(null);
35 + return e;
36 + }
37 + $[0] = x;
38 + } else {
39 + x = $[0];
40 + }
41 + return x;
42 +}
43 +
44 +```
45 +
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/try-catch-with-catch-param.js new
+12
@@ -0,0 +1,12 @@
1 +function Component(props) {
2 + let x = [];
3 + try {
4 + // foo could throw its argument...
5 + foo(x);
6 + } catch (e) {
7 + // ... in which case this could be mutating `x`!
8 + e.push(null);
9 + return e;
10 + }
11 + return x;
12 +}
compiler/packages/sprout/src/SproutTodoFilter.ts
+1
@@ -416,6 +416,7 @@ const skipFilter = new Set([
416 "try-catch-within-mutable-range",
417 "try-catch",
418 "try-catch-with-return",
419 + "try-catch-with-catch-param",
420
421 // TODO: 🌲
422 "forest-basic",