Skip to content

Commit ed343b9

Browse files
committed
[compiler] Do not inline IIFEs in value blocks
As discussed in chat, this is a simple fix to stop introducing labels inside expressions. The useMemo-with-optional test was added in d70b2c2 and crashes for the same reason- an unexpected label as a value block terminal.
1 parent 299762d commit ed343b9

8 files changed

+195
-155
lines changed

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

Lines changed: 90 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
InstructionKind,
1818
LabelTerminal,
1919
Place,
20+
isStatementBlockKind,
2021
makeInstructionId,
2122
promoteTemporary,
2223
reversePostorderBlocks,
@@ -90,100 +91,106 @@ export function inlineImmediatelyInvokedFunctionExpressions(
9091
*/
9192
const queue = Array.from(fn.body.blocks.values());
9293
queue: for (const block of queue) {
93-
for (let ii = 0; ii < block.instructions.length; ii++) {
94-
const instr = block.instructions[ii]!;
95-
switch (instr.value.kind) {
96-
case 'FunctionExpression': {
97-
if (instr.lvalue.identifier.name === null) {
98-
functions.set(instr.lvalue.identifier.id, instr.value);
99-
}
100-
break;
101-
}
102-
case 'CallExpression': {
103-
if (instr.value.args.length !== 0) {
104-
// We don't support inlining when there are arguments
105-
continue;
106-
}
107-
const body = functions.get(instr.value.callee.identifier.id);
108-
if (body === undefined) {
109-
// Not invoking a local function expression, can't inline
110-
continue;
94+
/*
95+
* We can't handle labels inside expressions yet, so we don't inline IIFEs if they are in an
96+
* expression block.
97+
*/
98+
if (isStatementBlockKind(block.kind)) {
99+
for (let ii = 0; ii < block.instructions.length; ii++) {
100+
const instr = block.instructions[ii]!;
101+
switch (instr.value.kind) {
102+
case 'FunctionExpression': {
103+
if (instr.lvalue.identifier.name === null) {
104+
functions.set(instr.lvalue.identifier.id, instr.value);
105+
}
106+
break;
111107
}
108+
case 'CallExpression': {
109+
if (instr.value.args.length !== 0) {
110+
// We don't support inlining when there are arguments
111+
continue;
112+
}
113+
const body = functions.get(instr.value.callee.identifier.id);
114+
if (body === undefined) {
115+
// Not invoking a local function expression, can't inline
116+
continue;
117+
}
112118

113-
if (
114-
body.loweredFunc.func.params.length > 0 ||
115-
body.loweredFunc.func.async ||
116-
body.loweredFunc.func.generator
117-
) {
118-
// Can't inline functions with params, or async/generator functions
119-
continue;
120-
}
119+
if (
120+
body.loweredFunc.func.params.length > 0 ||
121+
body.loweredFunc.func.async ||
122+
body.loweredFunc.func.generator
123+
) {
124+
// Can't inline functions with params, or async/generator functions
125+
continue;
126+
}
121127

122-
// We know this function is used for an IIFE and can prune it later
123-
inlinedFunctions.add(instr.value.callee.identifier.id);
128+
// We know this function is used for an IIFE and can prune it later
129+
inlinedFunctions.add(instr.value.callee.identifier.id);
124130

125-
// Create a new block which will contain code following the IIFE call
126-
const continuationBlockId = fn.env.nextBlockId;
127-
const continuationBlock: BasicBlock = {
128-
id: continuationBlockId,
129-
instructions: block.instructions.slice(ii + 1),
130-
kind: block.kind,
131-
phis: new Set(),
132-
preds: new Set(),
133-
terminal: block.terminal,
134-
};
135-
fn.body.blocks.set(continuationBlockId, continuationBlock);
131+
// Create a new block which will contain code following the IIFE call
132+
const continuationBlockId = fn.env.nextBlockId;
133+
const continuationBlock: BasicBlock = {
134+
id: continuationBlockId,
135+
instructions: block.instructions.slice(ii + 1),
136+
kind: block.kind,
137+
phis: new Set(),
138+
preds: new Set(),
139+
terminal: block.terminal,
140+
};
141+
fn.body.blocks.set(continuationBlockId, continuationBlock);
136142

137-
/*
138-
* Trim the original block to contain instructions up to (but not including)
139-
* the IIFE
140-
*/
141-
block.instructions.length = ii;
143+
/*
144+
* Trim the original block to contain instructions up to (but not including)
145+
* the IIFE
146+
*/
147+
block.instructions.length = ii;
142148

143-
/*
144-
* To account for complex control flow within the lambda, we treat the lambda
145-
* as if it were a single labeled statement, and replace all returns with gotos
146-
* to the label fallthrough.
147-
*/
148-
const newTerminal: LabelTerminal = {
149-
block: body.loweredFunc.func.body.entry,
150-
id: makeInstructionId(0),
151-
kind: 'label',
152-
fallthrough: continuationBlockId,
153-
loc: block.terminal.loc,
154-
};
155-
block.terminal = newTerminal;
149+
/*
150+
* To account for complex control flow within the lambda, we treat the lambda
151+
* as if it were a single labeled statement, and replace all returns with gotos
152+
* to the label fallthrough.
153+
*/
154+
const newTerminal: LabelTerminal = {
155+
block: body.loweredFunc.func.body.entry,
156+
id: makeInstructionId(0),
157+
kind: 'label',
158+
fallthrough: continuationBlockId,
159+
loc: block.terminal.loc,
160+
};
161+
block.terminal = newTerminal;
156162

157-
// We store the result in the IIFE temporary
158-
const result = instr.lvalue;
163+
// We store the result in the IIFE temporary
164+
const result = instr.lvalue;
159165

160-
// Declare the IIFE temporary
161-
declareTemporary(fn.env, block, result);
166+
// Declare the IIFE temporary
167+
declareTemporary(fn.env, block, result);
162168

163-
// Promote the temporary with a name as we require this to persist
164-
promoteTemporary(result.identifier);
169+
// Promote the temporary with a name as we require this to persist
170+
promoteTemporary(result.identifier);
165171

166-
/*
167-
* Rewrite blocks from the lambda to replace any `return` with a
168-
* store to the result and `goto` the continuation block
169-
*/
170-
for (const [id, block] of body.loweredFunc.func.body.blocks) {
171-
block.preds.clear();
172-
rewriteBlock(fn.env, block, continuationBlockId, result);
173-
fn.body.blocks.set(id, block);
174-
}
172+
/*
173+
* Rewrite blocks from the lambda to replace any `return` with a
174+
* store to the result and `goto` the continuation block
175+
*/
176+
for (const [id, block] of body.loweredFunc.func.body.blocks) {
177+
block.preds.clear();
178+
rewriteBlock(fn.env, block, continuationBlockId, result);
179+
fn.body.blocks.set(id, block);
180+
}
175181

176-
/*
177-
* Ensure we visit the continuation block, since there may have been
178-
* sequential IIFEs that need to be visited.
179-
*/
180-
queue.push(continuationBlock);
181-
continue queue;
182-
}
183-
default: {
184-
for (const place of eachInstructionValueOperand(instr.value)) {
185-
// Any other use of a function expression means it isn't an IIFE
186-
functions.delete(place.identifier.id);
182+
/*
183+
* Ensure we visit the continuation block, since there may have been
184+
* sequential IIFEs that need to be visited.
185+
*/
186+
queue.push(continuationBlock);
187+
continue queue;
188+
}
189+
default: {
190+
for (const place of eachInstructionValueOperand(instr.value)) {
191+
// Any other use of a function expression means it isn't an IIFE
192+
functions.delete(place.identifier.id);
193+
}
187194
}
188195
}
189196
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-iife-inline-ternary.expect.md

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

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useMemo-with-optional.expect.md

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

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useMemo-with-optional.js

Lines changed: 0 additions & 7 deletions
This file was deleted.
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
2+
## Input
3+
4+
```javascript
5+
function Component(props) {
6+
const x = props.foo
7+
? 1
8+
: (() => {
9+
throw new Error('Did not receive 1');
10+
})();
11+
return items;
12+
}
13+
14+
export const FIXTURE_ENTRYPOINT = {
15+
fn: Component,
16+
params: [{foo: true}],
17+
};
18+
19+
```
20+
21+
## Code
22+
23+
```javascript
24+
function Component(props) {
25+
props.foo ? 1 : _temp();
26+
return items;
27+
}
28+
function _temp() {
29+
throw new Error("Did not receive 1");
30+
}
31+
32+
export const FIXTURE_ENTRYPOINT = {
33+
fn: Component,
34+
params: [{ foo: true }],
35+
};
36+
37+
```
38+
39+
### Eval output
40+
(kind: exception) items is not defined
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,8 @@ function Component(props) {
66
})();
77
return items;
88
}
9+
10+
export const FIXTURE_ENTRYPOINT = {
11+
fn: Component,
12+
params: [{foo: true}],
13+
};
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
2+
## Input
3+
4+
```javascript
5+
import {useMemo} from 'react';
6+
function Component(props) {
7+
return (
8+
useMemo(() => {
9+
return [props.value];
10+
}) || []
11+
);
12+
}
13+
14+
export const FIXTURE_ENTRYPOINT = {
15+
fn: Component,
16+
params: [{value: 1}],
17+
};
18+
19+
```
20+
21+
## Code
22+
23+
```javascript
24+
import { c as _c } from "react/compiler-runtime";
25+
import { useMemo } from "react";
26+
function Component(props) {
27+
const $ = _c(2);
28+
let t0;
29+
if ($[0] !== props.value) {
30+
t0 = (() => [props.value])() || [];
31+
$[0] = props.value;
32+
$[1] = t0;
33+
} else {
34+
t0 = $[1];
35+
}
36+
return t0;
37+
}
38+
39+
export const FIXTURE_ENTRYPOINT = {
40+
fn: Component,
41+
params: [{ value: 1 }],
42+
};
43+
44+
```
45+
46+
### Eval output
47+
(kind: ok) [1]
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import {useMemo} from 'react';
2+
function Component(props) {
3+
return (
4+
useMemo(() => {
5+
return [props.value];
6+
}) || []
7+
);
8+
}
9+
10+
export const FIXTURE_ENTRYPOINT = {
11+
fn: Component,
12+
params: [{value: 1}],
13+
};

0 commit comments

Comments
 (0)