Skip to content

Commit 3af905d

Browse files
committed
[compiler] Fix issue with macro arguments being outlined
Summary: Fixes issue documented by #30435. We change the pipeline order so that outlining comes after tracking macro operands, and any function that is referenced in a macro will now not be outlined. ghstack-source-id: f731ad6 Pull Request resolved: #30587
1 parent aa8469f commit 3af905d

File tree

7 files changed

+89
-43
lines changed

7 files changed

+89
-43
lines changed

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ import {
5656
flattenReactiveLoops,
5757
flattenScopesWithHooksOrUse,
5858
inferReactiveScopeVariables,
59-
memoizeFbtOperandsInSameScope,
59+
memoizeFbtAndMacroOperandsInSameScope,
6060
mergeOverlappingReactiveScopes,
6161
mergeReactiveScopesThatInvalidateTogether,
6262
promoteUsedTemporaries,
@@ -243,8 +243,15 @@ function* runWithEnvironment(
243243
inferReactiveScopeVariables(hir);
244244
yield log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir});
245245

246+
const fbtOperands = memoizeFbtAndMacroOperandsInSameScope(hir);
247+
yield log({
248+
kind: 'hir',
249+
name: 'MemoizeFbtAndMacroOperandsInSameScope',
250+
value: hir,
251+
});
252+
246253
if (env.config.enableFunctionOutlining) {
247-
outlineFunctions(hir);
254+
outlineFunctions(hir, fbtOperands);
248255
yield log({kind: 'hir', name: 'OutlineFunctions', value: hir});
249256
}
250257

@@ -262,13 +269,6 @@ function* runWithEnvironment(
262269
value: hir,
263270
});
264271

265-
const fbtOperands = memoizeFbtOperandsInSameScope(hir);
266-
yield log({
267-
kind: 'hir',
268-
name: 'MemoizeFbtAndMacroOperandsInSameScope',
269-
value: hir,
270-
});
271-
272272
if (env.config.enableReactiveScopesInHIR) {
273273
pruneUnusedLabelsHIR(hir);
274274
yield log({

compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,11 @@ export function parseConfigPragma(pragma: string): EnvironmentConfig {
485485
continue;
486486
}
487487

488+
if (key === 'customMacros' && val) {
489+
maybeConfig[key] = [val];
490+
continue;
491+
}
492+
488493
if (typeof defaultConfig[key as keyof EnvironmentConfig] !== 'boolean') {
489494
// skip parsing non-boolean properties
490495
continue;

compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,30 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
import {HIRFunction} from '../HIR';
8+
import {HIRFunction, IdentifierId} from '../HIR';
99

10-
export function outlineFunctions(fn: HIRFunction): void {
10+
export function outlineFunctions(
11+
fn: HIRFunction,
12+
fbtOperands: Set<IdentifierId>,
13+
): void {
1114
for (const [, block] of fn.body.blocks) {
1215
for (const instr of block.instructions) {
13-
const {value} = instr;
16+
const {value, lvalue} = instr;
1417

1518
if (
1619
value.kind === 'FunctionExpression' ||
1720
value.kind === 'ObjectMethod'
1821
) {
1922
// Recurse in case there are inner functions which can be outlined
20-
outlineFunctions(value.loweredFunc.func);
23+
outlineFunctions(value.loweredFunc.func, fbtOperands);
2124
}
22-
2325
if (
2426
value.kind === 'FunctionExpression' &&
2527
value.loweredFunc.dependencies.length === 0 &&
2628
value.loweredFunc.func.context.length === 0 &&
2729
// TODO: handle outlining named functions
28-
value.loweredFunc.func.id === null
30+
value.loweredFunc.func.id === null &&
31+
!fbtOperands.has(lvalue.identifier.id)
2932
) {
3033
const loweredFunc = value.loweredFunc.func;
3134

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export {extractScopeDeclarationsFromDestructuring} from './ExtractScopeDeclarati
1616
export {flattenReactiveLoops} from './FlattenReactiveLoops';
1717
export {flattenScopesWithHooksOrUse} from './FlattenScopesWithHooksOrUse';
1818
export {inferReactiveScopeVariables} from './InferReactiveScopeVariables';
19-
export {memoizeFbtAndMacroOperandsInSameScope as memoizeFbtOperandsInSameScope} from './MemoizeFbtAndMacroOperandsInSameScope';
19+
export {memoizeFbtAndMacroOperandsInSameScope} from './MemoizeFbtAndMacroOperandsInSameScope';
2020
export {mergeOverlappingReactiveScopes} from './MergeOverlappingReactiveScopes';
2121
export {mergeReactiveScopesThatInvalidateTogether} from './MergeReactiveScopesThatInvalidateTogether';
2222
export {printReactiveFunction} from './PrintReactiveFunction';

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.idx-outlining.expect.md

Lines changed: 0 additions & 27 deletions
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @customMacros(idx)
6+
import idx from 'idx';
7+
8+
function Component(props) {
9+
// the lambda should not be outlined
10+
const groupName = idx(props, _ => _.group.label);
11+
return <div>{groupName}</div>;
12+
}
13+
14+
export const FIXTURE_ENTRYPOINT = {
15+
fn: Component,
16+
params: [{}],
17+
};
18+
19+
```
20+
21+
## Code
22+
23+
```javascript
24+
import { c as _c } from "react/compiler-runtime"; // @customMacros(idx)
25+
26+
function Component(props) {
27+
var _ref2;
28+
const $ = _c(4);
29+
let t0;
30+
if ($[0] !== props) {
31+
var _ref;
32+
33+
t0 =
34+
(_ref = props) != null
35+
? (_ref = _ref.group) != null
36+
? _ref.label
37+
: _ref
38+
: _ref;
39+
$[0] = props;
40+
$[1] = t0;
41+
} else {
42+
t0 = $[1];
43+
}
44+
const groupName = t0;
45+
let t1;
46+
if ($[2] !== groupName) {
47+
t1 = <div>{groupName}</div>;
48+
$[2] = groupName;
49+
$[3] = t1;
50+
} else {
51+
t1 = $[3];
52+
}
53+
return t1;
54+
}
55+
56+
export const FIXTURE_ENTRYPOINT = {
57+
fn: Component,
58+
params: [{}],
59+
};
60+
61+
```
62+
63+
### Eval output
64+
(kind: ok) <div></div>
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// @customMacros(idx)
12
import idx from 'idx';
23

34
function Component(props) {

0 commit comments

Comments
 (0)