Skip to content

Commit 60d8044

Browse files
josephsavonajosephsavona
committed
[compiler] Further improve new range inference
Further refine the abstract interpretation for InferMutationAliasingRanges to account for forward data flow: Alias/Capture a -> b, mutate(a) => mutate(b) ie mutation affects aliases of the place being mutated, not just things that place aliased. Fixes inference of which context vars of a function mutate, using the precise inference from the Range pass. Adds MutateFrozen and MutateGlobal effects but they're not fully hooked up yet. ghstack-source-id: 0d83b1a Pull Request resolved: #33411
1 parent 6c69f01 commit 60d8044

12 files changed

+685
-280
lines changed

‎compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,14 @@ export class CompilerErrorDetail {
115115
export class CompilerError extends Error {
116116
details: Array<CompilerErrorDetail> = [];
117117

118+
static from(details: Array<CompilerErrorDetailOptions>): CompilerError {
119+
const error = new CompilerError();
120+
for (const detail of details) {
121+
error.push(detail);
122+
}
123+
return error;
124+
}
125+
118126
static invariant(
119127
condition: unknown,
120128
options: Omit<CompilerErrorDetailOptions, 'severity'>,

‎compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,6 +1003,12 @@ export function printAliasingEffect(effect: AliasingEffect): string {
10031003
case 'MutateTransitiveConditionally': {
10041004
return `${effect.kind} ${printPlaceForAliasEffect(effect.value)}`;
10051005
}
1006+
case 'MutateFrozen': {
1007+
return `MutateFrozen ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
1008+
}
1009+
case 'MutateGlobal': {
1010+
return `MutateGlobal ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
1011+
}
10061012
default: {
10071013
assertExhaustive(effect, `Unexpected kind '${(effect as any).kind}'`);
10081014
}

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
Place,
1616
isRefOrRefValue,
1717
makeInstructionId,
18+
printFunction,
1819
} from '../HIR';
1920
import {deadCodeElimination} from '../Optimization';
2021
import {inferReactiveScopeVariables} from '../ReactiveScopes';
@@ -72,7 +73,10 @@ function lowerWithMutationAliasing(fn: HIRFunction): void {
7273
value: fn,
7374
});
7475
const effects = inferMutationAliasingFunctionEffects(fn);
75-
fn.aliasingEffects = effects;
76+
if (effects != null) {
77+
fn.aliasingEffects ??= [];
78+
fn.aliasingEffects?.push(...effects);
79+
}
7680

7781
const capturedOrMutated = new Set<IdentifierId>();
7882
for (const effect of effects ?? []) {
@@ -97,6 +101,8 @@ function lowerWithMutationAliasing(fn: HIRFunction): void {
97101
capturedOrMutated.add(effect.value.identifier.id);
98102
break;
99103
}
104+
case 'MutateFrozen':
105+
case 'MutateGlobal':
100106
case 'CreateFunction':
101107
case 'Create':
102108
case 'Freeze':
@@ -114,7 +120,10 @@ function lowerWithMutationAliasing(fn: HIRFunction): void {
114120
}
115121

116122
for (const operand of fn.context) {
117-
if (capturedOrMutated.has(operand.identifier.id)) {
123+
if (
124+
capturedOrMutated.has(operand.identifier.id) ||
125+
operand.effect === Effect.Capture
126+
) {
118127
operand.effect = Effect.Capture;
119128
} else {
120129
operand.effect = Effect.Read;

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

Lines changed: 111 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,13 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
import {CompilerError, Effect, ValueKind} from '..';
8+
import {
9+
CompilerError,
10+
CompilerErrorDetailOptions,
11+
Effect,
12+
ErrorSeverity,
13+
ValueKind,
14+
} from '..';
915
import {
1016
BasicBlock,
1117
BlockId,
@@ -214,11 +220,6 @@ function inferBlock(
214220
instructionSignature = computeSignatureForInstruction(state.env, instr);
215221
instructionSignatureCache.set(instr, instructionSignature);
216222
}
217-
/*
218-
* console.log(
219-
* printInstruction({...instr, effects: [...instructionSignature.effects]}),
220-
* );
221-
*/
222223
const effects = applySignature(
223224
state,
224225
instructionSignature,
@@ -244,6 +245,7 @@ function applySignature(
244245
instruction: Instruction,
245246
effectInstructionValueCache: Map<AliasingEffect, InstructionValue>,
246247
): Array<AliasingEffect> | null {
248+
const effects: Array<AliasingEffect> = [];
247249
/**
248250
* For function instructions, eagerly validate that they aren't mutating
249251
* a known-frozen value.
@@ -260,27 +262,33 @@ function applySignature(
260262
for (const effect of aliasingEffects) {
261263
if (effect.kind === 'Mutate' || effect.kind === 'MutateTransitive') {
262264
const value = state.kind(effect.value);
263-
if (value.kind === ValueKind.Frozen) {
264-
if (DEBUG) {
265-
console.log(printInstruction(instruction));
266-
console.log(printAliasingEffect(effect));
267-
console.log(prettyFormat(state.debugAbstractValue(value)));
265+
switch (value.kind) {
266+
case ValueKind.Global:
267+
case ValueKind.Frozen: {
268+
const reason = getWriteErrorReason({
269+
kind: value.kind,
270+
reason: value.reason,
271+
context: new Set(),
272+
});
273+
effects.push({
274+
kind:
275+
value.kind === ValueKind.Frozen
276+
? 'MutateFrozen'
277+
: 'MutateGlobal',
278+
place: effect.value,
279+
error: {
280+
severity: ErrorSeverity.InvalidReact,
281+
reason,
282+
description:
283+
effect.value.identifier.name !== null &&
284+
effect.value.identifier.name.kind === 'named'
285+
? `Found mutation of \`${effect.value.identifier.name}\``
286+
: null,
287+
loc: effect.value.loc,
288+
suggestions: null,
289+
},
290+
});
268291
}
269-
const reason = getWriteErrorReason({
270-
kind: value.kind,
271-
reason: value.reason,
272-
context: new Set(),
273-
});
274-
CompilerError.throwInvalidReact({
275-
reason,
276-
description:
277-
effect.value.identifier.name !== null &&
278-
effect.value.identifier.name.kind === 'named'
279-
? `Found mutation of \`${effect.value.identifier.name}\``
280-
: null,
281-
loc: effect.value.loc,
282-
suggestions: null,
283-
});
284292
}
285293
}
286294
}
@@ -296,7 +304,6 @@ function applySignature(
296304
console.log(printInstruction(instruction));
297305
}
298306

299-
const effects: Array<AliasingEffect> = [];
300307
for (const effect of signature.effects) {
301308
applyEffect(
302309
state,
@@ -429,6 +436,19 @@ function applyEffect(
429436
}
430437
}
431438
});
439+
for (const operand of effect.function.loweredFunc.func.context) {
440+
if (operand.effect !== Effect.Capture) {
441+
continue;
442+
}
443+
const kind = state.kind(operand).kind;
444+
if (
445+
kind === ValueKind.Primitive ||
446+
kind == ValueKind.Frozen ||
447+
kind == ValueKind.Global
448+
) {
449+
operand.effect = Effect.Read;
450+
}
451+
}
432452
state.initialize(effect.function, {
433453
kind: isMutable ? ValueKind.Mutable : ValueKind.Frozen,
434454
reason: new Set([]),
@@ -742,24 +762,36 @@ function applyEffect(
742762
console.log(printAliasingEffect(effect));
743763
console.log(prettyFormat(state.debugAbstractValue(value)));
744764
}
765+
745766
const reason = getWriteErrorReason({
746767
kind: value.kind,
747768
reason: value.reason,
748769
context: new Set(),
749770
});
750-
CompilerError.throwInvalidReact({
751-
reason,
752-
description:
753-
effect.value.identifier.name !== null &&
754-
effect.value.identifier.name.kind === 'named'
755-
? `Found mutation of \`${effect.value.identifier.name}\``
756-
: null,
757-
loc: effect.value.loc,
758-
suggestions: null,
771+
effects.push({
772+
kind:
773+
value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal',
774+
place: effect.value,
775+
error: {
776+
severity: ErrorSeverity.InvalidReact,
777+
reason,
778+
description:
779+
effect.value.identifier.name !== null &&
780+
effect.value.identifier.name.kind === 'named'
781+
? `Found mutation of \`${effect.value.identifier.name}\``
782+
: null,
783+
loc: effect.value.loc,
784+
suggestions: null,
785+
},
759786
});
760787
}
761788
break;
762789
}
790+
case 'MutateFrozen':
791+
case 'MutateGlobal': {
792+
effects.push(effect);
793+
break;
794+
}
763795
default: {
764796
assertExhaustive(
765797
effect,
@@ -933,7 +965,11 @@ class InferenceState {
933965
kind: ValueKind.Frozen,
934966
reason: new Set([reason]),
935967
});
936-
if (value.kind === 'FunctionExpression') {
968+
if (
969+
value.kind === 'FunctionExpression' &&
970+
(this.env.config.enablePreserveExistingMemoizationGuarantees ||
971+
this.env.config.enableTransitivelyFreezeFunctionExpressions)
972+
) {
937973
for (const place of value.loweredFunc.func.context) {
938974
this.freeze(place, reason);
939975
}
@@ -1552,15 +1588,31 @@ function computeSignatureForInstruction(
15521588
});
15531589
break;
15541590
}
1591+
case 'StartMemoize':
1592+
case 'FinishMemoize': {
1593+
if (env.config.enablePreserveExistingMemoizationGuarantees) {
1594+
for (const operand of eachInstructionValueOperand(value)) {
1595+
effects.push({
1596+
kind: 'Freeze',
1597+
value: operand,
1598+
reason: ValueReason.Other,
1599+
});
1600+
}
1601+
}
1602+
effects.push({
1603+
kind: 'Create',
1604+
into: lvalue,
1605+
value: ValueKind.Primitive,
1606+
});
1607+
break;
1608+
}
15551609
case 'TaggedTemplateExpression':
15561610
case 'BinaryExpression':
15571611
case 'Debugger':
1558-
case 'FinishMemoize':
15591612
case 'JSXText':
15601613
case 'MetaProperty':
15611614
case 'Primitive':
15621615
case 'RegExpLiteral':
1563-
case 'StartMemoize':
15641616
case 'TemplateLiteral':
15651617
case 'UnaryExpression':
15661618
case 'UnsupportedNode': {
@@ -1879,6 +1931,14 @@ function computeEffectsForSignature(
18791931
}
18801932
break;
18811933
}
1934+
case 'MutateFrozen':
1935+
case 'MutateGlobal': {
1936+
const values = substitutions.get(effect.place.identifier.id) ?? [];
1937+
for (const value of values) {
1938+
effects.push({kind: effect.kind, place: value, error: effect.error});
1939+
}
1940+
break;
1941+
}
18821942
case 'Mutate':
18831943
case 'MutateTransitive':
18841944
case 'MutateTransitiveConditionally':
@@ -2165,6 +2225,18 @@ export type AliasingEffect =
21652225
captures: Array<Place>;
21662226
function: FunctionExpression | ObjectMethod;
21672227
into: Place;
2228+
}
2229+
/**
2230+
* Mutation of a value known to be immutable
2231+
*/
2232+
| {kind: 'MutateFrozen'; place: Place; error: CompilerErrorDetailOptions}
2233+
/**
2234+
* Mutation of a global
2235+
*/
2236+
| {
2237+
kind: 'MutateGlobal';
2238+
place: Place;
2239+
error: CompilerErrorDetailOptions;
21682240
};
21692241

21702242
export type AliasingSignatureEffect = AliasingEffect;

0 commit comments

Comments
 (0)