Skip to content

Commit 42e1039

Browse files
committed
[compiler][newinference] Fixes, update remaining snapshots
A bunch of small fixes to make the remaining fixtures work. This is really really close now. ghstack-source-id: a3d6803 Pull Request resolved: #33477
1 parent bf7cbad commit 42e1039

File tree

32 files changed

+423
-681
lines changed

32 files changed

+423
-681
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,6 @@ function runWithEnvironment(
247247
}
248248

249249
if (!env.config.enableNewMutationAliasingModel) {
250-
// NOTE: in the new model this is part of validateNoFreezingKnownMutableFunctions
251250
validateLocalsNotReassignedAfterRender(hir);
252251
}
253252

@@ -275,6 +274,7 @@ function runWithEnvironment(
275274
if (mutabilityAliasingErrors.isErr()) {
276275
throw mutabilityAliasingErrors.unwrapErr();
277276
}
277+
validateLocalsNotReassignedAfterRender(hir);
278278
}
279279
}
280280

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

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
} from '../HIR/visitors';
4444
import {Ok, Result} from '../Utils/Result';
4545
import {
46+
getArgumentEffect,
4647
getFunctionCallSignature,
4748
isKnownMutableEffect,
4849
mergeValueKinds,
@@ -547,7 +548,11 @@ function applyEffect(
547548
effect =>
548549
effect.kind === 'MutateFrozen' || effect.kind === 'MutateGlobal',
549550
);
550-
const isMutable = hasCaptures || hasTrackedSideEffects;
551+
// For legacy compatibility
552+
const capturesRef = effect.function.loweredFunc.func.context.some(
553+
operand => isRefOrRefValue(operand.identifier),
554+
);
555+
const isMutable = hasCaptures || hasTrackedSideEffects || capturesRef;
551556
for (const operand of effect.function.loweredFunc.func.context) {
552557
if (operand.effect !== Effect.Capture) {
553558
continue;
@@ -805,6 +810,11 @@ function applyEffect(
805810
effects,
806811
);
807812
}
813+
const mutateIterator =
814+
arg.kind === 'Spread' ? conditionallyMutateIterator(operand) : null;
815+
if (mutateIterator) {
816+
applyEffect(context, state, mutateIterator, aliased, effects);
817+
}
808818
applyEffect(
809819
context,
810820
state,
@@ -1299,6 +1309,22 @@ type InstructionSignature = {
12991309
effects: ReadonlyArray<AliasingEffect>;
13001310
};
13011311

1312+
function conditionallyMutateIterator(place: Place): AliasingEffect | null {
1313+
if (
1314+
!(
1315+
isArrayType(place.identifier) ||
1316+
isSetType(place.identifier) ||
1317+
isMapType(place.identifier)
1318+
)
1319+
) {
1320+
return {
1321+
kind: 'MutateTransitiveConditionally',
1322+
value: place,
1323+
};
1324+
}
1325+
return null;
1326+
}
1327+
13021328
/**
13031329
* Computes an effect signature for the instruction _without_ looking at the inference state,
13041330
* and only using the semantics of the instructions and the inferred types. The idea is to make
@@ -1334,6 +1360,10 @@ function computeSignatureForInstruction(
13341360
into: lvalue,
13351361
});
13361362
} else if (element.kind === 'Spread') {
1363+
const mutateIterator = conditionallyMutateIterator(element.place);
1364+
if (mutateIterator != null) {
1365+
effects.push(mutateIterator);
1366+
}
13371367
effects.push({
13381368
kind: 'Capture',
13391369
from: element.place,
@@ -1567,7 +1597,7 @@ function computeSignatureForInstruction(
15671597
// Extracts part of the original collection into the result
15681598
effects.push({
15691599
kind: 'CreateFrom',
1570-
from: value.iterator,
1600+
from: value.collection,
15711601
into: lvalue,
15721602
});
15731603
break;
@@ -1623,11 +1653,20 @@ function computeSignatureForInstruction(
16231653
}
16241654
case 'Destructure': {
16251655
for (const patternLValue of eachInstructionValueLValue(value)) {
1626-
effects.push({
1627-
kind: 'CreateFrom',
1628-
from: value.value,
1629-
into: patternLValue,
1630-
});
1656+
if (isPrimitiveType(patternLValue.identifier)) {
1657+
effects.push({
1658+
kind: 'Create',
1659+
into: patternLValue,
1660+
value: ValueKind.Primitive,
1661+
reason: ValueReason.Other,
1662+
});
1663+
} else {
1664+
effects.push({
1665+
kind: 'CreateFrom',
1666+
from: value.value,
1667+
into: patternLValue,
1668+
});
1669+
}
16311670
}
16321671
effects.push({kind: 'Assign', from: value.value, into: lvalue});
16331672
break;
@@ -1947,17 +1986,11 @@ function computeEffectsForLegacySignature(
19471986
continue;
19481987
}
19491988
const place = arg.kind === 'Identifier' ? arg : arg.place;
1950-
const effect =
1989+
const signatureEffect =
19511990
arg.kind === 'Identifier' && i < signature.positionalParams.length
19521991
? signature.positionalParams[i]!
19531992
: (signature.restParam ?? Effect.ConditionallyMutate);
1954-
1955-
if (arg.kind === 'Spread' && effect === Effect.Freeze) {
1956-
CompilerError.throwTodo({
1957-
reason: 'Support spread syntax for hook arguments',
1958-
loc: arg.place.loc,
1959-
});
1960-
}
1993+
const effect = getArgumentEffect(signatureEffect, arg);
19611994

19621995
visit(place, effect);
19631996
}

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,12 @@ export function inferMutationAliasingRanges(
121121
});
122122
} else if (effect.kind === 'CreateFrom') {
123123
state.createFrom(index++, effect.from, effect.into);
124-
} else if (effect.kind === 'Assign' || effect.kind === 'Alias') {
124+
} else if (effect.kind === 'Assign') {
125+
if (!state.nodes.has(effect.into.identifier)) {
126+
state.create(effect.into, {kind: 'Object'});
127+
}
128+
state.assign(index++, effect.from, effect.into);
129+
} else if (effect.kind === 'Alias') {
125130
state.assign(index++, effect.from, effect.into);
126131
} else if (effect.kind === 'Capture') {
127132
state.capture(index++, effect.from, effect.into);
@@ -204,8 +209,8 @@ export function inferMutationAliasingRanges(
204209
errors,
205210
);
206211
}
207-
if (VERBOSE) {
208-
console.log(state.debug());
212+
if (DEBUG) {
213+
console.log(pretty([...state.nodes.keys()]));
209214
}
210215
fn.aliasingEffects ??= [];
211216
for (const param of [...fn.context, ...fn.params]) {
@@ -534,6 +539,11 @@ class AliasingState {
534539
loc: SourceLocation,
535540
errors: CompilerError,
536541
): void {
542+
if (DEBUG) {
543+
console.log(
544+
`mutate ix=${index} start=$${start.id} end=[${end}]${transitive ? ' transitive' : ''} kind=${kind}`,
545+
);
546+
}
537547
const seen = new Set<Identifier>();
538548
const queue: Array<{
539549
place: Identifier;
@@ -557,7 +567,7 @@ class AliasingState {
557567
}
558568
if (DEBUG) {
559569
console.log(
560-
`[${end}] mutate index=${index} ${printIdentifier(start)}: ${printIdentifier(node.id)}`,
570+
` mutate $${node.id.id} transitive=${transitive} direction=${direction}`,
561571
);
562572
}
563573
node.id.mutableRange.end = makeInstructionId(

compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1984,7 +1984,7 @@ function areArgumentsImmutableAndNonMutating(
19841984
return true;
19851985
}
19861986

1987-
function getArgumentEffect(
1987+
export function getArgumentEffect(
19881988
signatureEffect: Effect | null,
19891989
arg: Place | SpreadPattern,
19901990
): Effect {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,10 @@ export function validateNoFreezingKnownMutableFunctions(
131131
lvalue.identifier.id,
132132
knownMutation,
133133
);
134-
} else if (context.has(effect.value.identifier.id)) {
134+
} else if (
135+
context.has(effect.value.identifier.id) &&
136+
!isRefOrRefLikeMutableType(effect.value.identifier.type)
137+
) {
135138
contextMutationEffects.set(lvalue.identifier.id, {
136139
kind: 'ContextMutation',
137140
effect: Effect.Mutate,

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-nested-scope-truncated-dep.expect.md

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -175,21 +175,14 @@ import {
175175
* and mutability.
176176
*/
177177
function Component(t0) {
178-
const $ = _c(4);
178+
const $ = _c(2);
179179
const { prop } = t0;
180180
let t1;
181181
if ($[0] !== prop) {
182182
const obj = shallowCopy(prop);
183183
const aliasedObj = identity(obj);
184-
let t2;
185-
if ($[2] !== obj) {
186-
t2 = [obj.id];
187-
$[2] = obj;
188-
$[3] = t2;
189-
} else {
190-
t2 = $[3];
191-
}
192-
const id = t2;
184+
185+
const id = [obj.id];
193186

194187
mutate(aliasedObj);
195188
setPropertyByKey(aliasedObj, "id", prop.id + 1);

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-map-captures-receiver-noAlias.expect.md

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,34 @@ export const FIXTURE_ENTRYPOINT = {
2323
```javascript
2424
import { c as _c } from "react/compiler-runtime";
2525
function Component(props) {
26-
const $ = _c(2);
26+
const $ = _c(6);
2727
let t0;
2828
if ($[0] !== props.a) {
29-
const item = { a: props.a };
30-
const items = [item];
31-
t0 = items.map(_temp);
29+
t0 = { a: props.a };
3230
$[0] = props.a;
3331
$[1] = t0;
3432
} else {
3533
t0 = $[1];
3634
}
37-
const mapped = t0;
35+
const item = t0;
36+
let t1;
37+
if ($[2] !== item) {
38+
t1 = [item];
39+
$[2] = item;
40+
$[3] = t1;
41+
} else {
42+
t1 = $[3];
43+
}
44+
const items = t1;
45+
let t2;
46+
if ($[4] !== items) {
47+
t2 = items.map(_temp);
48+
$[4] = items;
49+
$[5] = t2;
50+
} else {
51+
t2 = $[5];
52+
}
53+
const mapped = t2;
3854
return mapped;
3955
}
4056
function _temp(item_0) {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -84,19 +84,11 @@ import { makeArray, mutate } from "shared-runtime";
8484
* used when we analyze CallExpressions.
8585
*/
8686
function Component(t0) {
87-
const $ = _c(5);
87+
const $ = _c(3);
8888
const { foo, bar } = t0;
89-
let t1;
90-
if ($[0] !== foo) {
91-
t1 = { foo };
92-
$[0] = foo;
93-
$[1] = t1;
94-
} else {
95-
t1 = $[1];
96-
}
97-
const x = t1;
9889
let y;
99-
if ($[2] !== bar || $[3] !== x) {
90+
if ($[0] !== bar || $[1] !== foo) {
91+
const x = { foo };
10092
y = { bar };
10193
const f0 = function () {
10294
const a = makeArray(y);
@@ -107,11 +99,11 @@ function Component(t0) {
10799

108100
f0();
109101
mutate(y.x);
110-
$[2] = bar;
111-
$[3] = x;
112-
$[4] = y;
102+
$[0] = bar;
103+
$[1] = foo;
104+
$[2] = y;
113105
} else {
114-
y = $[4];
106+
y = $[2];
115107
}
116108
return y;
117109
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.expect.md

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ export const FIXTURE_ENTRYPOINT = {
3838
## Code
3939
4040
```javascript
41-
import { c as _c } from "react/compiler-runtime";
4241
import { CONST_TRUE, Stringify, mutate, useIdentity } from "shared-runtime";
4342

4443
/**
@@ -54,31 +53,14 @@ import { CONST_TRUE, Stringify, mutate, useIdentity } from "shared-runtime";
5453
*
5554
*/
5655
function Component() {
57-
const $ = _c(4);
5856
const obj = CONST_TRUE ? { inner: { value: "hello" } } : null;
59-
let t0;
60-
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
61-
t0 = [obj?.inner];
62-
$[0] = t0;
63-
} else {
64-
t0 = $[0];
65-
}
66-
const boxedInner = t0;
57+
const boxedInner = [obj?.inner];
6758
useIdentity(null);
6859
mutate(obj);
6960
if (boxedInner[0] !== obj?.inner) {
7061
throw new Error("invariant broken");
7162
}
72-
let t1;
73-
if ($[1] !== boxedInner || $[2] !== obj) {
74-
t1 = <Stringify obj={obj} inner={boxedInner} />;
75-
$[1] = boxedInner;
76-
$[2] = obj;
77-
$[3] = t1;
78-
} else {
79-
t1 = $[3];
80-
}
81-
return t1;
63+
return <Stringify obj={obj} inner={boxedInner} />;
8264
}
8365

8466
export const FIXTURE_ENTRYPOINT = {

0 commit comments

Comments
 (0)