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] Implement support for hoisted and recursive functions #30922

Merged
merged 3 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
30 changes: 16 additions & 14 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,19 @@ function lowerStatement(
// Already hoisted
continue;
}
if (!binding.path.isVariableDeclarator()) {

let kind:
| InstructionKind.Let
| InstructionKind.HoistedConst
| InstructionKind.HoistedLet
| InstructionKind.HoistedFunction;
if (binding.kind === 'const' || binding.kind === 'var') {
kind = InstructionKind.HoistedConst;
} else if (binding.kind === 'let') {
kind = InstructionKind.HoistedLet;
} else if (binding.path.isFunctionDeclaration()) {
kind = InstructionKind.HoistedFunction;
} else if (!binding.path.isVariableDeclarator()) {
builder.errors.push({
severity: ErrorSeverity.Todo,
reason: 'Unsupported declaration type for hoisting',
Expand All @@ -429,11 +441,7 @@ function lowerStatement(
loc: id.parentPath.node.loc ?? GeneratedSource,
});
continue;
} else if (
binding.kind !== 'const' &&
binding.kind !== 'var' &&
binding.kind !== 'let'
) {
} else {
builder.errors.push({
severity: ErrorSeverity.Todo,
reason: 'Handle non-const declarations for hoisting',
Expand All @@ -443,6 +451,7 @@ function lowerStatement(
});
continue;
}

const identifier = builder.resolveIdentifier(id);
CompilerError.invariant(identifier.kind === 'Identifier', {
reason:
Expand All @@ -456,13 +465,6 @@ function lowerStatement(
reactive: false,
loc: id.node.loc ?? GeneratedSource,
};
const kind =
// Avoid double errors on var declarations, which we do not plan to support anyways
binding.kind === 'const' || binding.kind === 'var'
? InstructionKind.HoistedConst
: binding.kind === 'let'
? InstructionKind.HoistedLet
: assertExhaustive(binding.kind, 'Unexpected binding kind');
lowerValueToTemporary(builder, {
kind: 'DeclareContext',
lvalue: {
Expand Down Expand Up @@ -999,7 +1001,7 @@ function lowerStatement(
lowerAssignment(
builder,
stmt.node.loc ?? GeneratedSource,
InstructionKind.Let,
InstructionKind.Function,
id,
fn,
'Assignment',
Expand Down
6 changes: 5 additions & 1 deletion compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,9 @@ export enum InstructionKind {

// hoisted const declarations
HoistedLet = 'HoistedLet',

HoistedFunction = 'HoistedFunction',
Function = 'Function',
}

function _staticInvariantInstructionValueHasLocation(
Expand Down Expand Up @@ -865,7 +868,8 @@ export type InstructionValue =
kind:
| InstructionKind.Let
| InstructionKind.HoistedConst
| InstructionKind.HoistedLet;
| InstructionKind.HoistedLet
| InstructionKind.HoistedFunction;
place: Place;
};
loc: SourceLocation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,12 @@ export function printLValue(lval: LValue): string {
case InstructionKind.HoistedLet: {
return `HoistedLet ${lvalue}$`;
}
case InstructionKind.Function: {
return `Function ${lvalue}$`;
}
case InstructionKind.HoistedFunction: {
return `HoistedFunction ${lvalue}$`;
}
default: {
assertExhaustive(lval.kind, `Unexpected lvalue kind \`${lval.kind}\``);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -981,22 +981,12 @@ function codegenTerminal(
suggestions: null,
});
case InstructionKind.Catch:
CompilerError.invariant(false, {
reason: 'Unexpected catch variable as for..in collection',
description: null,
loc: iterableItem.loc,
suggestions: null,
});
case InstructionKind.HoistedConst:
CompilerError.invariant(false, {
reason: 'Unexpected HoistedConst variable in for..in collection',
description: null,
loc: iterableItem.loc,
suggestions: null,
});
case InstructionKind.HoistedLet:
case InstructionKind.HoistedFunction:
case InstructionKind.Function:
CompilerError.invariant(false, {
reason: 'Unexpected HoistedLet variable in for..in collection',
reason: `Unexpected ${iterableItem.value.lvalue.kind} variable in for..in collection`,
description: null,
loc: iterableItem.loc,
suggestions: null,
Expand Down Expand Up @@ -1075,30 +1065,13 @@ function codegenTerminal(
varDeclKind = 'let' as const;
break;
case InstructionKind.Reassign:
CompilerError.invariant(false, {
reason:
'Destructure should never be Reassign as it would be an Object/ArrayPattern',
description: null,
loc: iterableItem.loc,
suggestions: null,
});
case InstructionKind.Catch:
CompilerError.invariant(false, {
reason: 'Unexpected catch variable as for..of collection',
description: null,
loc: iterableItem.loc,
suggestions: null,
});
case InstructionKind.HoistedConst:
CompilerError.invariant(false, {
reason: 'Unexpected HoistedConst variable in for..of collection',
description: null,
loc: iterableItem.loc,
suggestions: null,
});
case InstructionKind.HoistedLet:
case InstructionKind.HoistedFunction:
case InstructionKind.Function:
CompilerError.invariant(false, {
reason: 'Unexpected HoistedLet variable in for..of collection',
reason: `Unexpected ${iterableItem.value.lvalue.kind} variable in for..of collection`,
description: null,
loc: iterableItem.loc,
suggestions: null,
Expand Down Expand Up @@ -1261,6 +1234,35 @@ function codegenInstructionNullable(
t.variableDeclarator(codegenLValue(cx, lvalue), value),
]);
}
case InstructionKind.Function: {
CompilerError.invariant(instr.lvalue === null, {
reason: `Function declaration cannot be referenced as an expression`,
description: null,
loc: instr.value.loc,
suggestions: null,
});
const genLvalue = codegenLValue(cx, lvalue);
CompilerError.invariant(genLvalue.type === 'Identifier', {
reason: 'Expected an identifier as a function declaration lvalue',
description: null,
loc: instr.value.loc,
suggestions: null,
});
CompilerError.invariant(value?.type === 'FunctionExpression', {
reason: 'Expected a function as a function declaration value',
description: null,
loc: instr.value.loc,
suggestions: null,
});
return createFunctionDeclaration(
instr.loc,
genLvalue,
value.params,
value.body,
value.generator,
value.async,
);
}
case InstructionKind.Let: {
CompilerError.invariant(instr.lvalue === null, {
reason: `Const declaration cannot be referenced as an expression`,
Expand Down Expand Up @@ -1303,19 +1305,11 @@ function codegenInstructionNullable(
case InstructionKind.Catch: {
return t.emptyStatement();
}
case InstructionKind.HoistedLet: {
CompilerError.invariant(false, {
reason:
'Expected HoistedLet to have been pruned in PruneHoistedContexts',
description: null,
loc: instr.loc,
suggestions: null,
});
}
case InstructionKind.HoistedConst: {
case InstructionKind.HoistedLet:
case InstructionKind.HoistedConst:
case InstructionKind.HoistedFunction: {
CompilerError.invariant(false, {
reason:
'Expected HoistedConsts to have been pruned in PruneHoistedContexts',
reason: `Expected ${kind} to have been pruned in PruneHoistedContexts`,
description: null,
loc: instr.loc,
suggestions: null,
Expand Down Expand Up @@ -1486,6 +1480,7 @@ const createBinaryExpression = withLoc(t.binaryExpression);
const createExpressionStatement = withLoc(t.expressionStatement);
const _createLabelledStatement = withLoc(t.labeledStatement);
const createVariableDeclaration = withLoc(t.variableDeclaration);
const createFunctionDeclaration = withLoc(t.functionDeclaration);
const _createWhileStatement = withLoc(t.whileStatement);
const createTaggedTemplateExpression = withLoc(t.taggedTemplateExpression);
const createLogicalExpression = withLoc(t.logicalExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,17 @@ class Visitor extends ReactiveFunctionTransform<HoistedIdentifiers> {
return {kind: 'remove'};
}

if (
instruction.value.kind === 'DeclareContext' &&
instruction.value.lvalue.kind === 'HoistedFunction'
) {
state.set(
instruction.value.lvalue.place.identifier.declarationId,
InstructionKind.Function,
);
return {kind: 'remove'};
}

if (
instruction.value.kind === 'StoreContext' &&
state.has(instruction.value.lvalue.place.identifier.declarationId)
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ export const FIXTURE_ENTRYPOINT = {
## Error

```
3 | return x;
4 | }
> 5 | return baz(); // OK: FuncDecls are HoistableDeclarations that have both declaration and value hoisting
| ^^^^^ Todo: Unsupported declaration type for hoisting. variable "baz" declared with FunctionDeclaration (5:5)
6 | function baz() {
7 | return bar();
8 | }
5 | return baz(); // OK: FuncDecls are HoistableDeclarations that have both declaration and value hoisting
6 | function baz() {
> 7 | return bar();
| ^^^ Todo: Support functions with unreachable code that may contain hoisted declarations (7:7)
8 | }
9 | }
10 |
```


Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@ function Component() {

```
1 | function Component() {
> 2 | return get2();
| ^^^^^^ Todo: Unsupported declaration type for hoisting. variable "get2" declared with FunctionDeclaration (2:2)
3 | function get2() {
4 | return 2;
5 | }
2 | return get2();
> 3 | function get2() {
| ^^^^^^^^^^^^^^^^^
> 4 | return 2;
| ^^^^^^^^^^^^^
> 5 | }
| ^^^^ Todo: Support functions with unreachable code that may contain hoisted declarations (3:5)
6 | }
7 |
```


This file was deleted.

Loading