Skip to content

Commit 7aa44d0

Browse files
josephsavonajosephsavona
committed
[compiler][newinference] Fixes for transitive function capturing, mutation via property loads
ghstack-source-id: 02d13be Pull Request resolved: #33430
1 parent bf1bf9c commit 7aa44d0

File tree

6 files changed

+170
-31
lines changed

6 files changed

+170
-31
lines changed

‎compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,12 @@ function lowerWithMutationAliasing(fn: HIRFunction): void {
6666
inferMutationAliasingRanges(fn);
6767
rewriteInstructionKindsBasedOnReassignment(fn);
6868
inferReactiveScopeVariables(fn);
69+
const effects = inferMutationAliasingFunctionEffects(fn);
6970
fn.env.logger?.debugLogIRs?.({
7071
kind: 'hir',
7172
name: 'AnalyseFunction (inner)',
7273
value: fn,
7374
});
74-
const effects = inferMutationAliasingFunctionEffects(fn);
7575
if (effects != null) {
7676
fn.aliasingEffects ??= [];
7777
fn.aliasingEffects?.push(...effects);

‎compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ export function inferMutationAliasingEffects(
155155
}
156156
queue(fn.body.entry, initialState);
157157

158-
const context = new Context(isFunctionExpression);
158+
const context = new Context(isFunctionExpression, fn);
159159

160160
let count = 0;
161161
while (queuedStates.size !== 0) {
@@ -193,9 +193,11 @@ class Context {
193193
new Map();
194194
catchHandlers: Map<BlockId, Place> = new Map();
195195
isFuctionExpression: boolean;
196+
fn: HIRFunction;
196197

197-
constructor(isFunctionExpression: boolean) {
198+
constructor(isFunctionExpression: boolean, fn: HIRFunction) {
198199
this.isFuctionExpression = isFunctionExpression;
200+
this.fn = fn;
199201
}
200202
}
201203

@@ -309,8 +311,14 @@ function applySignature(
309311
) {
310312
const aliasingEffects =
311313
instruction.value.loweredFunc.func.aliasingEffects ?? [];
314+
const context = new Set(
315+
instruction.value.loweredFunc.func.context.map(p => p.identifier.id),
316+
);
312317
for (const effect of aliasingEffects) {
313318
if (effect.kind === 'Mutate' || effect.kind === 'MutateTransitive') {
319+
if (!context.has(effect.value.identifier.id)) {
320+
continue;
321+
}
314322
const value = state.kind(effect.value);
315323
switch (value.kind) {
316324
case ValueKind.Global:
@@ -458,7 +466,7 @@ function applyEffect(
458466
default: {
459467
effects.push({
460468
// OK: recording information flow
461-
kind: 'Alias',
469+
kind: 'CreateFrom', // prev Alias
462470
from: effect.from,
463471
into: effect.into,
464472
});
@@ -467,6 +475,7 @@ function applyEffect(
467475
break;
468476
}
469477
case 'CreateFunction': {
478+
effects.push(effect);
470479
const isMutable = effect.captures.some(capture => {
471480
switch (state.kind(capture).kind) {
472481
case ValueKind.Context:
@@ -644,6 +653,13 @@ function applyEffect(
644653
if (DEBUG) {
645654
console.log('apply function expression effects');
646655
}
656+
applyEffect(
657+
context,
658+
state,
659+
{kind: 'MutateTransitiveConditionally', value: effect.function},
660+
aliased,
661+
effects,
662+
);
647663
for (const signatureEffect of signatureEffects) {
648664
applyEffect(context, state, signatureEffect, aliased, effects);
649665
}
@@ -1587,10 +1603,26 @@ function computeSignatureForInstruction(
15871603
break;
15881604
}
15891605
case 'StoreGlobal': {
1590-
CompilerError.throwTodo({
1591-
reason: `Handle StoreGlobal in new inference`,
1592-
loc: instr.loc,
1606+
effects.push({
1607+
kind: 'MutateGlobal',
1608+
place: value.value,
1609+
error: {
1610+
severity: ErrorSeverity.InvalidReact,
1611+
reason: getWriteErrorReason({
1612+
kind: ValueKind.Global,
1613+
reason: new Set([ValueReason.Global]),
1614+
context: new Set(),
1615+
}),
1616+
description:
1617+
value.value.identifier.name !== null &&
1618+
value.value.identifier.name.kind === 'named'
1619+
? `Found mutation of \`${value.value.identifier.name}\``
1620+
: null,
1621+
loc: value.value.loc,
1622+
suggestions: null,
1623+
},
15931624
});
1625+
break;
15941626
}
15951627
case 'TypeCastExpression': {
15961628
effects.push({kind: 'Assign', from: value.value, into: lvalue});

‎compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingFunctionEffects.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ export function inferMutationAliasingFunctionEffects(
2020
*/
2121
const tracked = new Map<IdentifierId, Place>();
2222
tracked.set(fn.returns.identifier.id, fn.returns);
23+
for (const operand of [...fn.context, ...fn.params]) {
24+
const place = operand.kind === 'Identifier' ? operand : operand.place;
25+
tracked.set(place.identifier.id, place);
26+
}
2327

2428
/**
2529
* Track capturing/aliasing of context vars and params into each other and into the return.
@@ -95,6 +99,13 @@ export function inferMutationAliasingFunctionEffects(
9599
).push(...from);
96100
}
97101
}
102+
} else if (
103+
effect.kind === 'Create' ||
104+
effect.kind === 'CreateFunction'
105+
) {
106+
getOrInsertDefault(dataFlow, effect.into.identifier.id, [
107+
{kind: 'Alias', place: effect.into},
108+
]);
98109
} else if (
99110
effect.kind === 'MutateFrozen' ||
100111
effect.kind === 'MutateGlobal'
@@ -121,6 +132,12 @@ export function inferMutationAliasingFunctionEffects(
121132
continue;
122133
}
123134
for (const aliased of from) {
135+
if (
136+
aliased.place.identifier.id === input.identifier.id ||
137+
!tracked.has(aliased.place.identifier.id)
138+
) {
139+
continue;
140+
}
124141
const effect = {kind: aliased.kind, from: aliased.place, into: input};
125142
effects.push(effect);
126143
if (
@@ -133,7 +150,7 @@ export function inferMutationAliasingFunctionEffects(
133150
}
134151
// TODO: more precise return effect inference
135152
if (!hasReturn) {
136-
effects.push({
153+
effects.unshift({
137154
kind: 'Create',
138155
into: fn.returns,
139156
value:
@@ -143,6 +160,9 @@ export function inferMutationAliasingFunctionEffects(
143160
});
144161
}
145162

163+
// console.log(pretty(tracked));
164+
// console.log(pretty(dataFlow));
165+
// console.log('FunctionEffects', effects.map(printAliasingEffect).join('\n'));
146166
return effects;
147167
}
148168

‎compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts‎

Lines changed: 63 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,9 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
107107
for (const effect of instr.effects) {
108108
if (effect.kind === 'Create' || effect.kind === 'CreateFunction') {
109109
state.create(effect.into);
110-
} else if (
111-
effect.kind === 'Assign' ||
112-
effect.kind === 'Alias' ||
113-
effect.kind === 'CreateFrom'
114-
) {
110+
} else if (effect.kind === 'CreateFrom') {
111+
state.createFrom(index++, effect.from, effect.into);
112+
} else if (effect.kind === 'Assign' || effect.kind === 'Alias') {
115113
state.assign(index++, effect.from, effect.into);
116114
} else if (effect.kind === 'Capture') {
117115
state.capture(index++, effect.from, effect.into);
@@ -181,7 +179,7 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
181179
for (const mutation of mutations) {
182180
state.mutate(
183181
mutation.index,
184-
mutation.place,
182+
mutation.place.identifier,
185183
makeInstructionId(mutation.id + 1),
186184
mutation.transitive,
187185
mutation.kind,
@@ -191,8 +189,9 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
191189
console.log(state.debug());
192190
}
193191
fn.aliasingEffects ??= [];
194-
for (const param of fn.context) {
195-
const node = state.nodes.get(param.identifier);
192+
for (const param of [...fn.context, ...fn.params]) {
193+
const place = param.kind === 'Identifier' ? param : param.place;
194+
const node = state.nodes.get(place.identifier);
196195
if (node == null) {
197196
continue;
198197
}
@@ -201,30 +200,30 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
201200
mutated = true;
202201
fn.aliasingEffects.push({
203202
kind: 'MutateConditionally',
204-
value: param,
203+
value: place,
205204
});
206205
} else if (node.local === MutationKind.Definite) {
207206
mutated = true;
208207
fn.aliasingEffects.push({
209208
kind: 'Mutate',
210-
value: param,
209+
value: place,
211210
});
212211
}
213212
if (node.transitive === MutationKind.Conditional) {
214213
mutated = true;
215214
fn.aliasingEffects.push({
216215
kind: 'MutateTransitiveConditionally',
217-
value: param,
216+
value: place,
218217
});
219218
} else if (node.transitive === MutationKind.Definite) {
220219
mutated = true;
221220
fn.aliasingEffects.push({
222221
kind: 'MutateTransitive',
223-
value: param,
222+
value: place,
224223
});
225224
}
226225
if (mutated) {
227-
param.effect = Effect.Capture;
226+
place.effect = Effect.Capture;
228227
}
229228
}
230229

@@ -247,6 +246,21 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
247246
? Effect.Capture
248247
: Effect.Read;
249248
}
249+
if (
250+
isPhiMutatedAfterCreation &&
251+
phi.place.identifier.mutableRange.start === 0
252+
) {
253+
/*
254+
* TODO: ideally we'd construct a precise start range, but what really
255+
* matters is that the phi's range appears mutable (end > start + 1)
256+
* so we just set the start to the previous instruction before this block
257+
*/
258+
const firstInstructionIdOfBlock =
259+
block.instructions.at(0)?.id ?? block.terminal.id;
260+
phi.place.identifier.mutableRange.start = makeInstructionId(
261+
firstInstructionIdOfBlock - 1,
262+
);
263+
}
250264
}
251265
for (const instr of block.instructions) {
252266
for (const lvalue of eachInstructionLValue(instr)) {
@@ -357,6 +371,7 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
357371

358372
type Node = {
359373
id: Identifier;
374+
createdFrom: Map<Identifier, number>;
360375
captures: Map<Identifier, number>;
361376
aliases: Map<Identifier, number>;
362377
edges: Array<{index: number; node: Identifier; kind: 'capture' | 'alias'}>;
@@ -369,6 +384,7 @@ class AliasingState {
369384
create(place: Place): void {
370385
this.nodes.set(place.identifier, {
371386
id: place.identifier,
387+
createdFrom: new Map(),
372388
captures: new Map(),
373389
aliases: new Map(),
374390
edges: [],
@@ -377,6 +393,24 @@ class AliasingState {
377393
});
378394
}
379395

396+
createFrom(index: number, from: Place, into: Place): void {
397+
this.create(into);
398+
const fromNode = this.nodes.get(from.identifier);
399+
const toNode = this.nodes.get(into.identifier);
400+
if (fromNode == null || toNode == null) {
401+
if (DEBUG) {
402+
console.log(
403+
`skip: createFrom ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`,
404+
);
405+
}
406+
return;
407+
}
408+
fromNode.edges.push({index, node: into.identifier, kind: 'alias'});
409+
if (!toNode.createdFrom.has(from.identifier)) {
410+
toNode.createdFrom.set(from.identifier, index);
411+
}
412+
}
413+
380414
capture(index: number, from: Place, into: Place): void {
381415
const fromNode = this.nodes.get(from.identifier);
382416
const toNode = this.nodes.get(into.identifier);
@@ -406,22 +440,22 @@ class AliasingState {
406440
return;
407441
}
408442
fromNode.edges.push({index, node: into.identifier, kind: 'alias'});
409-
if (!toNode.captures.has(from.identifier)) {
443+
if (!toNode.aliases.has(from.identifier)) {
410444
toNode.aliases.set(from.identifier, index);
411445
}
412446
}
413447

414448
mutate(
415449
index: number,
416-
start: Place,
450+
start: Identifier,
417451
end: InstructionId,
418452
transitive: boolean,
419453
kind: MutationKind,
420454
): void {
421455
const seen = new Set<Identifier>();
422-
const queue = [start.identifier];
456+
const queue = [{place: start, transitive}];
423457
while (queue.length !== 0) {
424-
const current = queue.pop()!;
458+
const {place: current, transitive} = queue.pop()!;
425459
if (seen.has(current)) {
426460
continue;
427461
}
@@ -430,14 +464,14 @@ class AliasingState {
430464
if (node == null) {
431465
if (DEBUG) {
432466
console.log(
433-
`no node! ${printPlace(start)} for identifier ${printIdentifier(current)}`,
467+
`no node! ${printIdentifier(start)} for identifier ${printIdentifier(current)}`,
434468
);
435469
}
436470
continue;
437471
}
438472
if (DEBUG) {
439473
console.log(
440-
`mutate $${node.id.id} via ${printPlace(start)} at [${end}]`,
474+
`mutate $${node.id.id} via ${printIdentifier(start)} at [${end}]`,
441475
);
442476
}
443477
node.id.mutableRange.end = makeInstructionId(
@@ -457,7 +491,13 @@ class AliasingState {
457491
if (edge.index >= index) {
458492
break;
459493
}
460-
queue.push(edge.node);
494+
queue.push({place: edge.node, transitive});
495+
}
496+
for (const [alias, when] of node.createdFrom) {
497+
if (when >= index) {
498+
continue;
499+
}
500+
queue.push({place: alias, transitive: true});
461501
}
462502
/**
463503
* all mutations affect backward alias edges by the rules:
@@ -468,7 +508,7 @@ class AliasingState {
468508
if (when >= index) {
469509
continue;
470510
}
471-
queue.push(alias);
511+
queue.push({place: alias, transitive});
472512
}
473513
/**
474514
* but only transitive mutations affect captures
@@ -478,7 +518,7 @@ class AliasingState {
478518
if (when >= index) {
479519
continue;
480520
}
481-
queue.push(capture);
521+
queue.push({place: capture, transitive});
482522
}
483523
}
484524
}
@@ -489,7 +529,7 @@ class AliasingState {
489529
}
490530
}
491531

492-
function pretty(v: any): string {
532+
export function pretty(v: any): string {
493533
return prettyFormat(v, {
494534
plugins: [
495535
{

0 commit comments

Comments
 (0)