Skip to content

[pull] main from facebook:main #129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 31 additions & 22 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3609,31 +3609,40 @@ function lowerAssignment(

let temporary;
if (builder.isContextIdentifier(lvalue)) {
if (kind !== InstructionKind.Reassign && !isHoistedIdentifier) {
if (kind === InstructionKind.Const) {
builder.errors.push({
reason: `Expected \`const\` declaration not to be reassigned`,
severity: ErrorSeverity.InvalidJS,
loc: lvalue.node.loc ?? null,
suggestions: null,
});
}
lowerValueToTemporary(builder, {
kind: 'DeclareContext',
lvalue: {
kind: InstructionKind.Let,
place: {...place},
},
loc: place.loc,
if (kind === InstructionKind.Const && !isHoistedIdentifier) {
builder.errors.push({
reason: `Expected \`const\` declaration not to be reassigned`,
severity: ErrorSeverity.InvalidJS,
loc: lvalue.node.loc ?? null,
suggestions: null,
});
}

temporary = lowerValueToTemporary(builder, {
kind: 'StoreContext',
lvalue: {place: {...place}, kind: InstructionKind.Reassign},
value,
loc,
});
if (
kind !== InstructionKind.Const &&
kind !== InstructionKind.Reassign &&
kind !== InstructionKind.Let &&
kind !== InstructionKind.Function
) {
builder.errors.push({
reason: `Unexpected context variable kind`,
severity: ErrorSeverity.InvalidJS,
loc: lvalue.node.loc ?? null,
suggestions: null,
});
temporary = lowerValueToTemporary(builder, {
kind: 'UnsupportedNode',
node: lvalueNode,
loc: lvalueNode.loc ?? GeneratedSource,
});
} else {
temporary = lowerValueToTemporary(builder, {
kind: 'StoreContext',
lvalue: {place: {...place}, kind},
value,
loc,
});
}
} else {
const typeAnnotation = lvalue.get('typeAnnotation');
let type: t.FlowType | t.TSType | null;
Expand Down
35 changes: 34 additions & 1 deletion compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,27 @@ export enum InstructionKind {
Function = 'Function',
}

export function convertHoistedLValueKind(
kind: InstructionKind,
): InstructionKind | null {
switch (kind) {
case InstructionKind.HoistedLet:
return InstructionKind.Let;
case InstructionKind.HoistedConst:
return InstructionKind.Const;
case InstructionKind.HoistedFunction:
return InstructionKind.Function;
case InstructionKind.Let:
case InstructionKind.Const:
case InstructionKind.Function:
case InstructionKind.Reassign:
case InstructionKind.Catch:
return null;
default:
assertExhaustive(kind, 'Unexpected lvalue kind');
}
}

function _staticInvariantInstructionValueHasLocation(
value: InstructionValue,
): SourceLocation {
Expand Down Expand Up @@ -880,8 +901,20 @@ export type InstructionValue =
| StoreLocal
| {
kind: 'StoreContext';
/**
* StoreContext kinds:
* Reassign: context variable reassignment in source
* Const: const declaration + assignment in source
* ('const' context vars are ones whose declarations are hoisted)
* Let: let declaration + assignment in source
* Function: function declaration in source (similar to `const`)
*/
lvalue: {
kind: InstructionKind.Reassign;
kind:
| InstructionKind.Reassign
| InstructionKind.Const
| InstructionKind.Let
| InstructionKind.Function;
place: Place;
};
value: Place;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
FunctionExpression,
ObjectMethod,
PropertyLiteral,
convertHoistedLValueKind,
} from './HIR';
import {
collectHoistablePropertyLoads,
Expand Down Expand Up @@ -246,12 +247,18 @@ function isLoadContextMutable(
id: InstructionId,
): instrValue is LoadContext {
if (instrValue.kind === 'LoadContext') {
CompilerError.invariant(instrValue.place.identifier.scope != null, {
reason:
'[PropagateScopeDependencies] Expected all context variables to be assigned a scope',
loc: instrValue.loc,
});
return id >= instrValue.place.identifier.scope.range.end;
/**
* Not all context variables currently have scopes due to limitations of
* mutability analysis for function expressions.
*
* Currently, many function expressions references are inferred to be
* 'Read' | 'Freeze' effects which don't replay mutable effects of captured
* context.
*/
return (
instrValue.place.identifier.scope != null &&
id >= instrValue.place.identifier.scope.range.end
);
}
return false;
}
Expand Down Expand Up @@ -471,6 +478,9 @@ export class DependencyCollectionContext {
}
this.#reassignments.set(identifier, decl);
}
hasDeclared(identifier: Identifier): boolean {
return this.#declarations.has(identifier.declarationId);
}

// Checks if identifier is a valid dependency in the current scope
#checkValidDependency(maybeDependency: ReactiveScopeDependency): boolean {
Expand Down Expand Up @@ -672,21 +682,21 @@ export function handleInstruction(
});
} else if (value.kind === 'DeclareLocal' || value.kind === 'DeclareContext') {
/*
* Some variables may be declared and never initialized. We need
* to retain (and hoist) these declarations if they are included
* in a reactive scope. One approach is to simply add all `DeclareLocal`s
* as scope declarations.
* Some variables may be declared and never initialized. We need to retain
* (and hoist) these declarations if they are included in a reactive scope.
* One approach is to simply add all `DeclareLocal`s as scope declarations.
*
* Context variables with hoisted declarations only become live after their
* first assignment. We only declare real DeclareLocal / DeclareContext
* instructions (not hoisted ones) to avoid generating dependencies on
* hoisted declarations.
*/

/*
* We add context variable declarations here, not at `StoreContext`, since
* context Store / Loads are modeled as reads and mutates to the underlying
* variable reference (instead of through intermediate / inlined temporaries)
*/
context.declare(value.lvalue.place.identifier, {
id,
scope: context.currentScope,
});
if (convertHoistedLValueKind(value.lvalue.kind) === null) {
context.declare(value.lvalue.place.identifier, {
id,
scope: context.currentScope,
});
}
} else if (value.kind === 'Destructure') {
context.visitOperand(value.value);
for (const place of eachPatternOperand(value.lvalue.pattern)) {
Expand All @@ -698,6 +708,26 @@ export function handleInstruction(
scope: context.currentScope,
});
}
} else if (value.kind === 'StoreContext') {
/**
* Some StoreContext variables have hoisted declarations. If we're storing
* to a context variable that hasn't yet been declared, the StoreContext is
* the declaration.
* (see corresponding logic in PruneHoistedContext)
*/
if (
!context.hasDeclared(value.lvalue.place.identifier) ||
value.lvalue.kind !== InstructionKind.Reassign
) {
context.declare(value.lvalue.place.identifier, {
id,
scope: context.currentScope,
});
}

for (const operand of eachInstructionValueOperand(value)) {
context.visitOperand(operand);
}
} else {
for (const operand of eachInstructionValueOperand(value)) {
context.visitOperand(operand);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,15 @@ export function inferMutableLifetimes(
if (
instr.value.kind === 'DeclareContext' ||
(instr.value.kind === 'StoreContext' &&
instr.value.lvalue.kind !== InstructionKind.Reassign)
instr.value.lvalue.kind !== InstructionKind.Reassign &&
!contextVariableDeclarationInstructions.has(
instr.value.lvalue.place.identifier,
))
) {
// Save declarations of context variables
/**
* Save declarations of context variables if they hasn't already been
* declared (due to hoisted declarations).
*/
contextVariableDeclarationInstructions.set(
instr.value.lvalue.place.identifier,
instr.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ export default function inferReferenceEffects(
* Initial state contains function params
* TODO: include module declarations here as well
*/
const initialState = InferenceState.empty(fn.env);
const initialState = InferenceState.empty(
fn.env,
options.isFunctionExpression,
);
const value: InstructionValue = {
kind: 'Primitive',
loc: fn.loc,
Expand Down Expand Up @@ -255,6 +258,7 @@ type FreezeAction = {values: Set<InstructionValue>; reason: Set<ValueReason>};
// Maintains a mapping of top-level variables to the kind of value they hold
class InferenceState {
env: Environment;
#isFunctionExpression: boolean;

// The kind of each value, based on its allocation site
#values: Map<InstructionValue, AbstractValue>;
Expand All @@ -267,16 +271,25 @@ class InferenceState {

constructor(
env: Environment,
isFunctionExpression: boolean,
values: Map<InstructionValue, AbstractValue>,
variables: Map<IdentifierId, Set<InstructionValue>>,
) {
this.env = env;
this.#isFunctionExpression = isFunctionExpression;
this.#values = values;
this.#variables = variables;
}

static empty(env: Environment): InferenceState {
return new InferenceState(env, new Map(), new Map());
static empty(
env: Environment,
isFunctionExpression: boolean,
): InferenceState {
return new InferenceState(env, isFunctionExpression, new Map(), new Map());
}

get isFunctionExpression(): boolean {
return this.#isFunctionExpression;
}

// (Re)initializes a @param value with its default @param kind.
Expand Down Expand Up @@ -394,9 +407,14 @@ class InferenceState {

freezeValues(values: Set<InstructionValue>, reason: Set<ValueReason>): void {
for (const value of values) {
if (value.kind === 'DeclareContext') {
if (
value.kind === 'DeclareContext' ||
(value.kind === 'StoreContext' &&
(value.lvalue.kind === InstructionKind.Let ||
value.lvalue.kind === InstructionKind.Const))
) {
/**
* Avoid freezing hoisted context declarations
* Avoid freezing context variable declarations, hoisted or otherwise
* function Component() {
* const cb = useBar(() => foo(2)); // produces a hoisted context declaration
* const foo = useFoo(); // reassigns to the context variable
Expand Down Expand Up @@ -613,6 +631,7 @@ class InferenceState {
} else {
return new InferenceState(
this.env,
this.#isFunctionExpression,
nextValues ?? new Map(this.#values),
nextVariables ?? new Map(this.#variables),
);
Expand All @@ -627,6 +646,7 @@ class InferenceState {
clone(): InferenceState {
return new InferenceState(
this.env,
this.#isFunctionExpression,
new Map(this.#values),
new Map(this.#variables),
);
Expand Down Expand Up @@ -1591,6 +1611,14 @@ function inferBlock(
);

const lvalue = instr.lvalue;
if (instrValue.lvalue.kind !== InstructionKind.Reassign) {
state.initialize(instrValue, {
kind: ValueKind.Mutable,
reason: new Set([ValueReason.Other]),
context: new Set(),
});
state.define(instrValue.lvalue.place, instrValue);
}
state.alias(lvalue, instrValue.value);
lvalue.effect = Effect.Store;
continuation = {kind: 'funeffects'};
Expand Down Expand Up @@ -1781,8 +1809,15 @@ function inferBlock(
if (block.terminal.kind === 'return' || block.terminal.kind === 'throw') {
if (
state.isDefined(operand) &&
state.kind(operand).kind === ValueKind.Context
((operand.identifier.type.kind === 'Function' &&
state.isFunctionExpression) ||
state.kind(operand).kind === ValueKind.Context)
) {
/**
* Returned values should only be typed as 'frozen' if they are both (1)
* local and (2) not a function expression which may capture and mutate
* this function's outer context.
*/
effect = Effect.ConditionallyMutate;
} else {
effect = Effect.Freeze;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,14 @@ function codegenTerminal(
lval = codegenLValue(cx, iterableItem.value.lvalue.pattern);
break;
}
case 'StoreContext': {
CompilerError.throwTodo({
reason: 'Support non-trivial for..in inits',
description: null,
loc: terminal.init.loc,
suggestions: null,
});
}
default:
CompilerError.invariant(false, {
reason: `Expected a StoreLocal or Destructure to be assigned to the collection`,
Expand Down Expand Up @@ -1092,6 +1100,14 @@ function codegenTerminal(
lval = codegenLValue(cx, iterableItem.value.lvalue.pattern);
break;
}
case 'StoreContext': {
CompilerError.throwTodo({
reason: 'Support non-trivial for..of inits',
description: null,
loc: terminal.init.loc,
suggestions: null,
});
}
default:
CompilerError.invariant(false, {
reason: `Expected a StoreLocal or Destructure to be assigned to the collection`,
Expand Down
Loading
Loading