Skip to content

Commit a93a8d2

Browse files
committed
[compiler] fixes for effects capturing context vars, hoisted functions
ghstack-source-id: 7528f90 Pull Request resolved: #33469
1 parent f4bb1ec commit a93a8d2

File tree

5 files changed

+80
-4
lines changed

5 files changed

+80
-4
lines changed

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

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

8-
import {Effect, ValueKind, ValueReason} from './HIR';
8+
import {Effect, makeIdentifierId, ValueKind, ValueReason} from './HIR';
99
import {
1010
BUILTIN_SHAPES,
1111
BuiltInArrayId,
@@ -32,6 +32,7 @@ import {
3232
addFunction,
3333
addHook,
3434
addObject,
35+
signatureArgument,
3536
} from './ObjectShape';
3637
import {BuiltInType, ObjectType, PolyType} from './Types';
3738
import {TypeConfig} from './TypeSchema';
@@ -642,6 +643,41 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
642643
calleeEffect: Effect.Read,
643644
hookKind: 'useEffect',
644645
returnValueKind: ValueKind.Frozen,
646+
aliasing: {
647+
receiver: makeIdentifierId(0),
648+
params: [],
649+
rest: makeIdentifierId(1),
650+
returns: makeIdentifierId(2),
651+
temporaries: [signatureArgument(3)],
652+
effects: [
653+
// Freezes the function and deps
654+
{
655+
kind: 'Freeze',
656+
value: signatureArgument(1),
657+
reason: ValueReason.Effect,
658+
},
659+
// Internally creates an effect object that captures the function and deps
660+
{
661+
kind: 'Create',
662+
into: signatureArgument(3),
663+
value: ValueKind.Frozen,
664+
reason: ValueReason.KnownReturnSignature,
665+
},
666+
// The effect stores the function and dependencies
667+
{
668+
kind: 'Capture',
669+
from: signatureArgument(1),
670+
into: signatureArgument(3),
671+
},
672+
// Returns undefined
673+
{
674+
kind: 'Create',
675+
into: signatureArgument(2),
676+
value: ValueKind.Primitive,
677+
reason: ValueReason.KnownReturnSignature,
678+
},
679+
],
680+
},
645681
},
646682
BuiltInUseEffectHookId,
647683
),

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1396,6 +1396,11 @@ export enum ValueReason {
13961396
*/
13971397
JsxCaptured = 'jsx-captured',
13981398

1399+
/**
1400+
* Passed to an effect
1401+
*/
1402+
Effect = 'effect',
1403+
13991404
/**
14001405
* Return value of a function with known frozen return value, e.g. `useState`.
14011406
*/

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,8 @@ export function getWriteErrorReason(abstractValue: AbstractValue): string {
357357
return "Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead";
358358
} else if (abstractValue.reason.has(ValueReason.ReducerState)) {
359359
return "Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead";
360+
} else if (abstractValue.reason.has(ValueReason.Effect)) {
361+
return 'Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect()';
360362
} else {
361363
return 'This mutates a variable that React considers immutable';
362364
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ import {getWriteErrorReason} from './InferFunctionEffects';
6666
import prettyFormat from 'pretty-format';
6767
import {createTemporaryPlace} from '../HIR/HIRBuilder';
6868

69+
const DEBUG = false;
70+
6971
export function inferMutationAliasingEffects(
7072
fn: HIRFunction,
7173
{isFunctionExpression}: {isFunctionExpression: boolean} = {
@@ -849,7 +851,7 @@ function applyEffect(
849851
) {
850852
const value = state.kind(effect.value);
851853
if (DEBUG) {
852-
console.log(printAliasingEffect(effect));
854+
console.log(`invalid mutation: ${printAliasingEffect(effect)}`);
853855
console.log(prettyFormat(state.debugAbstractValue(value)));
854856
}
855857

@@ -891,8 +893,6 @@ function applyEffect(
891893
}
892894
}
893895

894-
const DEBUG = false;
895-
896896
class InferenceState {
897897
env: Environment;
898898
#isFunctionExpression: boolean;

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,39 @@ export function inferMutationAliasingRanges(
381381
const effect = operandEffects.get(operand.identifier.id) ?? Effect.Read;
382382
operand.effect = effect;
383383
}
384+
385+
/**
386+
* This case is targeted at hoisted functions like:
387+
*
388+
* ```
389+
* x();
390+
* function x() { ... }
391+
* ```
392+
*
393+
* Which turns into:
394+
*
395+
* t0 = DeclareContext HoistedFunction x
396+
* t1 = LoadContext x
397+
* t2 = CallExpression t1 ( )
398+
* t3 = FunctionExpression ...
399+
* t4 = StoreContext Function x = t3
400+
*
401+
* If the function had captured mutable values, it would already have its
402+
* range extended to include the StoreContext. But if the function doesn't
403+
* capture any mutable values its range won't have been extended yet. We
404+
* want to ensure that the value is memoized along with the context variable,
405+
* not independently of it (bc of the way we do codegen for hoisted functions).
406+
* So here we check for StoreContext rvalues and if they haven't already had
407+
* their range extended to at least this instruction, we extend it.
408+
*/
409+
if (
410+
instr.value.kind === 'StoreContext' &&
411+
instr.value.value.identifier.mutableRange.end <= instr.id
412+
) {
413+
instr.value.value.identifier.mutableRange.end = makeInstructionId(
414+
instr.id + 1,
415+
);
416+
}
384417
}
385418
if (block.terminal.kind === 'return') {
386419
block.terminal.value.effect = isFunctionExpression

0 commit comments

Comments
 (0)