Skip to content

Commit

Permalink
[compiler] Add fallthrough to branch terminal
Browse files Browse the repository at this point in the history
Branch terminals didn't have a fallthrough because they correspond to an outer terminal (optional, logical, etc) that has the "real" fallthrough. But understanding how branch terminals correspond to these outer terminals requires knowing the branch fallthrough. For example, `foo?.bar?.baz` creates terminals along the lines of:

```
bb0:
  optional fallthrough=bb4
bb1:
  optional fallthrough=bb3
bb2:
  ...
  branch ... (fallthrough=bb3)

...

bb3:
  ...
  branch ... (fallthrough=bb4)

...

bb4:
  ...
```

Without a fallthrough on `branch` terminals, it's unclear that the optional from bb0 has its branch node in bb3. With the fallthroughs, we can see look for a branch with the same fallthrough as the outer optional terminal to match them up.

ghstack-source-id: 064178d377e1eb8a308f4d2f0d3f0132817a5b56
Pull Request resolved: facebook/react#30814
  • Loading branch information
josephsavona committed Aug 28, 2024
1 parent 7212340 commit 49ab600
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 7 deletions.
14 changes: 10 additions & 4 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,7 @@ function lowerStatement(
),
consequent: bodyBlock,
alternate: continuationBlock.id,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc: stmt.node.loc ?? GeneratedSource,
},
Expand Down Expand Up @@ -656,16 +657,13 @@ function lowerStatement(
},
conditionalBlock,
);
/*
* The conditional block is empty and exists solely as conditional for
* (re)entering or exiting the loop
*/
const test = lowerExpressionToTemporary(builder, stmt.get('test'));
const terminal: BranchTerminal = {
kind: 'branch',
test,
consequent: loopBlock,
alternate: continuationBlock.id,
fallthrough: conditionalBlock.id,
id: makeInstructionId(0),
loc: stmt.node.loc ?? GeneratedSource,
};
Expand Down Expand Up @@ -975,6 +973,7 @@ function lowerStatement(
test,
consequent: loopBlock,
alternate: continuationBlock.id,
fallthrough: conditionalBlock.id,
id: makeInstructionId(0),
loc,
};
Expand Down Expand Up @@ -1118,6 +1117,7 @@ function lowerStatement(
consequent: loopBlock,
alternate: continuationBlock.id,
loc: stmt.node.loc ?? GeneratedSource,
fallthrough: continuationBlock.id,
},
continuationBlock,
);
Expand Down Expand Up @@ -1203,6 +1203,7 @@ function lowerStatement(
test,
consequent: loopBlock,
alternate: continuationBlock.id,
fallthrough: continuationBlock.id,
loc: stmt.node.loc ?? GeneratedSource,
},
continuationBlock,
Expand Down Expand Up @@ -1800,6 +1801,7 @@ function lowerExpression(
test: {...testPlace},
consequent: consequentBlock,
alternate: alternateBlock,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc: exprLoc,
},
Expand Down Expand Up @@ -1878,6 +1880,7 @@ function lowerExpression(
test: {...leftPlace},
consequent,
alternate,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc: exprLoc,
},
Expand Down Expand Up @@ -2611,6 +2614,7 @@ function lowerOptionalMemberExpression(
test: {...object},
consequent: consequent.id,
alternate,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc,
};
Expand Down Expand Up @@ -2750,6 +2754,7 @@ function lowerOptionalCallExpression(
test: {...testPlace},
consequent: consequent.id,
alternate,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc,
};
Expand Down Expand Up @@ -4025,6 +4030,7 @@ function lowerAssignment(
test: {...test},
consequent,
alternate,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ export type BranchTerminal = {
alternate: BlockId;
id: InstructionId;
loc: SourceLocation;
fallthrough?: never;
fallthrough: BlockId;
};

export type SwitchTerminal = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -660,11 +660,13 @@ export function mapTerminalSuccessors(
case 'branch': {
const consequent = fn(terminal.consequent);
const alternate = fn(terminal.alternate);
const fallthrough = fn(terminal.fallthrough);
return {
kind: 'branch',
test: terminal.test,
consequent,
alternate,
fallthrough,
id: makeInstructionId(0),
loc: terminal.loc,
};
Expand Down Expand Up @@ -883,7 +885,6 @@ export function terminalHasFallthrough<
>(terminal: T): terminal is U {
switch (terminal.kind) {
case 'maybe-throw':
case 'branch':
case 'goto':
case 'return':
case 'throw':
Expand All @@ -892,6 +893,7 @@ export function terminalHasFallthrough<
const _: undefined = terminal.fallthrough;
return false;
}
case 'branch':
case 'try':
case 'do-while':
case 'for-of':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -476,3 +476,12 @@ export function dropManualMemoization(func: HIRFunction): void {
}
}
}

function findOptionalPlaces(fn: HIRFunction): Set<IdentifierId> {
const optionals = new Set<IdentifierId>();
for (const [, block] of fn.body.blocks) {
if (block.terminal.kind === 'optional') {
}
}
return optionals;
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export function alignReactiveScopesToBlockScopesHIR(fn: HIRFunction): void {
}

const fallthrough = terminalFallthrough(terminal);
if (fallthrough !== null) {
if (fallthrough !== null && terminal.kind !== 'branch') {
/*
* Any currently active scopes that overlaps the block-fallthrough range
* need their range extended to at least the first instruction of the
Expand Down

0 comments on commit 49ab600

Please sign in to comment.