Skip to content

Commit 8e65f61

Browse files
committed
[compiler] Switch to track setStates by aliasing and id instead of identifier names
1 parent c770752 commit 8e65f61

File tree

2 files changed

+88
-79
lines changed

2 files changed

+88
-79
lines changed

compiler/apps/playground/yarn.lock

Lines changed: 3 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -854,23 +854,11 @@
854854
resolved "https://registry.yarnpkg.com/@types/node/-/node-18.11.9.tgz#02d013de7058cea16d36168ef2fc653464cfbad4"
855855
integrity sha512-CRpX21/kGdzjOpFsZSkcrXMGIBWMGNIHXXBVFSH+ggkftxg+XYP20TESbh+zFvFj3EQOl5byk0HTRn1IL6hbqg==
856856

857-
858-
version "19.1.9"
859-
resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-19.1.9.tgz#5ab695fce1e804184767932365ae6569c11b4b4b"
860-
integrity sha512-qXRuZaOsAdXKFyOhRBg6Lqqc0yay13vN7KrIg4L7N4aaHN68ma9OK3NE1BoDFgFOTfM7zg+3/8+2n8rLUH3OKQ==
861-
862857
863858
version "19.2.2"
864859
resolved "https://registry.npmjs.org/@types/react-dom/-/react-dom-19.2.2.tgz#a4cc874797b7ddc9cb180ef0d5dc23f596fc2332"
865860
integrity sha512-9KQPoO6mZCi7jcIStSnlOWn2nEF3mNmyr3rIAsGnAbQKYbRLyqmeSc39EVgtxXVia+LMT8j3knZLAZAh+xLmrw==
866861

867-
868-
version "19.1.12"
869-
resolved "https://registry.yarnpkg.com/@types/react/-/react-19.1.12.tgz#7bfaa76aabbb0b4fe0493c21a3a7a93d33e8937b"
870-
integrity sha512-cMoR+FoAf/Jyq6+Df2/Z41jISvGZZ2eTlnsaJRptmZ76Caldwy1odD4xTr/gNV9VLj0AWgg/nmkevIyUfIIq5w==
871-
dependencies:
872-
csstype "^3.0.2"
873-
874862
875863
version "19.2.2"
876864
resolved "https://registry.npmjs.org/@types/react/-/react-19.2.2.tgz#ba123a75d4c2a51158697160a4ea2ff70aa6bf36"
@@ -3982,16 +3970,7 @@ stop-iteration-iterator@^1.1.0:
39823970
es-errors "^1.3.0"
39833971
internal-slot "^1.1.0"
39843972

3985-
"string-width-cjs@npm:string-width@^4.2.0":
3986-
version "4.2.3"
3987-
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
3988-
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
3989-
dependencies:
3990-
emoji-regex "^8.0.0"
3991-
is-fullwidth-code-point "^3.0.0"
3992-
strip-ansi "^6.0.1"
3993-
3994-
string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
3973+
"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
39953974
version "4.2.3"
39963975
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
39973976
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
@@ -4095,14 +4074,7 @@ string.prototype.trimstart@^1.0.8:
40954074
define-properties "^1.2.1"
40964075
es-object-atoms "^1.0.0"
40974076

4098-
"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
4099-
version "6.0.1"
4100-
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
4101-
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
4102-
dependencies:
4103-
ansi-regex "^5.0.1"
4104-
4105-
strip-ansi@^6.0.0, strip-ansi@^6.0.1:
4077+
"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1:
41064078
version "6.0.1"
41074079
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
41084080
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
@@ -4523,16 +4495,7 @@ word-wrap@^1.2.5:
45234495
resolved "https://registry.yarnpkg.com/word-wrap/-/word-wrap-1.2.5.tgz#d2c45c6dd4fbce621a66f136cbe328afd0410b34"
45244496
integrity sha512-BN22B5eaMMI9UMtjrGd5g5eCYPpCPDUy0FJXbYsaT5zYxjFOckS53SQDE3pWkVoWpHXVb3BrYcEN4Twa55B5cA==
45254497

4526-
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
4527-
version "7.0.0"
4528-
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
4529-
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
4530-
dependencies:
4531-
ansi-styles "^4.0.0"
4532-
string-width "^4.1.0"
4533-
strip-ansi "^6.0.0"
4534-
4535-
wrap-ansi@^7.0.0:
4498+
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
45364499
version "7.0.0"
45374500
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
45384501
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==

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

Lines changed: 85 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@ import {
2323
isUseRefType,
2424
GeneratedSource,
2525
SourceLocation,
26+
IdentifierName,
2627
} from '../HIR';
2728
import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors';
2829
import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
2930
import {assertExhaustive} from '../Utils/utils';
31+
import {printInstruction} from '../HIR/PrintHIR';
3032

3133
type TypeOfValue = 'ignored' | 'fromProps' | 'fromState' | 'fromPropsAndState';
3234

@@ -41,8 +43,9 @@ type ValidationContext = {
4143
readonly errors: CompilerError;
4244
readonly derivationCache: DerivationCache;
4345
readonly effects: Set<HIRFunction>;
44-
readonly setStateCache: Map<string | undefined | null, Array<Place>>;
45-
readonly effectSetStateCache: Map<string | undefined | null, Array<Place>>;
46+
// These should replace the setStateCache and effectSetStateCache once we have
47+
readonly setStateLoads: Map<IdentifierId, IdentifierId | null>;
48+
readonly setStateUsages: Map<IdentifierId, Set<SourceLocation>>;
4649
};
4750

4851
class DerivationCache {
@@ -188,13 +191,16 @@ export function validateNoDerivedComputationsInEffects_exp(
188191
Array<Place>
189192
> = new Map();
190193

194+
const setStateLoads: Map<IdentifierId, IdentifierId> = new Map();
195+
const setStateUsages: Map<IdentifierId, Set<SourceLocation>> = new Map();
196+
191197
const context: ValidationContext = {
192198
functions,
193199
errors,
194200
derivationCache,
195201
effects,
196-
setStateCache,
197-
effectSetStateCache,
202+
setStateLoads,
203+
setStateUsages,
198204
};
199205

200206
if (fn.fnType === 'Hook') {
@@ -241,10 +247,7 @@ export function validateNoDerivedComputationsInEffects_exp(
241247
validateEffect(effect, context);
242248
}
243249

244-
console.log(derivationCache.cache);
245-
if (errors.hasAnyErrors()) {
246-
throw errors;
247-
}
250+
return errors.asResult();
248251
}
249252

250253
function recordPhiDerivations(
@@ -287,11 +290,56 @@ function joinValue(
287290
return 'fromPropsAndState';
288291
}
289292

293+
function maybeRecordSetState(
294+
instr: Instruction,
295+
loads: Map<IdentifierId, IdentifierId | null>,
296+
usages: Map<IdentifierId, Set<SourceLocation>>,
297+
) {
298+
for (const operand of eachInstructionLValue(instr)) {
299+
if (isSetStateType(operand.identifier)) {
300+
if (instr.value.kind === 'LoadLocal') {
301+
loads.set(operand.identifier.id, instr.value.place.identifier.id);
302+
} else {
303+
// this is a root setState
304+
loads.set(operand.identifier.id, null);
305+
usages.set(operand.identifier.id, new Set([operand.loc]));
306+
}
307+
}
308+
}
309+
}
310+
311+
function getRootSetState(
312+
key: IdentifierId,
313+
loads: Map<IdentifierId, IdentifierId | null>,
314+
visited: Set<IdentifierId> = new Set(),
315+
): IdentifierId | null {
316+
if (visited.has(key)) {
317+
return null;
318+
}
319+
visited.add(key);
320+
321+
const parentId = loads.get(key);
322+
323+
if (parentId === undefined) {
324+
return null;
325+
}
326+
327+
if (parentId === null) {
328+
return key;
329+
}
330+
331+
return getRootSetState(parentId, loads, visited);
332+
}
333+
290334
function recordInstructionDerivations(
291335
instr: Instruction,
292336
context: ValidationContext,
293337
isFirstPass: boolean,
294338
): void {
339+
if (isFirstPass) {
340+
maybeRecordSetState(instr, context.setStateLoads, context.setStateUsages);
341+
}
342+
295343
let typeOfValue: TypeOfValue = 'ignored';
296344
const sources: Set<IdentifierId> = new Set();
297345
const {lvalue, value} = instr;
@@ -326,15 +374,14 @@ function recordInstructionDerivations(
326374
}
327375

328376
for (const operand of eachInstructionOperand(instr)) {
329-
if (
330-
isSetStateType(operand.identifier) &&
331-
operand.loc !== GeneratedSource &&
332-
isFirstPass
333-
) {
334-
if (context.setStateCache.has(operand.loc.identifierName)) {
335-
context.setStateCache.get(operand.loc.identifierName)!.push(operand);
336-
} else {
337-
context.setStateCache.set(operand.loc.identifierName, [operand]);
377+
if (isSetStateType(operand.identifier) && isFirstPass) {
378+
// this is a root setState
379+
const rootSetStateId = getRootSetState(
380+
operand.identifier.id,
381+
context.setStateLoads,
382+
);
383+
if (rootSetStateId !== null) {
384+
context.setStateUsages.get(rootSetStateId)?.add(operand.loc);
338385
}
339386
}
340387

@@ -472,11 +519,17 @@ function validateEffect(
472519

473520
const effectDerivedSetStateCalls: Array<{
474521
value: CallExpression;
475-
loc: SourceLocation;
522+
id: IdentifierId;
476523
sourceIds: Set<IdentifierId>;
477524
typeOfValue: TypeOfValue;
478525
}> = [];
479526

527+
const effectSetStateLoads: Map<IdentifierId, IdentifierId> = new Map();
528+
const effectSetStateUsages: Map<
529+
IdentifierId,
530+
Set<SourceLocation>
531+
> = new Map();
532+
480533
const globals: Set<IdentifierId> = new Set();
481534
for (const block of effectFunction.body.blocks.values()) {
482535
for (const pred of block.preds) {
@@ -492,19 +545,16 @@ function validateEffect(
492545
return;
493546
}
494547

548+
maybeRecordSetState(instr, effectSetStateLoads, effectSetStateUsages);
549+
495550
for (const operand of eachInstructionOperand(instr)) {
496-
if (
497-
isSetStateType(operand.identifier) &&
498-
operand.loc !== GeneratedSource
499-
) {
500-
if (context.effectSetStateCache.has(operand.loc.identifierName)) {
501-
context.effectSetStateCache
502-
.get(operand.loc.identifierName)!
503-
.push(operand);
504-
} else {
505-
context.effectSetStateCache.set(operand.loc.identifierName, [
506-
operand,
507-
]);
551+
if (isSetStateType(operand.identifier)) {
552+
const rootSetStateId = getRootSetState(
553+
operand.identifier.id,
554+
effectSetStateLoads,
555+
);
556+
if (rootSetStateId !== null) {
557+
effectSetStateUsages.get(rootSetStateId)?.add(operand.loc);
508558
}
509559
}
510560
}
@@ -522,7 +572,7 @@ function validateEffect(
522572
if (argMetadata !== undefined) {
523573
effectDerivedSetStateCalls.push({
524574
value: instr.value,
525-
loc: instr.value.callee.loc,
575+
id: instr.value.callee.identifier.id,
526576
sourceIds: argMetadata.sourcesIds,
527577
typeOfValue: argMetadata.typeOfValue,
528578
});
@@ -557,14 +607,10 @@ function validateEffect(
557607

558608
for (const derivedSetStateCall of effectDerivedSetStateCalls) {
559609
if (
560-
derivedSetStateCall.loc !== GeneratedSource &&
561-
context.effectSetStateCache.has(derivedSetStateCall.loc.identifierName) &&
562-
context.setStateCache.has(derivedSetStateCall.loc.identifierName) &&
563-
context.effectSetStateCache.get(derivedSetStateCall.loc.identifierName)!
564-
.length ===
565-
context.setStateCache.get(derivedSetStateCall.loc.identifierName)!
566-
.length -
567-
1
610+
effectSetStateUsages.has(derivedSetStateCall.id) &&
611+
context.setStateUsages.has(derivedSetStateCall.id) &&
612+
effectSetStateUsages.get(derivedSetStateCall.id)!.size ===
613+
context.setStateUsages.get(derivedSetStateCall.id)!.size - 1
568614
) {
569615
const allSourceIds = Array.from(derivedSetStateCall.sourceIds);
570616
const trees = allSourceIds

0 commit comments

Comments
 (0)