Skip to content

Commit

Permalink
[compiler] clean up retry pipeline: fireRetry flag -> compileMode
Browse files Browse the repository at this point in the history 
Removes `EnvironmentConfig.enableMinimalTransformsForRetry` in favor of `run` modes. This is a minimal difference but lets us explicitly opt certain compiler passes in/out of different modes.

Retry flags don't really make sense to have in `EnvironmentConfig` anyways as the config is user-facing API, while retrying is a compiler implementation detail.

(per @josephsavona's feedback #32164 (comment))
> Re the "hacky" framing of this in the PR title: I think this is fine. I can see having something like a compilation or output mode that we use when running the pipeline. Rather than changing environment settings when we re-run, various passes could take effect based on the combination of the mode + env flags. The modes might be:
>
> * Full: transform, validate, memoize. This is the default today.
> * Transform: Along the lines of the backup mode in this PR. Only applies transforms that do not require following the rules of React, like `fire()`.
> * Validate: This could be used for ESLint.
  • Loading branch information
@mofeiZ
mofeiZ committed Mar 4, 2025
1 parent 443b7ff commit d33f183
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 57 deletions.
73 changes: 46 additions & 27 deletions compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
pruneUnusedLabelsHIR,
} from '../HIR';
import {
CompilerMode,
Environment,
EnvironmentConfig,
ReactFunctionType,
Expand Down Expand Up @@ -100,6 +101,7 @@ import {outlineJSX} from '../Optimization/OutlineJsx';
import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls';
import {transformFire} from '../Transform';
import {validateNoImpureFunctionsInRender} from '../Validation/ValiateNoImpureFunctionsInRender';
import {CompilerError} from '..';

export type CompilerPipelineValue =
| {kind: 'ast'; name: string; value: CodegenFunction}
Expand All @@ -113,6 +115,7 @@ function run(
>,
config: EnvironmentConfig,
fnType: ReactFunctionType,
mode: CompilerMode,
useMemoCacheIdentifier: string,
logger: Logger | null,
filename: string | null,
Expand All @@ -122,6 +125,7 @@ function run(
const env = new Environment(
func.scope,
fnType,
mode,
config,
contextIdentifiers,
logger,
Expand Down Expand Up @@ -160,10 +164,10 @@ function runWithEnvironment(
validateUseMemo(hir);

if (
env.isInferredMemoEnabled &&
!env.config.enablePreserveExistingManualUseMemo &&
!env.config.disableMemoizationForDebugging &&
!env.config.enableChangeDetectionForDebugging &&
!env.config.enableMinimalTransformsForRetry
!env.config.enableChangeDetectionForDebugging
) {
dropManualMemoization(hir);
log({kind: 'hir', name: 'DropManualMemoization', value: hir});
Expand Down Expand Up @@ -196,19 +200,20 @@ function runWithEnvironment(
inferTypes(hir);
log({kind: 'hir', name: 'InferTypes', value: hir});

if (env.config.validateHooksUsage) {
validateHooksUsage(hir);
if (env.isInferredMemoEnabled) {
if (env.config.validateHooksUsage) {
validateHooksUsage(hir);
}
if (env.config.validateNoCapitalizedCalls) {
validateNoCapitalizedCalls(hir);
}
}

if (env.config.enableFire) {
transformFire(hir);
log({kind: 'hir', name: 'TransformFire', value: hir});
}

if (env.config.validateNoCapitalizedCalls) {
validateNoCapitalizedCalls(hir);
}

if (env.config.lowerContextAccess) {
lowerContextAccess(hir, env.config.lowerContextAccess);
}
Expand All @@ -219,7 +224,12 @@ function runWithEnvironment(
analyseFunctions(hir);
log({kind: 'hir', name: 'AnalyseFunctions', value: hir});

inferReferenceEffects(hir);
const referenceEffectsResult = inferReferenceEffects(hir);
if (env.isInferredMemoEnabled) {
if (referenceEffectsResult.fnEffectErrors.length > 0) {
CompilerError.throw(referenceEffectsResult.fnEffectErrors[0]);
}
}
log({kind: 'hir', name: 'InferReferenceEffects', value: hir});

validateLocalsNotReassignedAfterRender(hir);
Expand All @@ -239,28 +249,30 @@ function runWithEnvironment(
inferMutableRanges(hir);
log({kind: 'hir', name: 'InferMutableRanges', value: hir});

if (env.config.assertValidMutableRanges) {
assertValidMutableRanges(hir);
}
if (env.isInferredMemoEnabled) {
if (env.config.assertValidMutableRanges) {
assertValidMutableRanges(hir);
}

if (env.config.validateRefAccessDuringRender) {
validateNoRefAccessInRender(hir);
}
if (env.config.validateRefAccessDuringRender) {
validateNoRefAccessInRender(hir);
}

if (env.config.validateNoSetStateInRender) {
validateNoSetStateInRender(hir);
}
if (env.config.validateNoSetStateInRender) {
validateNoSetStateInRender(hir);
}

if (env.config.validateNoSetStateInPassiveEffects) {
validateNoSetStateInPassiveEffects(hir);
}
if (env.config.validateNoSetStateInPassiveEffects) {
validateNoSetStateInPassiveEffects(hir);
}

if (env.config.validateNoJSXInTryStatements) {
validateNoJSXInTryStatement(hir);
}
if (env.config.validateNoJSXInTryStatements) {
validateNoJSXInTryStatement(hir);
}

if (env.config.validateNoImpureFunctionsInRender) {
validateNoImpureFunctionsInRender(hir);
if (env.config.validateNoImpureFunctionsInRender) {
validateNoImpureFunctionsInRender(hir);
}
}

inferReactivePlaces(hir);
Expand All @@ -280,7 +292,12 @@ function runWithEnvironment(
value: hir,
});

if (!env.config.enableMinimalTransformsForRetry) {
if (env.isInferredMemoEnabled) {
/**
* Only create reactive scopes (which directly map to generated memo blocks)
* if inferred memoization is enabled. This makes all later passes which
* transform reactive-scope labeled instructions no-ops.
*/
inferReactiveScopeVariables(hir);
log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir});
}
Expand Down Expand Up @@ -529,6 +546,7 @@ export function compileFn(
>,
config: EnvironmentConfig,
fnType: ReactFunctionType,
mode: CompilerMode,
useMemoCacheIdentifier: string,
logger: Logger | null,
filename: string | null,
Expand All @@ -538,6 +556,7 @@ export function compileFn(
func,
config,
fnType,
mode,
useMemoCacheIdentifier,
logger,
filename,
Expand Down
9 changes: 4 additions & 5 deletions compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import {
EnvironmentConfig,
ExternalFunction,
ReactFunctionType,
MINIMAL_RETRY_CONFIG,
tryParseExternalFunction,
CompilerMode,
} from '../HIR/Environment';
import {CodegenFunction} from '../ReactiveScopes';
import {isComponentDeclaration} from '../Utils/ComponentDeclaration';
Expand Down Expand Up @@ -408,6 +408,7 @@ export function compileProgram(
fn,
environment,
fnType,
CompilerMode.AllFeatures,
useMemoCacheIdentifier.name,
pass.opts.logger,
pass.filename,
Expand All @@ -425,11 +426,9 @@ export function compileProgram(
kind: 'compile',
compiledFn: compileFn(
fn,
{
...environment,
...MINIMAL_RETRY_CONFIG,
},
environment,
fnType,
CompilerMode.NoInferredMemo,
useMemoCacheIdentifier.name,
pass.opts.logger,
pass.filename,
Expand Down
25 changes: 12 additions & 13 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ export const MacroSchema = z.union([
z.tuple([z.string(), z.array(MacroMethodSchema)]),
]);

export enum CompilerMode {
AllFeatures = 'all_features',
NoInferredMemo = 'no_inferred_memo',
}

export type Macro = z.infer<typeof MacroSchema>;
export type MacroMethod = z.infer<typeof MacroMethodSchema>;

Expand Down Expand Up @@ -550,8 +555,6 @@ const EnvironmentConfigSchema = z.object({
*/
disableMemoizationForDebugging: z.boolean().default(false),

enableMinimalTransformsForRetry: z.boolean().default(false),

/**
* When true, rather using memoized values, the compiler will always re-compute
* values, and then use a heuristic to compare the memoized value to the newly
Expand Down Expand Up @@ -626,17 +629,6 @@ const EnvironmentConfigSchema = z.object({

export type EnvironmentConfig = z.infer<typeof EnvironmentConfigSchema>;

export const MINIMAL_RETRY_CONFIG: PartialEnvironmentConfig = {
validateHooksUsage: false,
validateRefAccessDuringRender: false,
validateNoSetStateInRender: false,
validateNoSetStateInPassiveEffects: false,
validateNoJSXInTryStatements: false,
validateMemoizedEffectDependencies: false,
validateNoCapitalizedCalls: null,
validateBlocklistedImports: null,
enableMinimalTransformsForRetry: true,
};
/**
* For test fixtures and playground only.
*
Expand Down Expand Up @@ -851,6 +843,7 @@ export class Environment {
code: string | null;
config: EnvironmentConfig;
fnType: ReactFunctionType;
compilerMode: CompilerMode;
useMemoCacheIdentifier: string;
hasLoweredContextAccess: boolean;
hasFireRewrite: boolean;
Expand All @@ -861,6 +854,7 @@ export class Environment {
constructor(
scope: BabelScope,
fnType: ReactFunctionType,
compilerMode: CompilerMode,
config: EnvironmentConfig,
contextIdentifiers: Set<t.Identifier>,
logger: Logger | null,
Expand All @@ -870,6 +864,7 @@ export class Environment {
) {
this.#scope = scope;
this.fnType = fnType;
this.compilerMode = compilerMode;
this.config = config;
this.filename = filename;
this.code = code;
Expand Down Expand Up @@ -924,6 +919,10 @@ export class Environment {
this.#hoistedIdentifiers = new Set();
}

get isInferredMemoEnabled(): boolean {
return this.compilerMode !== CompilerMode.NoInferredMemo;
}

get nextIdentifierId(): IdentifierId {
return makeIdentifierId(this.#nextIdentifer++);
}
Expand Down
19 changes: 12 additions & 7 deletions compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
* LICENSE file in the root directory of this source tree.
*/

import {CompilerError, ErrorSeverity, ValueKind} from '..';
import {
CompilerError,
CompilerErrorDetailOptions,
ErrorSeverity,
ValueKind,
} from '..';
import {
AbstractValue,
BasicBlock,
Expand Down Expand Up @@ -290,21 +295,21 @@ export function inferTerminalFunctionEffects(
return functionEffects;
}

export function raiseFunctionEffectErrors(
export function transformFunctionEffectErrors(
functionEffects: Array<FunctionEffect>,
): void {
functionEffects.forEach(eff => {
): Array<CompilerErrorDetailOptions> {
return functionEffects.map(eff => {
switch (eff.kind) {
case 'ReactMutation':
case 'GlobalMutation': {
CompilerError.throw(eff.error);
return eff.error;
}
case 'ContextMutation': {
CompilerError.throw({
return {
severity: ErrorSeverity.Invariant,
reason: `Unexpected ContextMutation in top-level function effects`,
loc: eff.loc,
});
};
}
default:
assertExhaustive(
Expand Down
17 changes: 12 additions & 5 deletions compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import {CompilerError} from '../CompilerError';
import {CompilerError, CompilerErrorDetailOptions} from '../CompilerError';
import {Environment} from '../HIR';
import {
AbstractValue,
Expand Down Expand Up @@ -49,7 +49,7 @@ import {assertExhaustive} from '../Utils/utils';
import {
inferTerminalFunctionEffects,
inferInstructionFunctionEffects,
raiseFunctionEffectErrors,
transformFunctionEffectErrors,
} from './InferFunctionEffects';

const UndefinedValue: InstructionValue = {
Expand Down Expand Up @@ -103,7 +103,9 @@ const UndefinedValue: InstructionValue = {
export default function inferReferenceEffects(
fn: HIRFunction,
options: {isFunctionExpression: boolean} = {isFunctionExpression: false},
): void {
): {
fnEffectErrors: Array<CompilerErrorDetailOptions>;
} {
/*
* Initial state contains function params
* TODO: include module declarations here as well
Expand Down Expand Up @@ -241,8 +243,13 @@ export default function inferReferenceEffects(

if (options.isFunctionExpression) {
fn.effects = functionEffects;
} else if (!fn.env.config.enableMinimalTransformsForRetry) {
raiseFunctionEffectErrors(functionEffects);
return {
fnEffectErrors: [],
};
} else {
return {
fnEffectErrors: transformFunctionEffectErrors(functionEffects),
};
}
}

Expand Down

0 comments on commit d33f183

Please sign in to comment.