Skip to content

Commit f0e4d14

Browse files
committed
[compiler][poc] Improve impurity/ref tracking
We currently model impure functions like `Date.now()` with an `Impure` effect, but the way this works is that the effect is considered an error if it's called at all. But we really want to reflect the fact that the _value_ is impure, and is okay to be used outside of render. For example, `useRef(Date.now())` should be fine. So this PR changes the Impure effect to describe that an impure value flows `into: Place`. We flow impurity through the data flow graph during InferMutationAliasingRanges, including propagating this to the function's effects. So if a function expression returns a transitively impure value, that's expressed in our inferred signature for that function. Calling it propagates the impurity via our effect system. We stop this propagation when reaching a ref, allowing `useRef(Date.now())` or `useRef(localFunctionThatReturnsDateNow())`. This lets us also model accessing a ref as an impure value - we just emit an `Impure` event for PropertyLoad/ComputedLoad off of a ref, and the above tracking kicks in. A final piece is that we can also use our existing machinery to disallow writing to global values during render to handle writing to refs - except that we need to allow the lazy init pattern. So it probably makes sense to keep the ref validation pass, but scope it back to just handling writes and not reads. This definitely needs more polish, the error messages would ideally be something like: ``` Error: cannot call impure function in render Blah blah description of why this is bad... foo.js:0:0 <Foo x={x} /> ^ This value reads from an impure function foo.js:0:0 x = Date.now(); ^^^^^^^^ The value derives from Date.now which is impure ``` Ie show you both the local site (where impurity flows into render) and the ultiamte source of the impure value.
1 parent 0526c79 commit f0e4d14

23 files changed

+623
-155
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,12 @@ export class CompilerDiagnostic {
132132
return new CompilerDiagnostic({...options, details: []});
133133
}
134134

135+
clone(): CompilerDiagnostic {
136+
const cloned = CompilerDiagnostic.create({...this.options});
137+
cloned.options.details = [...this.options.details];
138+
return cloned;
139+
}
140+
135141
get reason(): CompilerDiagnosticOptions['reason'] {
136142
return this.options.reason;
137143
}

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ import {propagateScopeDependenciesHIR} from '../HIR/PropagateScopeDependenciesHI
9696
import {outlineJSX} from '../Optimization/OutlineJsx';
9797
import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls';
9898
import {transformFire} from '../Transform';
99-
import {validateNoImpureFunctionsInRender} from '../Validation/ValidateNoImpureFunctionsInRender';
10099
import {validateStaticComponents} from '../Validation/ValidateStaticComponents';
101100
import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoFreezingKnownMutableFunctions';
102101
import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEffects';
@@ -293,10 +292,6 @@ function runWithEnvironment(
293292
env.logErrors(validateNoJSXInTryStatement(hir));
294293
}
295294

296-
if (env.config.validateNoImpureFunctionsInRender) {
297-
validateNoImpureFunctionsInRender(hir).unwrap();
298-
}
299-
300295
validateNoFreezingKnownMutableFunctions(hir).unwrap();
301296
}
302297

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

Lines changed: 85 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import {
3838
addObject,
3939
} from './ObjectShape';
4040
import {BuiltInType, ObjectType, PolyType} from './Types';
41-
import {TypeConfig} from './TypeSchema';
41+
import {AliasingSignatureConfig, TypeConfig} from './TypeSchema';
4242
import {assertExhaustive} from '../Utils/utils';
4343
import {isHookName} from './Environment';
4444
import {CompilerError, SourceLocation} from '..';
@@ -626,6 +626,78 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [
626626
// TODO: rest of Global objects
627627
];
628628

629+
const RenderHookAliasing: (
630+
reason: ValueReason,
631+
) => AliasingSignatureConfig = reason => ({
632+
receiver: '@receiver',
633+
params: [],
634+
rest: '@rest',
635+
returns: '@returns',
636+
temporaries: [],
637+
effects: [
638+
// Freeze the arguments
639+
{
640+
kind: 'Freeze',
641+
value: '@rest',
642+
reason: ValueReason.HookCaptured,
643+
},
644+
// Render the arguments
645+
{
646+
kind: 'Render',
647+
place: '@rest',
648+
},
649+
// Returns a frozen value
650+
{
651+
kind: 'Create',
652+
into: '@returns',
653+
value: ValueKind.Frozen,
654+
reason,
655+
},
656+
// May alias any arguments into the return
657+
{
658+
kind: 'Alias',
659+
from: '@rest',
660+
into: '@returns',
661+
},
662+
],
663+
});
664+
665+
const EffectHookAliasing: AliasingSignatureConfig = {
666+
receiver: '@receiver',
667+
params: [],
668+
rest: '@rest',
669+
returns: '@returns',
670+
temporaries: ['@effect'],
671+
effects: [
672+
// Freezes the function and deps
673+
{
674+
kind: 'Freeze',
675+
value: '@rest',
676+
reason: ValueReason.Effect,
677+
},
678+
// Internally creates an effect object that captures the function and deps
679+
{
680+
kind: 'Create',
681+
into: '@effect',
682+
value: ValueKind.Frozen,
683+
reason: ValueReason.KnownReturnSignature,
684+
},
685+
// The effect stores the function and dependencies
686+
{
687+
kind: 'Capture',
688+
from: '@rest',
689+
into: '@effect',
690+
},
691+
// Returns undefined
692+
{
693+
kind: 'Create',
694+
into: '@returns',
695+
value: ValueKind.Primitive,
696+
reason: ValueReason.KnownReturnSignature,
697+
},
698+
],
699+
};
700+
629701
/*
630702
* TODO(mofeiZ): We currently only store rest param effects for hooks.
631703
* now that FeatureFlag `enableTreatHooksAsFunctions` is removed we can
@@ -644,6 +716,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
644716
hookKind: 'useContext',
645717
returnValueKind: ValueKind.Frozen,
646718
returnValueReason: ValueReason.Context,
719+
aliasing: RenderHookAliasing(ValueReason.Context),
647720
},
648721
BuiltInUseContextHookId,
649722
),
@@ -658,6 +731,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
658731
hookKind: 'useState',
659732
returnValueKind: ValueKind.Frozen,
660733
returnValueReason: ValueReason.State,
734+
aliasing: RenderHookAliasing(ValueReason.State),
661735
}),
662736
],
663737
[
@@ -670,6 +744,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
670744
hookKind: 'useActionState',
671745
returnValueKind: ValueKind.Frozen,
672746
returnValueReason: ValueReason.State,
747+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
673748
}),
674749
],
675750
[
@@ -682,6 +757,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
682757
hookKind: 'useReducer',
683758
returnValueKind: ValueKind.Frozen,
684759
returnValueReason: ValueReason.ReducerState,
760+
aliasing: RenderHookAliasing(ValueReason.ReducerState),
685761
}),
686762
],
687763
[
@@ -715,6 +791,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
715791
calleeEffect: Effect.Read,
716792
hookKind: 'useMemo',
717793
returnValueKind: ValueKind.Frozen,
794+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
718795
}),
719796
],
720797
[
@@ -726,6 +803,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
726803
calleeEffect: Effect.Read,
727804
hookKind: 'useCallback',
728805
returnValueKind: ValueKind.Frozen,
806+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
729807
}),
730808
],
731809
[
@@ -739,41 +817,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
739817
calleeEffect: Effect.Read,
740818
hookKind: 'useEffect',
741819
returnValueKind: ValueKind.Frozen,
742-
aliasing: {
743-
receiver: '@receiver',
744-
params: [],
745-
rest: '@rest',
746-
returns: '@returns',
747-
temporaries: ['@effect'],
748-
effects: [
749-
// Freezes the function and deps
750-
{
751-
kind: 'Freeze',
752-
value: '@rest',
753-
reason: ValueReason.Effect,
754-
},
755-
// Internally creates an effect object that captures the function and deps
756-
{
757-
kind: 'Create',
758-
into: '@effect',
759-
value: ValueKind.Frozen,
760-
reason: ValueReason.KnownReturnSignature,
761-
},
762-
// The effect stores the function and dependencies
763-
{
764-
kind: 'Capture',
765-
from: '@rest',
766-
into: '@effect',
767-
},
768-
// Returns undefined
769-
{
770-
kind: 'Create',
771-
into: '@returns',
772-
value: ValueKind.Primitive,
773-
reason: ValueReason.KnownReturnSignature,
774-
},
775-
],
776-
},
820+
aliasing: EffectHookAliasing,
777821
},
778822
BuiltInUseEffectHookId,
779823
),
@@ -789,6 +833,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
789833
calleeEffect: Effect.Read,
790834
hookKind: 'useLayoutEffect',
791835
returnValueKind: ValueKind.Frozen,
836+
aliasing: EffectHookAliasing,
792837
},
793838
BuiltInUseLayoutEffectHookId,
794839
),
@@ -804,6 +849,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
804849
calleeEffect: Effect.Read,
805850
hookKind: 'useInsertionEffect',
806851
returnValueKind: ValueKind.Frozen,
852+
aliasing: EffectHookAliasing,
807853
},
808854
BuiltInUseInsertionEffectHookId,
809855
),
@@ -817,6 +863,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
817863
calleeEffect: Effect.Read,
818864
hookKind: 'useTransition',
819865
returnValueKind: ValueKind.Frozen,
866+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
820867
}),
821868
],
822869
[
@@ -829,6 +876,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
829876
hookKind: 'useOptimistic',
830877
returnValueKind: ValueKind.Frozen,
831878
returnValueReason: ValueReason.State,
879+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
832880
}),
833881
],
834882
[
@@ -842,6 +890,7 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
842890
returnType: {kind: 'Poly'},
843891
calleeEffect: Effect.Read,
844892
returnValueKind: ValueKind.Frozen,
893+
aliasing: RenderHookAliasing(ValueReason.HookCaptured),
845894
},
846895
BuiltInUseOperatorId,
847896
),

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

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

8-
import {CompilerError} from '../CompilerError';
8+
import {
9+
CompilerDiagnostic,
10+
CompilerError,
11+
ErrorCategory,
12+
} from '../CompilerError';
913
import {AliasingEffect, AliasingSignature} from '../Inference/AliasingEffects';
1014
import {assertExhaustive} from '../Utils/utils';
1115
import {
@@ -190,16 +194,28 @@ function parseAliasingSignatureConfig(
190194
};
191195
}
192196
case 'Impure': {
193-
const place = lookup(effect.place);
197+
const into = lookup(effect.into);
194198
return {
195199
kind: 'Impure',
196-
place,
197-
error: CompilerError.throwTodo({
198-
reason: 'Support impure effect declarations',
199-
loc: GeneratedSource,
200+
into,
201+
error: CompilerDiagnostic.create({
202+
category: ErrorCategory.Purity,
203+
reason: 'Cannot call impure function during render',
204+
description: effect.reason,
205+
}).withDetails({
206+
kind: 'error',
207+
loc,
208+
message: 'Cannot call impure function',
200209
}),
201210
};
202211
}
212+
case 'Render': {
213+
const place = lookup(effect.place);
214+
return {
215+
kind: 'Render',
216+
place,
217+
};
218+
}
203219
case 'Apply': {
204220
const receiver = lookup(effect.receiver);
205221
const fn = lookup(effect.function);
@@ -1513,6 +1529,11 @@ export const DefaultNonmutatingHook = addHook(
15131529
value: '@rest',
15141530
reason: ValueReason.HookCaptured,
15151531
},
1532+
// Render the arguments
1533+
{
1534+
kind: 'Render',
1535+
place: '@rest',
1536+
},
15161537
// Returns a frozen value
15171538
{
15181539
kind: 'Create',

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1009,7 +1009,7 @@ export function printAliasingEffect(effect: AliasingEffect): string {
10091009
return `MutateGlobal ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
10101010
}
10111011
case 'Impure': {
1012-
return `Impure ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
1012+
return `Impure ${printPlaceForAliasEffect(effect.into)} reason=${effect.error.reason}`;
10131013
}
10141014
case 'Render': {
10151015
return `Render ${printPlaceForAliasEffect(effect.place)}`;

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,23 @@ export const ApplyEffectSchema: z.ZodType<ApplyEffectConfig> = z.object({
185185

186186
export type ImpureEffectConfig = {
187187
kind: 'Impure';
188-
place: string;
188+
into: string;
189+
reason: string;
189190
};
190191

191192
export const ImpureEffectSchema: z.ZodType<ImpureEffectConfig> = z.object({
192193
kind: z.literal('Impure'),
194+
into: LifetimeIdSchema,
195+
reason: z.string(),
196+
});
197+
198+
export type RenderEffectConfig = {
199+
kind: 'Render';
200+
place: string;
201+
};
202+
203+
export const RenderEffectSchema: z.ZodType<RenderEffectConfig> = z.object({
204+
kind: z.literal('Render'),
193205
place: LifetimeIdSchema,
194206
});
195207

@@ -204,7 +216,8 @@ export type AliasingEffectConfig =
204216
| ImpureEffectConfig
205217
| MutateEffectConfig
206218
| MutateTransitiveConditionallyConfig
207-
| ApplyEffectConfig;
219+
| ApplyEffectConfig
220+
| RenderEffectConfig;
208221

209222
export const AliasingEffectSchema: z.ZodType<AliasingEffectConfig> = z.union([
210223
FreezeEffectSchema,

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ export type AliasingEffect =
162162
/**
163163
* Indicates a side-effect that is not safe during render
164164
*/
165-
| {kind: 'Impure'; place: Place; error: CompilerDiagnostic}
165+
| {kind: 'Impure'; into: Place; error: CompilerDiagnostic}
166166
/**
167167
* Indicates that a given place is accessed during render. Used to distingush
168168
* hook arguments that are known to be called immediately vs those used for
@@ -222,6 +222,14 @@ export function hashEffect(effect: AliasingEffect): string {
222222
return [effect.kind, effect.value.identifier.id, effect.reason].join(':');
223223
}
224224
case 'Impure':
225+
return [
226+
effect.kind,
227+
effect.into.identifier.id,
228+
effect.error.severity,
229+
effect.error.reason,
230+
effect.error.description,
231+
printSourceLocation(effect.error.primaryLocation() ?? GeneratedSource),
232+
].join(':');
225233
case 'Render': {
226234
return [effect.kind, effect.place.identifier.id].join(':');
227235
}

0 commit comments

Comments
 (0)