Skip to content

Commit a44efd5

Browse files
committed
[compiler] Switch to track setStates by aliasing and id instead of identifier names
Summary: This makes the setState usage logic much more robust. We no longer rely on identifierName. Now we track when a setState is loaded into a new promoted identifier variable and track this in a map `setStateLoaded` map. For other types of instructions we consider the setState to be being used. In this case we record its usage into the `setStateUsages` map. Test Plan: We expect no changes in behavior for the current tests
1 parent 9548e2e commit a44efd5

File tree

1 file changed

+88
-41
lines changed

1 file changed

+88
-41
lines changed

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts

Lines changed: 88 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import {
2121
isUseStateType,
2222
BasicBlock,
2323
isUseRefType,
24-
GeneratedSource,
2524
SourceLocation,
2625
} from '../HIR';
2726
import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors';
@@ -41,8 +40,8 @@ type ValidationContext = {
4140
readonly errors: CompilerError;
4241
readonly derivationCache: DerivationCache;
4342
readonly effects: Set<HIRFunction>;
44-
readonly setStateCache: Map<string | undefined | null, Array<Place>>;
45-
readonly effectSetStateCache: Map<string | undefined | null, Array<Place>>;
43+
readonly setStateLoads: Map<IdentifierId, IdentifierId | null>;
44+
readonly setStateUsages: Map<IdentifierId, Set<SourceLocation>>;
4645
};
4746

4847
class DerivationCache {
@@ -182,19 +181,16 @@ export function validateNoDerivedComputationsInEffects_exp(
182181
const errors = new CompilerError();
183182
const effects: Set<HIRFunction> = new Set();
184183

185-
const setStateCache: Map<string | undefined | null, Array<Place>> = new Map();
186-
const effectSetStateCache: Map<
187-
string | undefined | null,
188-
Array<Place>
189-
> = new Map();
184+
const setStateLoads: Map<IdentifierId, IdentifierId> = new Map();
185+
const setStateUsages: Map<IdentifierId, Set<SourceLocation>> = new Map();
190186

191187
const context: ValidationContext = {
192188
functions,
193189
errors,
194190
derivationCache,
195191
effects,
196-
setStateCache,
197-
effectSetStateCache,
192+
setStateLoads,
193+
setStateUsages,
198194
};
199195

200196
if (fn.fnType === 'Hook') {
@@ -284,11 +280,60 @@ function joinValue(
284280
return 'fromPropsAndState';
285281
}
286282

283+
function getRootSetState(
284+
key: IdentifierId,
285+
loads: Map<IdentifierId, IdentifierId | null>,
286+
visited: Set<IdentifierId> = new Set(),
287+
): IdentifierId | null {
288+
if (visited.has(key)) {
289+
return null;
290+
}
291+
visited.add(key);
292+
293+
const parentId = loads.get(key);
294+
295+
if (parentId === undefined) {
296+
return null;
297+
}
298+
299+
if (parentId === null) {
300+
return key;
301+
}
302+
303+
return getRootSetState(parentId, loads, visited);
304+
}
305+
306+
function maybeRecordSetState(
307+
instr: Instruction,
308+
loads: Map<IdentifierId, IdentifierId | null>,
309+
usages: Map<IdentifierId, Set<SourceLocation>>,
310+
): void {
311+
for (const operand of eachInstructionLValue(instr)) {
312+
if (isSetStateType(operand.identifier)) {
313+
if (instr.value.kind === 'LoadLocal') {
314+
loads.set(operand.identifier.id, instr.value.place.identifier.id);
315+
} else {
316+
// this is a root setState
317+
loads.set(operand.identifier.id, null);
318+
}
319+
320+
const rootSetState = getRootSetState(operand.identifier.id, loads);
321+
if (rootSetState !== null && usages.get(rootSetState) === undefined) {
322+
usages.set(rootSetState, new Set([operand.loc]));
323+
}
324+
}
325+
}
326+
}
327+
287328
function recordInstructionDerivations(
288329
instr: Instruction,
289330
context: ValidationContext,
290331
isFirstPass: boolean,
291332
): void {
333+
if (isFirstPass) {
334+
maybeRecordSetState(instr, context.setStateLoads, context.setStateUsages);
335+
}
336+
292337
let typeOfValue: TypeOfValue = 'ignored';
293338
const sources: Set<IdentifierId> = new Set();
294339
const {lvalue, value} = instr;
@@ -323,15 +368,13 @@ function recordInstructionDerivations(
323368
}
324369

325370
for (const operand of eachInstructionOperand(instr)) {
326-
if (
327-
isSetStateType(operand.identifier) &&
328-
operand.loc !== GeneratedSource &&
329-
isFirstPass
330-
) {
331-
if (context.setStateCache.has(operand.loc.identifierName)) {
332-
context.setStateCache.get(operand.loc.identifierName)!.push(operand);
333-
} else {
334-
context.setStateCache.set(operand.loc.identifierName, [operand]);
371+
if (isSetStateType(operand.identifier) && isFirstPass) {
372+
const rootSetStateId = getRootSetState(
373+
operand.identifier.id,
374+
context.setStateLoads,
375+
);
376+
if (rootSetStateId !== null) {
377+
context.setStateUsages.get(rootSetStateId)?.add(operand.loc);
335378
}
336379
}
337380

@@ -469,11 +512,16 @@ function validateEffect(
469512

470513
const effectDerivedSetStateCalls: Array<{
471514
value: CallExpression;
472-
loc: SourceLocation;
515+
id: IdentifierId;
473516
sourceIds: Set<IdentifierId>;
474517
typeOfValue: TypeOfValue;
475518
}> = [];
476519

520+
const effectSetStateUsages: Map<
521+
IdentifierId,
522+
Set<SourceLocation>
523+
> = new Map();
524+
477525
const globals: Set<IdentifierId> = new Set();
478526
for (const block of effectFunction.body.blocks.values()) {
479527
for (const pred of block.preds) {
@@ -489,19 +537,16 @@ function validateEffect(
489537
return;
490538
}
491539

540+
maybeRecordSetState(instr, context.setStateLoads, effectSetStateUsages);
541+
492542
for (const operand of eachInstructionOperand(instr)) {
493-
if (
494-
isSetStateType(operand.identifier) &&
495-
operand.loc !== GeneratedSource
496-
) {
497-
if (context.effectSetStateCache.has(operand.loc.identifierName)) {
498-
context.effectSetStateCache
499-
.get(operand.loc.identifierName)!
500-
.push(operand);
501-
} else {
502-
context.effectSetStateCache.set(operand.loc.identifierName, [
503-
operand,
504-
]);
543+
if (isSetStateType(operand.identifier)) {
544+
const rootSetStateId = getRootSetState(
545+
operand.identifier.id,
546+
context.setStateLoads,
547+
);
548+
if (rootSetStateId !== null) {
549+
effectSetStateUsages.get(rootSetStateId)?.add(operand.loc);
505550
}
506551
}
507552
}
@@ -519,7 +564,7 @@ function validateEffect(
519564
if (argMetadata !== undefined) {
520565
effectDerivedSetStateCalls.push({
521566
value: instr.value,
522-
loc: instr.value.callee.loc,
567+
id: instr.value.callee.identifier.id,
523568
sourceIds: argMetadata.sourcesIds,
524569
typeOfValue: argMetadata.typeOfValue,
525570
});
@@ -553,15 +598,17 @@ function validateEffect(
553598
}
554599

555600
for (const derivedSetStateCall of effectDerivedSetStateCalls) {
601+
const rootSetStateCall = getRootSetState(
602+
derivedSetStateCall.id,
603+
context.setStateLoads,
604+
);
605+
556606
if (
557-
derivedSetStateCall.loc !== GeneratedSource &&
558-
context.effectSetStateCache.has(derivedSetStateCall.loc.identifierName) &&
559-
context.setStateCache.has(derivedSetStateCall.loc.identifierName) &&
560-
context.effectSetStateCache.get(derivedSetStateCall.loc.identifierName)!
561-
.length ===
562-
context.setStateCache.get(derivedSetStateCall.loc.identifierName)!
563-
.length -
564-
1
607+
rootSetStateCall !== null &&
608+
effectSetStateUsages.has(rootSetStateCall) &&
609+
context.setStateUsages.has(rootSetStateCall) &&
610+
effectSetStateUsages.get(rootSetStateCall)!.size ===
611+
context.setStateUsages.get(rootSetStateCall)!.size - 1
565612
) {
566613
const allSourceIds = Array.from(derivedSetStateCall.sourceIds);
567614
const trees = allSourceIds

0 commit comments

Comments
 (0)