Skip to content

Commit b95885f

Browse files
committed
[compiler] Fix for PropertyStore object effect
Fix for the issue in the previous PR. Long-term the ideal thing would be to make InferMutableRanges smarter about Store effects, and recognize that they are also transitive mutations of whatever was captured into the object. So in the following: ``` const x = {y: {z: {}}}; x.y.z.key = value; ``` That the `PropertyStore z . 'key' = value` is a transitive mutation of x and all three object expressions (x, x.y, x.y.z). But for now it's simpler to stick to the original idea of Store only counting if we know that the type is an object. ghstack-source-id: c96a66f Pull Request resolved: #33164
1 parent 6153279 commit b95885f

File tree

5 files changed

+37
-36
lines changed

5 files changed

+37
-36
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ function isMutable(range: MutableRange): boolean {
731731
}
732732

733733
const DEBUG_MUTABLE_RANGES = false;
734-
function printMutableRange(identifier: Identifier): string {
734+
export function printMutableRange(identifier: Identifier): string {
735735
if (DEBUG_MUTABLE_RANGES) {
736736
// if debugging, print both the identifier and scope range if they differ
737737
const range = identifier.mutableRange;

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,13 @@ function inferAliasesForCapturing(
8585
for (const block of fn.body.blocks.values()) {
8686
for (const instr of block.instructions) {
8787
const {lvalue, value} = instr;
88-
const hasStore =
88+
const hasCapture =
8989
lvalue.effect === Effect.Store ||
9090
Iterable_some(
9191
eachInstructionValueOperand(value),
92-
operand => operand.effect === Effect.Store,
92+
operand => operand.effect === Effect.Capture,
9393
);
94-
if (!hasStore) {
94+
if (!hasCapture) {
9595
continue;
9696
}
9797
const operands: Array<Identifier> = [];
@@ -101,6 +101,7 @@ function inferAliasesForCapturing(
101101
for (const operand of eachInstructionValueOperand(instr.value)) {
102102
if (
103103
operand.effect === Effect.Store ||
104+
operand.effect === Effect.Mutate ||
104105
operand.effect === Effect.Capture
105106
) {
106107
operands.push(operand.identifier);

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

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

8+
import prettyFormat from 'pretty-format';
89
import {HIRFunction, Identifier} from '../HIR/HIR';
910
import DisjointSet from '../Utils/DisjointSet';
1011
import {inferAliasForUncalledFunctions} from './InerAliasForUncalledFunctions';
@@ -16,6 +17,11 @@ import {inferMutableLifetimes} from './InferMutableLifetimes';
1617
import {inferMutableRangesForAlias} from './InferMutableRangesForAlias';
1718
import {inferMutableRangesForComutation} from './InferMutableRangesForComutation';
1819
import {inferTryCatchAliases} from './InferTryCatchAliases';
20+
import {
21+
printFunction,
22+
printIdentifier,
23+
printMutableRange,
24+
} from '../HIR/PrintHIR';
1925

2026
export function inferMutableRanges(ir: HIRFunction): DisjointSet<Identifier> {
2127
// Infer mutable ranges for non fields
@@ -96,6 +102,20 @@ export function inferMutableRanges(ir: HIRFunction): DisjointSet<Identifier> {
96102
return aliases;
97103
}
98104

105+
export function debugAliases(aliases: DisjointSet<Identifier>): void {
106+
console.log(
107+
prettyFormat(
108+
aliases
109+
.buildSets()
110+
.map(set =>
111+
[...set].map(
112+
ident => printIdentifier(ident) + printMutableRange(ident),
113+
),
114+
),
115+
),
116+
);
117+
}
118+
99119
/**
100120
* Canonicalizes the alias set and mutable range information calculated at the current time.
101121
* The returned value maps each identifier in the program to the root identifier of its alias

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

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
isMapType,
3333
isMutableEffect,
3434
isSetType,
35+
isObjectType,
3536
} from '../HIR/HIR';
3637
import {FunctionSignature} from '../HIR/ObjectShape';
3738
import {
@@ -1313,7 +1314,7 @@ function inferBlock(
13131314
state.referenceAndRecordEffects(
13141315
freezeActions,
13151316
instrValue.object,
1316-
typeof instrValue.property === 'string'
1317+
isObjectType(instrValue.object.identifier)
13171318
? Effect.Store
13181319
: Effect.Mutate,
13191320
ValueReason.Other,
@@ -1342,25 +1343,21 @@ function inferBlock(
13421343
state.referenceAndRecordEffects(
13431344
freezeActions,
13441345
instrValue.object,
1345-
Effect.Read,
1346+
Effect.Capture,
13461347
ValueReason.Other,
13471348
);
13481349
const lvalue = instr.lvalue;
1349-
lvalue.effect = Effect.ConditionallyMutate;
1350+
lvalue.effect = Effect.Store;
13501351
state.initialize(instrValue, state.kind(instrValue.object));
13511352
state.define(lvalue, instrValue);
13521353
continuation = {kind: 'funeffects'};
13531354
break;
13541355
}
13551356
case 'ComputedStore': {
1356-
const effect =
1357-
state.kind(instrValue.object).kind === ValueKind.Context
1358-
? Effect.ConditionallyMutate
1359-
: Effect.Capture;
13601357
state.referenceAndRecordEffects(
13611358
freezeActions,
13621359
instrValue.value,
1363-
effect,
1360+
Effect.Capture,
13641361
ValueReason.Other,
13651362
);
13661363
state.referenceAndRecordEffects(
@@ -1372,7 +1369,9 @@ function inferBlock(
13721369
state.referenceAndRecordEffects(
13731370
freezeActions,
13741371
instrValue.object,
1375-
Effect.Store,
1372+
isObjectType(instrValue.object.identifier)
1373+
? Effect.Store
1374+
: Effect.Mutate,
13761375
ValueReason.Other,
13771376
);
13781377

@@ -1409,7 +1408,7 @@ function inferBlock(
14091408
state.referenceAndRecordEffects(
14101409
freezeActions,
14111410
instrValue.object,
1412-
Effect.Read,
1411+
Effect.Capture,
14131412
ValueReason.Other,
14141413
);
14151414
state.referenceAndRecordEffects(
@@ -1419,7 +1418,7 @@ function inferBlock(
14191418
ValueReason.Other,
14201419
);
14211420
const lvalue = instr.lvalue;
1422-
lvalue.effect = Effect.ConditionallyMutate;
1421+
lvalue.effect = Effect.Store;
14231422
state.initialize(instrValue, state.kind(instrValue.object));
14241423
state.define(lvalue, instrValue);
14251424
continuation = {kind: 'funeffects'};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-access-assignment.expect.md

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export const FIXTURE_ENTRYPOINT = {
3333
```javascript
3434
import { c as _c } from "react/compiler-runtime";
3535
function Component(t0) {
36-
const $ = _c(10);
36+
const $ = _c(6);
3737
const { a, b, c } = t0;
3838
let t1;
3939
if ($[0] !== a || $[1] !== b || $[2] !== c) {
@@ -47,26 +47,7 @@ function Component(t0) {
4747
t2 = $[5];
4848
}
4949
const y = t2;
50-
let t3;
51-
let t4;
52-
if ($[6] === Symbol.for("react.memo_cache_sentinel")) {
53-
t3 = {};
54-
t4 = {};
55-
$[6] = t3;
56-
$[7] = t4;
57-
} else {
58-
t3 = $[6];
59-
t4 = $[7];
60-
}
61-
let t5;
62-
if ($[8] !== c) {
63-
t5 = { zero: c };
64-
$[8] = c;
65-
$[9] = t5;
66-
} else {
67-
t5 = $[9];
68-
}
69-
const z = { zero: t3, one: t4, two: t5 };
50+
const z = { zero: {}, one: {}, two: { zero: c } };
7051
x.zero = y.one;
7152
z.zero.zero = x.zero;
7253
t1 = { zero: x, one: z };

0 commit comments

Comments
 (0)