Skip to content

Commit 79e188d

Browse files
committed
[compiler] Support optional/logical/etc within try/catch
Control-flow expressions such as logicals, optionals, and ternaries were not supported within try/catch. BuildReactiveFunction recursively traversed these terminals, and was not expecting their condition blocks to end in a maybe-throw terminal, which can occur due to the try/catch. The fix here is to treat the maybe-throw similarly to goto, passing through to the continuation block.
1 parent 870cccd commit 79e188d

30 files changed

+1088
-230
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,10 @@ function findOptionalPlaces(fn: HIRFunction): Set<IdentifierId> {
583583
testBlock = fn.body.blocks.get(terminal.fallthrough)!;
584584
break;
585585
}
586+
case 'maybe-throw': {
587+
testBlock = fn.body.blocks.get(terminal.continuation)!;
588+
break;
589+
}
586590
default: {
587591
CompilerError.invariant(false, {
588592
reason: `Unexpected terminal in optional`,

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/BuildReactiveFunction.ts

Lines changed: 152 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
} from '../HIR';
2020
import {
2121
HIRFunction,
22+
isStatementBlockKind,
2223
ReactiveBreakTerminal,
2324
ReactiveContinueTerminal,
2425
ReactiveFunction,
@@ -62,6 +63,51 @@ class Driver {
6263
this.cx = cx;
6364
}
6465

66+
/*
67+
* Extracts the result value from instructions at the end of a value block.
68+
* Value blocks generally end in a StoreLocal to assign the value of the
69+
* expression. These StoreLocal instructions can be pruned since we represent
70+
* value blocks as compound values in ReactiveFunction (no phis). However,
71+
* it's also possible to have a value block that ends in an AssignmentExpression,
72+
* which we need to keep. So we only prune StoreLocal for temporaries.
73+
*/
74+
extractValueBlockResult(
75+
instructions: BasicBlock['instructions'],
76+
blockId: BlockId,
77+
loc: SourceLocation,
78+
): {block: BlockId; place: Place; value: ReactiveValue; id: InstructionId} {
79+
CompilerError.invariant(instructions.length !== 0, {
80+
reason: `Expected non-empty instructions in extractValueBlockResult`,
81+
description: null,
82+
loc,
83+
});
84+
const instr = instructions.at(-1)!;
85+
let place: Place = instr.lvalue;
86+
let value: ReactiveValue = instr.value;
87+
if (
88+
value.kind === 'StoreLocal' &&
89+
value.lvalue.place.identifier.name === null
90+
) {
91+
place = value.lvalue.place;
92+
value = {
93+
kind: 'LoadLocal',
94+
place: value.value,
95+
loc: value.value.loc,
96+
};
97+
}
98+
if (instructions.length === 1) {
99+
return {block: blockId, place, value, id: instr.id};
100+
}
101+
const sequence: ReactiveSequenceValue = {
102+
kind: 'SequenceExpression',
103+
instructions: instructions.slice(0, -1),
104+
id: instr.id,
105+
value,
106+
loc,
107+
};
108+
return {block: blockId, place, value: sequence, id: instr.id};
109+
}
110+
65111
traverseBlock(block: BasicBlock): ReactiveBlock {
66112
const blockValue: ReactiveBlock = [];
67113
this.visitBlock(block, blockValue);
@@ -899,78 +945,118 @@ class Driver {
899945
} else if (defaultBlock.terminal.kind === 'goto') {
900946
const instructions = defaultBlock.instructions;
901947
if (instructions.length === 0) {
902-
CompilerError.invariant(false, {
903-
reason: 'Expected goto value block to have at least one instruction',
904-
loc: GeneratedSource,
948+
/*
949+
* Empty goto blocks just forward to the next block.
950+
* Follow the goto to get the actual value.
951+
*/
952+
return this.visitValueBlock(defaultBlock.terminal.block, loc);
953+
}
954+
return this.extractValueBlockResult(instructions, defaultBlock.id, loc);
955+
} else if (defaultBlock.terminal.kind === 'maybe-throw') {
956+
/*
957+
* ReactiveFunction does not explicitly model maybe-throw semantics,
958+
* so maybe-throw terminals in value blocks flatten away. We continue
959+
* to the continuation block if it's still part of the value block
960+
* (expression-level), but stop if it's a statement-level block.
961+
*/
962+
const continuationBlock = this.cx.ir.blocks.get(
963+
defaultBlock.terminal.continuation,
964+
)!;
965+
if (isStatementBlockKind(continuationBlock.kind)) {
966+
/*
967+
* The continuation is a statement-level block. The value block ends at this block.
968+
* Process this block's instructions like a goto case.
969+
*/
970+
const instructions = defaultBlock.instructions;
971+
CompilerError.invariant(instructions.length !== 0, {
972+
reason: `Unexpected empty maybe-throw block with statement-level continuation`,
973+
description: null,
974+
loc: defaultBlock.terminal.loc,
905975
});
906-
} else if (defaultBlock.instructions.length === 1) {
907-
const instr = defaultBlock.instructions[0]!;
908-
let place: Place = instr.lvalue;
909-
let value: ReactiveValue = instr.value;
910-
if (
911-
/*
912-
* Value blocks generally end in a StoreLocal to assign the value of the
913-
* expression for this branch. These StoreLocal instructions can be pruned,
914-
* since we represent the value blocks as a compund value in ReactiveFunction
915-
* (no phis). However, it's also possible to have a value block that ends in
916-
* an AssignmentExpression, which we need to keep. So we only prune
917-
* StoreLocal for temporaries — any named/promoted values must be used
918-
* elsewhere and aren't safe to prune.
919-
*/
920-
value.kind === 'StoreLocal' &&
921-
value.lvalue.place.identifier.name === null
922-
) {
923-
place = value.lvalue.place;
924-
value = {
925-
kind: 'LoadLocal',
926-
place: value.value,
927-
loc: value.value.loc,
928-
};
929-
}
930-
return {
931-
block: defaultBlock.id,
932-
place,
933-
value,
934-
id: instr.id,
935-
};
936-
} else {
937-
const instr = defaultBlock.instructions.at(-1)!;
938-
let place: Place = instr.lvalue;
939-
let value: ReactiveValue = instr.value;
940-
if (
976+
return this.extractValueBlockResult(instructions, defaultBlock.id, loc);
977+
}
978+
/*
979+
* The continuation is expression-level. If it's empty (just has a terminal
980+
* like branch/optional/logical), we need to follow to it and include the
981+
* current block's instructions in a sequence. If it has instructions, we
982+
* recursively visit and merge.
983+
*/
984+
const isContinuationEmpty = continuationBlock.instructions.length === 0;
985+
if (isContinuationEmpty && defaultBlock.instructions.length === 0) {
986+
/*
987+
* Both the current block and continuation are empty. Just follow to
988+
* the continuation block.
989+
*/
990+
return this.visitValueBlock(defaultBlock.terminal.continuation, loc);
991+
} else if (isContinuationEmpty) {
992+
/*
993+
* The continuation block is empty but has a terminal.
994+
* If the terminal is also maybe-throw, we need to follow through to find
995+
* the actual terminal (branch/optional/logical/etc).
996+
*/
997+
if (continuationBlock.terminal.kind === 'maybe-throw') {
941998
/*
942-
* Value blocks generally end in a StoreLocal to assign the value of the
943-
* expression for this branch. These StoreLocal instructions can be pruned,
944-
* since we represent the value blocks as a compund value in ReactiveFunction
945-
* (no phis). However, it's also possible to have a value block that ends in
946-
* an AssignmentExpression, which we need to keep. So we only prune
947-
* StoreLocal for temporaries — any named/promoted values must be used
948-
* elsewhere and aren't safe to prune.
999+
* The continuation is also a maybe-throw. Follow through recursively
1000+
* to find the final block with the actual terminal.
9491001
*/
950-
value.kind === 'StoreLocal' &&
951-
value.lvalue.place.identifier.name === null
952-
) {
953-
place = value.lvalue.place;
954-
value = {
955-
kind: 'LoadLocal',
956-
place: value.value,
957-
loc: value.value.loc,
958-
};
1002+
const nextContinuation = this.visitValueBlock(
1003+
continuationBlock.terminal.continuation,
1004+
loc,
1005+
);
1006+
if (defaultBlock.instructions.length > 0) {
1007+
const innerSequence: ReactiveSequenceValue = {
1008+
kind: 'SequenceExpression',
1009+
instructions: defaultBlock.instructions,
1010+
id: nextContinuation.id,
1011+
value: nextContinuation.value,
1012+
loc,
1013+
};
1014+
return {
1015+
block: nextContinuation.block,
1016+
value: innerSequence,
1017+
place: nextContinuation.place,
1018+
id: nextContinuation.id,
1019+
};
1020+
}
1021+
return nextContinuation;
9591022
}
960-
const sequence: ReactiveSequenceValue = {
1023+
/*
1024+
* The continuation block is empty with a non-maybe-throw terminal
1025+
* (branch/optional/logical/etc). Extract the result from the current
1026+
* block's instructions but return the continuation block's ID so that
1027+
* visitValueBlockTerminal can find the terminal.
1028+
*/
1029+
const result = this.extractValueBlockResult(
1030+
defaultBlock.instructions,
1031+
continuationBlock.id,
1032+
loc,
1033+
);
1034+
return result;
1035+
}
1036+
/*
1037+
* The continuation is still expression-level with instructions, so continue visiting.
1038+
* Include any instructions from this block in a sequence.
1039+
*/
1040+
const continuation = this.visitValueBlock(
1041+
defaultBlock.terminal.continuation,
1042+
loc,
1043+
);
1044+
if (defaultBlock.instructions.length > 0) {
1045+
const innerSequence: ReactiveSequenceValue = {
9611046
kind: 'SequenceExpression',
962-
instructions: defaultBlock.instructions.slice(0, -1),
963-
id: instr.id,
964-
value,
965-
loc: loc,
1047+
instructions: defaultBlock.instructions,
1048+
id: continuation.id,
1049+
value: continuation.value,
1050+
loc,
9661051
};
9671052
return {
968-
block: defaultBlock.id,
969-
place,
970-
value: sequence,
971-
id: instr.id,
1053+
block: continuation.block,
1054+
value: innerSequence,
1055+
place: continuation.place,
1056+
id: continuation.id,
9721057
};
9731058
}
1059+
return continuation;
9741060
} else {
9751061
/*
9761062
* The value block ended in a value terminal, recurse to get the value
@@ -996,7 +1082,7 @@ class Driver {
9961082
loc,
9971083
};
9981084
return {
999-
block: init.fallthrough,
1085+
block: final.block,
10001086
value: sequence,
10011087
place: final.place,
10021088
id: final.id,
@@ -1145,11 +1231,10 @@ class Driver {
11451231
};
11461232
}
11471233
case 'maybe-throw': {
1148-
CompilerError.throwTodo({
1149-
reason: `Support value blocks (conditional, logical, optional chaining, etc) within a try/catch statement`,
1234+
CompilerError.invariant(false, {
1235+
reason: `Unexpected maybe-throw in visitValueBlockTerminal - should be handled in visitValueBlock`,
11501236
description: null,
11511237
loc: terminal.loc,
1152-
suggestions: null,
11531238
});
11541239
}
11551240
case 'label': {

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,6 +1016,10 @@ export function findOptionalPlaces(
10161016
testBlock = fn.body.blocks.get(terminal.block)!;
10171017
break;
10181018
}
1019+
case 'maybe-throw': {
1020+
testBlock = fn.body.blocks.get(terminal.continuation)!;
1021+
break;
1022+
}
10191023
default: {
10201024
CompilerError.invariant(false, {
10211025
reason: `Unexpected terminal in optional`,

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-invariant-unexpected-terminal-in-optional.expect.md

Lines changed: 0 additions & 34 deletions
This file was deleted.

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-invariant-unexpected-terminal-in-optional.js

Lines changed: 0 additions & 8 deletions
This file was deleted.

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-logical-expression-within-try-catch.expect.md

Lines changed: 0 additions & 35 deletions
This file was deleted.

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-logical-expression-within-try-catch.js

Lines changed: 0 additions & 9 deletions
This file was deleted.

0 commit comments

Comments
 (0)