Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[compiler] Add fallthrough to branch terminal #30814

Merged
merged 4 commits into from
Aug 28, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[compiler] Add fallthrough to branch terminal
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-poisoned]
josephsavona committed Aug 26, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 1846220d81f85c44868d910bdc6f5526eb0a5c5b
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
@@ -607,6 +607,7 @@ function lowerStatement(
),
consequent: bodyBlock,
alternate: continuationBlock.id,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc: stmt.node.loc ?? GeneratedSource,
},
@@ -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,
};
@@ -975,6 +973,7 @@ function lowerStatement(
test,
consequent: loopBlock,
alternate: continuationBlock.id,
fallthrough: conditionalBlock.id,
id: makeInstructionId(0),
loc,
};
@@ -1118,6 +1117,7 @@ function lowerStatement(
consequent: loopBlock,
alternate: continuationBlock.id,
loc: stmt.node.loc ?? GeneratedSource,
fallthrough: continuationBlock.id,
},
continuationBlock,
);
@@ -1203,6 +1203,7 @@ function lowerStatement(
test,
consequent: loopBlock,
alternate: continuationBlock.id,
fallthrough: continuationBlock.id,
loc: stmt.node.loc ?? GeneratedSource,
},
continuationBlock,
@@ -1800,6 +1801,7 @@ function lowerExpression(
test: {...testPlace},
consequent: consequentBlock,
alternate: alternateBlock,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc: exprLoc,
},
@@ -1878,6 +1880,7 @@ function lowerExpression(
test: {...leftPlace},
consequent,
alternate,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc: exprLoc,
},
@@ -2611,6 +2614,7 @@ function lowerOptionalMemberExpression(
test: {...object},
consequent: consequent.id,
alternate,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc,
};
@@ -2750,6 +2754,7 @@ function lowerOptionalCallExpression(
test: {...testPlace},
consequent: consequent.id,
alternate,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc,
};
@@ -4025,6 +4030,7 @@ function lowerAssignment(
test: {...test},
consequent,
alternate,
fallthrough: continuationBlock.id,
id: makeInstructionId(0),
loc,
},
Original file line number Diff line number Diff line change
@@ -491,7 +491,7 @@ export type BranchTerminal = {
alternate: BlockId;
id: InstructionId;
loc: SourceLocation;
fallthrough?: never;
fallthrough: BlockId;
};

export type SwitchTerminal = {
Original file line number Diff line number Diff line change
@@ -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,
};
@@ -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':
@@ -892,6 +893,7 @@ export function terminalHasFallthrough<
const _: undefined = terminal.fallthrough;
return false;
}
case 'branch':
case 'try':
case 'do-while':
case 'for-of':
Original file line number Diff line number Diff line change
@@ -476,3 +476,12 @@
}
}
}

function findOptionalPlaces(fn: HIRFunction): Set<IdentifierId> {

Check failure on line 480 in compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts

GitHub Actions / Lint babel-plugin-react-compiler

'findOptionalPlaces' is defined but never used. Allowed unused vars must match /^_/u
const optionals = new Set<IdentifierId>();
for (const [, block] of fn.body.blocks) {
if (block.terminal.kind === 'optional') {

Check failure on line 483 in compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts

GitHub Actions / Lint babel-plugin-react-compiler

Empty block statement
}
}
return optionals;
}
Original file line number Diff line number Diff line change
@@ -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

Unchanged files with check annotations Beta

import {CompilerError} from '..';
import {
DeclarationId,
DependencyPath,

Check failure on line 11 in compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts

GitHub Actions / Lint babel-plugin-react-compiler

'DependencyPath' is defined but never used. Allowed unused vars must match /^_/u
InstructionId,
InstructionKind,
Place,