Compare commits

..

9 Commits

Author SHA1 Message Date
Joe Savona
6dc956a396 [compiler] Fix infinite loop due to uncached applied signatures
When we apply new aliasing signatures we can generate new temporaries, which causes the abstract memory model to not converge. The fix is to make sure we cache the applications of these signatures.
2025-06-18 15:30:52 -07:00
Joseph Savona
8f4ce72f0b [commit] Improve error for hoisting violations (#33514)
The previous error for hoisting violations pointed only to the variable
declaration, but didn't show where the value was accessed before that
declaration. We now track where each hoisted variable is first accessed
and report two errors, one for the reference and one for the
declaration. When we improve our diagnostic infra to support reporting
errors at multiple locations we can merge these into a single conceptual
error.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33514).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* __->__ #33514
* #33573
2025-06-18 15:24:41 -07:00
Joseph Savona
7ce2a63acc [compiler] update fixtures (#33573)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33573).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* __->__ #33573
2025-06-18 15:24:30 -07:00
Joseph Savona
b067c6fe79 [compiler] Improve error message for mutating hook args/return (#33513)
The previous error message was generic, because the old style function
signature didn't support a way to specify a reason alongside a freeze
effect. This meant we could only say why a value was frozen for
instructions, but not hooks which use function signatures. By defining a
new aliasing signature for custom hooks we can specify a reason and
provide a better error message.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33513).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* __->__ #33513
2025-06-18 13:04:53 -07:00
Joseph Savona
e081cb3446 [compiler] FunctionExpression context locations point to first reference (#33512)
This has always been awkward: `FunctionExpression.context` places have
locations set to the declaration of the identifier, whereas other
references have locations pointing to the reference itself. Here, we
update context operands to have their location point to the first
reference of that variable within the function.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33512).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* #33513
* __->__ #33512
* #33504
* #33500
* #33497
* #33496
2025-06-18 13:02:43 -07:00
Joseph Savona
7b67dc92b0 [commit] Better error message for invalid hoisting (#33504)
We're already tracking which variables are hoisted context variables, so
if we see a mutation of a frozen value we can emit a custom error
message to help users identify the problem.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33504).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* #33513
* #33512
* __->__ #33504
* #33500
* #33497
* #33496
2025-06-18 13:02:32 -07:00
Joseph Savona
7c28c15465 [compiler] Fix AnalyzeFunctions to fully reset context identifiers (#33500)
AnalyzeFunctions had logic to reset the mutable ranges of context
variables after visiting inner function expressions. However, there was
a bug in that logic: InferReactiveScopeVariables makes all the
identifiers in a scope point to the same mutable range instance. That
meant that it was possible for a later function expression to indirectly
cause an earlier function expressions' context variables to get a
non-zero mutable range.

The fix is to not just reset start/end of context var ranges, but assign
a new range instance. Thanks for the help on debugging, @mofeiz!

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33500).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* #33513
* #33512
* #33504
* __->__ #33500
* #33497
* #33496
2025-06-18 13:02:23 -07:00
Joseph Savona
90ccbd71c1 [compiler] Enable new inference by default (#33497)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33497).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* #33513
* #33512
* #33504
* #33500
* __->__ #33497
* #33496
2025-06-18 13:02:12 -07:00
Joseph Savona
0cf6d0c929 [compiler] Update fixtures for new inference (#33496)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33496).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* #33513
* #33512
* #33504
* #33500
* #33497
* __->__ #33496
2025-06-18 13:01:56 -07:00
19 changed files with 477 additions and 126 deletions

View File

@@ -72,7 +72,7 @@ export function lower(
env: Environment,
// Bindings captured from the outer function, in case lower() is called recursively (for lambdas)
bindings: Bindings | null = null,
capturedRefs: Array<t.Identifier> = [],
capturedRefs: Map<t.Identifier, SourceLocation> = new Map(),
): Result<HIRFunction, CompilerError> {
const builder = new HIRBuilder(env, {
bindings,
@@ -80,13 +80,13 @@ export function lower(
});
const context: HIRFunction['context'] = [];
for (const ref of capturedRefs ?? []) {
for (const [ref, loc] of capturedRefs ?? []) {
context.push({
kind: 'Identifier',
identifier: builder.resolveBinding(ref),
effect: Effect.Unknown,
reactive: false,
loc: ref.loc ?? GeneratedSource,
loc,
});
}
@@ -3439,10 +3439,12 @@ function lowerFunction(
* This isn't a problem in practice because use Babel's scope analysis to
* identify the correct references.
*/
const lowering = lower(expr, builder.environment, builder.bindings, [
...builder.context,
...capturedContext,
]);
const lowering = lower(
expr,
builder.environment,
builder.bindings,
new Map([...builder.context, ...capturedContext]),
);
let loweredFunc: HIRFunction;
if (lowering.isErr()) {
lowering
@@ -4160,6 +4162,11 @@ function captureScopes({from, to}: {from: Scope; to: Scope}): Set<Scope> {
return scopes;
}
/**
* Returns a mapping of "context" identifiers — references to free variables that
* will become part of the function expression's `context` array — along with the
* source location of their first reference within the function.
*/
function gatherCapturedContext(
fn: NodePath<
| t.FunctionExpression
@@ -4168,8 +4175,8 @@ function gatherCapturedContext(
| t.ObjectMethod
>,
componentScope: Scope,
): Array<t.Identifier> {
const capturedIds = new Set<t.Identifier>();
): Map<t.Identifier, SourceLocation> {
const capturedIds = new Map<t.Identifier, SourceLocation>();
/*
* Capture all the scopes from the parent of this function up to and including
@@ -4212,8 +4219,15 @@ function gatherCapturedContext(
// Add the base identifier binding as a dependency.
const binding = baseIdentifier.scope.getBinding(baseIdentifier.node.name);
if (binding !== undefined && pureScopes.has(binding.scope)) {
capturedIds.add(binding.identifier);
if (
binding !== undefined &&
pureScopes.has(binding.scope) &&
!capturedIds.has(binding.identifier)
) {
capturedIds.set(
binding.identifier,
path.node.loc ?? binding.identifier.loc ?? GeneratedSource,
);
}
}
@@ -4250,7 +4264,7 @@ function gatherCapturedContext(
},
});
return [...capturedIds.keys()];
return capturedIds;
}
function notNull<T>(value: T | null): value is T {

View File

@@ -1388,6 +1388,16 @@ export enum ValueReason {
*/
JsxCaptured = 'jsx-captured',
/**
* Argument to a hook
*/
HookCaptured = 'hook-captured',
/**
* Return value of a hook
*/
HookReturn = 'hook-return',
/**
* Passed to an effect
*/

View File

@@ -106,7 +106,7 @@ export default class HIRBuilder {
#current: WipBlock;
#entry: BlockId;
#scopes: Array<Scope> = [];
#context: Array<t.Identifier>;
#context: Map<t.Identifier, SourceLocation>;
#bindings: Bindings;
#env: Environment;
#exceptionHandlerStack: Array<BlockId> = [];
@@ -121,7 +121,7 @@ export default class HIRBuilder {
return this.#env.nextIdentifierId;
}
get context(): Array<t.Identifier> {
get context(): Map<t.Identifier, SourceLocation> {
return this.#context;
}
@@ -137,13 +137,13 @@ export default class HIRBuilder {
env: Environment,
options?: {
bindings?: Bindings | null;
context?: Array<t.Identifier>;
context?: Map<t.Identifier, SourceLocation>;
entryBlockKind?: BlockKind;
},
) {
this.#env = env;
this.#bindings = options?.bindings ?? new Map();
this.#context = options?.context ?? [];
this.#context = options?.context ?? new Map();
this.#entry = makeBlockId(env.nextBlockId);
this.#current = newBlock(this.#entry, options?.entryBlockKind ?? 'block');
}

View File

@@ -1302,6 +1302,34 @@ export const DefaultNonmutatingHook = addHook(
calleeEffect: Effect.Read,
hookKind: 'Custom',
returnValueKind: ValueKind.Frozen,
aliasing: {
receiver: makeIdentifierId(0),
params: [],
rest: makeIdentifierId(1),
returns: makeIdentifierId(2),
temporaries: [],
effects: [
// Freeze the arguments
{
kind: 'Freeze',
value: signatureArgument(1),
reason: ValueReason.HookCaptured,
},
// Returns a frozen value
{
kind: 'Create',
into: signatureArgument(2),
value: ValueKind.Frozen,
reason: ValueReason.HookReturn,
},
// May alias any arguments into the return
{
kind: 'Alias',
from: signatureArgument(1),
into: signatureArgument(2),
},
],
},
},
'DefaultNonmutatingHook',
);

View File

@@ -8,6 +8,7 @@
import {CompilerErrorDetailOptions} from '../CompilerError';
import {
FunctionExpression,
GeneratedSource,
Hole,
IdentifierId,
ObjectMethod,
@@ -18,6 +19,7 @@ import {
ValueReason,
} from '../HIR';
import {FunctionSignature} from '../HIR/ObjectShape';
import {printSourceLocation} from '../HIR/PrintHIR';
/**
* `AliasingEffect` describes a set of "effects" that an instruction/terminal has on one or
@@ -200,10 +202,19 @@ export function hashEffect(effect: AliasingEffect): string {
return [effect.kind, effect.value.identifier.id, effect.reason].join(':');
}
case 'Impure':
case 'Render':
case 'Render': {
return [effect.kind, effect.place.identifier.id].join(':');
}
case 'MutateFrozen':
case 'MutateGlobal': {
return [effect.kind, effect.place.identifier.id].join(':');
return [
effect.kind,
effect.place.identifier.id,
effect.error.severity,
effect.error.reason,
effect.error.description,
printSourceLocation(effect.error.loc ?? GeneratedSource),
].join(':');
}
case 'Mutate':
case 'MutateConditionally':

View File

@@ -341,6 +341,10 @@ export function getWriteErrorReason(abstractValue: AbstractValue): string {
return "Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead";
} else if (abstractValue.reason.has(ValueReason.Effect)) {
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()';
} else if (abstractValue.reason.has(ValueReason.HookCaptured)) {
return 'Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook';
} else if (abstractValue.reason.has(ValueReason.HookReturn)) {
return 'Updating a value returned from a hook is not allowed. Consider moving the mutation into the hook where the value is constructed';
} else {
return 'This mutates a variable that React considers immutable';
}

View File

@@ -38,6 +38,7 @@ import {
import {
eachInstructionValueLValue,
eachInstructionValueOperand,
eachTerminalOperand,
eachTerminalSuccessor,
} from '../HIR/visitors';
import {Ok, Result} from '../Utils/Result';
@@ -49,12 +50,14 @@ import {
} from './InferReferenceEffects';
import {
assertExhaustive,
getOrInsertDefault,
getOrInsertWith,
Set_isSuperset,
} from '../Utils/utils';
import {
printAliasingEffect,
printAliasingSignature,
printFunction,
printIdentifier,
printInstruction,
printInstructionValue,
@@ -194,12 +197,15 @@ export function inferMutationAliasingEffects(
let count = 0;
while (queuedStates.size !== 0) {
count++;
if (count > 1000) {
if (count > 100) {
console.log(
'oops infinite loop',
fn.id,
typeof fn.loc !== 'symbol' ? fn.loc?.filename : null,
);
if (DEBUG) {
console.log(printFunction(fn));
}
throw new Error('infinite loop');
}
for (const [blockId, block] of fn.body.blocks) {
@@ -211,6 +217,11 @@ export function inferMutationAliasingEffects(
statesByBlock.set(blockId, incomingState);
const state = incomingState.clone();
if (DEBUG) {
console.log('*************');
console.log(`bb${block.id}`);
console.log('*************');
}
inferBlock(context, state, block);
for (const nextBlockId of eachTerminalSuccessor(block.terminal)) {
@@ -221,8 +232,19 @@ export function inferMutationAliasingEffects(
return Ok(undefined);
}
function findHoistedContextDeclarations(fn: HIRFunction): Set<DeclarationId> {
const hoisted = new Set<DeclarationId>();
function findHoistedContextDeclarations(
fn: HIRFunction,
): Map<DeclarationId, Place | null> {
const hoisted = new Map<DeclarationId, Place | null>();
function visit(place: Place): void {
if (
hoisted.has(place.identifier.declarationId) &&
hoisted.get(place.identifier.declarationId) == null
) {
// If this is the first load of the value, store the location
hoisted.set(place.identifier.declarationId, place);
}
}
for (const block of fn.body.blocks.values()) {
for (const instr of block.instructions) {
if (instr.value.kind === 'DeclareContext') {
@@ -232,10 +254,17 @@ function findHoistedContextDeclarations(fn: HIRFunction): Set<DeclarationId> {
kind == InstructionKind.HoistedFunction ||
kind == InstructionKind.HoistedLet
) {
hoisted.add(instr.value.lvalue.place.identifier.declarationId);
hoisted.set(instr.value.lvalue.place.identifier.declarationId, null);
}
} else {
for (const operand of eachInstructionValueOperand(instr.value)) {
visit(operand);
}
}
}
for (const operand of eachTerminalOperand(block.terminal)) {
visit(operand);
}
}
return hoisted;
}
@@ -245,21 +274,40 @@ class Context {
instructionSignatureCache: Map<Instruction, InstructionSignature> = new Map();
effectInstructionValueCache: Map<AliasingEffect, InstructionValue> =
new Map();
applySignatureCache: Map<
AliasingSignature,
Map<AliasingEffect, Array<AliasingEffect> | null>
> = new Map();
catchHandlers: Map<BlockId, Place> = new Map();
functionSignatureCache: Map<FunctionExpression, AliasingSignature> =
new Map();
isFuctionExpression: boolean;
fn: HIRFunction;
hoistedContextDeclarations: Set<DeclarationId>;
hoistedContextDeclarations: Map<DeclarationId, Place | null>;
constructor(
isFunctionExpression: boolean,
fn: HIRFunction,
hoistedContextDeclarations: Set<DeclarationId>,
hoistedContextDeclarations: Map<DeclarationId, Place | null>,
) {
this.isFuctionExpression = isFunctionExpression;
this.fn = fn;
this.hoistedContextDeclarations = hoistedContextDeclarations;
}
cacheApplySignature(
signature: AliasingSignature,
effect: Extract<AliasingEffect, {kind: 'Apply'}>,
f: () => Array<AliasingEffect> | null,
): Array<AliasingEffect> | null {
const inner = getOrInsertDefault(
this.applySignatureCache,
signature,
new Map(),
);
return getOrInsertWith(inner, effect, f);
}
internEffect(effect: AliasingEffect): AliasingEffect {
const hash = hashEffect(effect);
let interned = this.internedEffects.get(hash);
@@ -333,11 +381,13 @@ function inferBlock(
state.appendAlias(handlerParam, instr.lvalue);
const kind = state.kind(instr.lvalue).kind;
if (kind === ValueKind.Mutable || kind == ValueKind.Context) {
effects.push({
kind: 'Alias',
from: instr.lvalue,
into: handlerParam,
});
effects.push(
context.internEffect({
kind: 'Alias',
from: instr.lvalue,
into: handlerParam,
}),
);
}
}
}
@@ -346,11 +396,11 @@ function inferBlock(
} else if (terminal.kind === 'return') {
if (!context.isFuctionExpression) {
terminal.effects = [
{
context.internEffect({
kind: 'Freeze',
value: terminal.value,
reason: ValueReason.JsxCaptured,
},
}),
];
}
}
@@ -527,20 +577,21 @@ function applyEffect(
break;
}
case ValueKind.Frozen: {
effects.push({
kind: 'ImmutableCapture',
from: effect.from,
into: effect.into,
});
applyEffect(
context,
state,
{
kind: 'ImmutableCapture',
from: effect.from,
into: effect.into,
},
aliased,
effects,
);
break;
}
default: {
effects.push({
// OK: recording information flow
kind: 'CreateFrom', // prev Alias
from: effect.from,
into: effect.into,
});
effects.push(effect);
}
}
break;
@@ -639,11 +690,17 @@ function applyEffect(
}
case ValueKind.Frozen: {
isMutableReferenceType = false;
effects.push({
kind: 'ImmutableCapture',
from: effect.from,
into: effect.into,
});
applyEffect(
context,
state,
{
kind: 'ImmutableCapture',
from: effect.from,
into: effect.into,
},
aliased,
effects,
);
break;
}
default: {
@@ -665,11 +722,17 @@ function applyEffect(
const fromKind = fromValue.kind;
switch (fromKind) {
case ValueKind.Frozen: {
effects.push({
kind: 'ImmutableCapture',
from: effect.from,
into: effect.into,
});
applyEffect(
context,
state,
{
kind: 'ImmutableCapture',
from: effect.from,
into: effect.into,
},
aliased,
effects,
);
let value = context.effectInstructionValueCache.get(effect);
if (value == null) {
value = {
@@ -727,23 +790,33 @@ function applyEffect(
* We're calling a locally declared function, we already know it's effects!
* We just have to substitute in the args for the params
*/
const signature = buildSignatureFromFunctionExpression(
state.env,
functionValues[0],
);
const functionExpr = functionValues[0];
let signature = context.functionSignatureCache.get(functionExpr);
if (signature == null) {
signature = buildSignatureFromFunctionExpression(
state.env,
functionExpr,
);
context.functionSignatureCache.set(functionExpr, signature);
}
if (DEBUG) {
console.log(
`constructed alias signature:\n${printAliasingSignature(signature)}`,
);
}
const signatureEffects = computeEffectsForSignature(
state.env,
const signatureEffects = context.cacheApplySignature(
signature,
effect.into,
effect.receiver,
effect.args,
functionValues[0].loweredFunc.func.context,
effect.loc,
effect,
() =>
computeEffectsForSignature(
state.env,
signature,
effect.into,
effect.receiver,
effect.args,
functionExpr.loweredFunc.func.context,
effect.loc,
),
);
if (signatureEffects != null) {
if (DEBUG) {
@@ -762,18 +835,24 @@ function applyEffect(
break;
}
}
const signatureEffects =
effect.signature?.aliasing != null
? computeEffectsForSignature(
let signatureEffects = null;
if (effect.signature?.aliasing != null) {
const signature = effect.signature.aliasing;
signatureEffects = context.cacheApplySignature(
effect.signature.aliasing,
effect,
() =>
computeEffectsForSignature(
state.env,
effect.signature.aliasing,
signature,
effect.into,
effect.receiver,
effect.args,
[],
effect.loc,
)
: null;
),
);
}
if (signatureEffects != null) {
if (DEBUG) {
console.log('apply aliasing signature effects');
@@ -901,27 +980,89 @@ function applyEffect(
console.log(prettyFormat(state.debugAbstractValue(value)));
}
const reason = getWriteErrorReason({
kind: value.kind,
reason: value.reason,
context: new Set(),
});
effects.push({
kind:
value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal',
place: effect.value,
error: {
severity: ErrorSeverity.InvalidReact,
reason,
description:
effect.value.identifier.name !== null &&
effect.value.identifier.name.kind === 'named'
? `Found mutation of \`${effect.value.identifier.name.value}\``
: null,
loc: effect.value.loc,
suggestions: null,
},
});
if (
mutationKind === 'mutate-frozen' &&
context.hoistedContextDeclarations.has(
effect.value.identifier.declarationId,
)
) {
const description =
effect.value.identifier.name !== null &&
effect.value.identifier.name.kind === 'named'
? `Variable \`${effect.value.identifier.name.value}\` is accessed before it is declared`
: null;
const hoistedAccess = context.hoistedContextDeclarations.get(
effect.value.identifier.declarationId,
);
if (hoistedAccess != null && hoistedAccess.loc != effect.value.loc) {
applyEffect(
context,
state,
{
kind: 'MutateFrozen',
place: effect.value,
error: {
severity: ErrorSeverity.InvalidReact,
reason: `This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time`,
description,
loc: hoistedAccess.loc,
suggestions: null,
},
},
aliased,
effects,
);
}
applyEffect(
context,
state,
{
kind: 'MutateFrozen',
place: effect.value,
error: {
severity: ErrorSeverity.InvalidReact,
reason: `This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`,
description,
loc: effect.value.loc,
suggestions: null,
},
},
aliased,
effects,
);
} else {
const reason = getWriteErrorReason({
kind: value.kind,
reason: value.reason,
context: new Set(),
});
const description =
effect.value.identifier.name !== null &&
effect.value.identifier.name.kind === 'named'
? `Found mutation of \`${effect.value.identifier.name.value}\``
: null;
applyEffect(
context,
state,
{
kind:
value.kind === ValueKind.Frozen
? 'MutateFrozen'
: 'MutateGlobal',
place: effect.value,
error: {
severity: ErrorSeverity.InvalidReact,
reason,
description,
loc: effect.value.loc,
suggestions: null,
},
},
aliased,
effects,
);
}
}
break;
}
@@ -1959,28 +2100,17 @@ function computeEffectsForLegacySignature(
break;
}
case Effect.ConditionallyMutateIterator: {
if (
isArrayType(place.identifier) ||
isSetType(place.identifier) ||
isMapType(place.identifier)
) {
effects.push({
kind: 'Capture',
from: place,
into: lvalue,
});
} else {
effects.push({
kind: 'Capture',
from: place,
into: lvalue,
});
const mutateIterator = conditionallyMutateIterator(place);
if (mutateIterator != null) {
effects.push(mutateIterator);
// TODO: should we always push to captures?
captures.push(place);
effects.push({
kind: 'MutateTransitiveConditionally',
value: place,
});
}
effects.push({
kind: 'Capture',
from: place,
into: lvalue,
});
break;
}
case Effect.Freeze: {
@@ -2170,6 +2300,7 @@ function computeEffectsForSignature(
return null;
}
// Build substitutions
const mutableSpreads = new Set<IdentifierId>();
const substitutions: Map<IdentifierId, Array<Place>> = new Map();
substitutions.set(signature.receiver, [receiver]);
substitutions.set(signature.returns, [lvalue]);
@@ -2187,6 +2318,13 @@ function computeEffectsForSignature(
}
const place = arg.kind === 'Identifier' ? arg : arg.place;
getOrInsertWith(substitutions, signature.rest, () => []).push(place);
if (arg.kind === 'Spread') {
const mutateIterator = conditionallyMutateIterator(arg.place);
if (mutateIterator != null) {
mutableSpreads.add(arg.place.identifier.id);
}
}
} else {
const param = params[i];
substitutions.set(param, [arg]);
@@ -2258,6 +2396,12 @@ function computeEffectsForSignature(
case 'Freeze': {
const values = substitutions.get(effect.value.identifier.id) ?? [];
for (const value of values) {
if (mutableSpreads.has(value.identifier.id)) {
CompilerError.throwTodo({
reason: 'Support spread syntax for hook arguments',
loc: value.loc,
});
}
effects.push({kind: 'Freeze', value, reason: effect.reason});
}
break;

View File

@@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = {
11 | });
12 |
> 13 | x.value += count;
| ^ InvalidReact: This mutates a variable that React considers immutable (13:13)
| ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (13:13)
14 | return <Stringify x={x} cb={cb} />;
15 | }
16 |

View File

@@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = {
11 | });
12 |
> 13 | x.value += count;
| ^ InvalidReact: This mutates a variable that React considers immutable (13:13)
| ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (13:13)
14 | return <Stringify x={x} cb={cb} />;
15 | }
16 |

View File

@@ -38,13 +38,15 @@ export const FIXTURE_ENTRYPOINT = {
## Error
```
19 | useEffect(() => setState(2), []);
17 | * $2 = Function context=setState
18 | */
> 19 | useEffect(() => setState(2), []);
| ^^^^^^^^ InvalidReact: This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time. Variable `setState` is accessed before it is declared (19:19)
InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Variable `setState` is accessed before it is declared (21:21)
20 |
> 21 | const [state, setState] = useState(0);
| ^^^^^^^^ InvalidReact: Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect(). Found mutation of `setState` (21:21)
21 | const [state, setState] = useState(0);
22 | return <Stringify state={state} />;
23 | }
24 |
```

View File

@@ -27,7 +27,7 @@ function SomeComponent() {
9 | return (
10 | <Button
> 11 | onPress={() => (sharedVal.value = Math.random())}
| ^^^^^^^^^ InvalidReact: This mutates a variable that React considers immutable. Found mutation of `sharedVal` (11:11)
| ^^^^^^^^^ InvalidReact: Updating a value returned from a hook is not allowed. Consider moving the mutation into the hook where the value is constructed. Found mutation of `sharedVal` (11:11)
12 | title="Randomize"
13 | />
14 | );

View File

@@ -34,13 +34,13 @@ export const FIXTURE_ENTRYPOINT = {
## Error
```
13 | return bar();
11 |
12 | function foo() {
> 13 | return bar();
| ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (13:13)
14 | }
> 15 | function bar() {
| ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (15:15)
15 | function bar() {
16 | return 42;
17 | }
18 |
```

View File

@@ -52,7 +52,7 @@ export const FIXTURE_ENTRYPOINT = {
## Logs
```
{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":320},"end":{"line":11,"column":6,"index":324},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"detail":{"reason":"Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":320},"end":{"line":11,"column":6,"index":324},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":7,"column":2,"index":216},"end":{"line":7,"column":36,"index":250},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":7,"column":31,"index":245},"end":{"line":7,"column":34,"index":248},"filename":"retry-no-emit.ts","identifierName":"arr"}]}
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":10,"column":2,"index":274},"end":{"line":10,"column":44,"index":316},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":10,"column":25,"index":297},"end":{"line":10,"column":29,"index":301},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":25,"index":297},"end":{"line":10,"column":29,"index":301},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":35,"index":307},"end":{"line":10,"column":42,"index":314},"filename":"retry-no-emit.ts","identifierName":"propVal"}]}
{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"fnName":"Foo","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}

View File

@@ -0,0 +1,45 @@
## Input
```javascript
//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
import {useCallback} from 'react';
import {useIdentity} from 'shared-runtime';
function Component({content, refetch}) {
// This callback function accesses a hoisted const as a dependency,
// but it cannot reference it as a dependency since that would be a
// TDZ violation!
const onRefetch = useCallback(() => {
refetch(data);
}, [refetch]);
// The context variable gets frozen here since it's passed to a hook
const onSubmit = useIdentity(onRefetch);
// This has to error: onRefetch needs to memoize with `content` as a
// dependency, but the dependency comes later
const {data = null} = content;
return <Foo data={data} onSubmit={onSubmit} />;
}
```
## Error
```
9 | // TDZ violation!
10 | const onRefetch = useCallback(() => {
> 11 | refetch(data);
| ^^^^ InvalidReact: This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time. Variable `data` is accessed before it is declared (11:11)
InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Variable `data` is accessed before it is declared (19:19)
12 | }, [refetch]);
13 |
14 | // The context variable gets frozen here since it's passed to a hook
```

View File

@@ -0,0 +1,22 @@
//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
import {useCallback} from 'react';
import {useIdentity} from 'shared-runtime';
function Component({content, refetch}) {
// This callback function accesses a hoisted const as a dependency,
// but it cannot reference it as a dependency since that would be a
// TDZ violation!
const onRefetch = useCallback(() => {
refetch(data);
}, [refetch]);
// The context variable gets frozen here since it's passed to a hook
const onSubmit = useIdentity(onRefetch);
// This has to error: onRefetch needs to memoize with `content` as a
// dependency, but the dependency comes later
const {data = null} = content;
return <Foo data={data} onSubmit={onSubmit} />;
}

View File

@@ -19,7 +19,7 @@ function Component({a, b}) {
3 | const x = {a};
4 | useFreeze(x);
> 5 | x.y = true;
| ^ InvalidReact: This mutates a variable that React considers immutable (5:5)
| ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (5:5)
6 | return <div>error</div>;
7 | }
8 |

View File

@@ -0,0 +1,54 @@
## Input
```javascript
// @flow @enableNewMutationAliasingModel
import fbt from 'fbt';
component Component() {
const sections = Object.keys(items);
for (let i = 0; i < sections.length; i += 3) {
chunks.push(
sections.slice(i, i + 3).map(section => {
return <Child />;
})
);
}
return <Child />;
}
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime";
import fbt from "fbt";
function Component() {
const $ = _c(1);
const sections = Object.keys(items);
for (let i = 0; i < sections.length; i = i + 3, i) {
chunks.push(sections.slice(i, i + 3).map(_temp));
}
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = <Child />;
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}
function _temp(section) {
return <Child />;
}
```
### Eval output
(kind: exception) Fixture not implemented

View File

@@ -0,0 +1,17 @@
// @flow @enableNewMutationAliasingModel
import fbt from 'fbt';
component Component() {
const sections = Object.keys(items);
for (let i = 0; i < sections.length; i += 3) {
chunks.push(
sections.slice(i, i + 3).map(section => {
return <Child />;
})
);
}
return <Child />;
}

View File

@@ -52,7 +52,7 @@ export const FIXTURE_ENTRYPOINT = {
## Logs
```
{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":352},"end":{"line":11,"column":6,"index":356},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"detail":{"reason":"Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":352},"end":{"line":11,"column":6,"index":356},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":7,"column":2,"index":248},"end":{"line":7,"column":36,"index":282},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":7,"column":31,"index":277},"end":{"line":7,"column":34,"index":280},"filename":"retry-no-emit.ts","identifierName":"arr"}]}
{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":10,"column":2,"index":306},"end":{"line":10,"column":44,"index":348},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":10,"column":25,"index":329},"end":{"line":10,"column":29,"index":333},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":25,"index":329},"end":{"line":10,"column":29,"index":333},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":35,"index":339},"end":{"line":10,"column":42,"index":346},"filename":"retry-no-emit.ts","identifierName":"propVal"}]}
{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"fnName":"Foo","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}