Compare commits

..

2 Commits

Author SHA1 Message Date
Joe Savona
2824f88468 [compiler] Cleanup: consistent tryRecord() wrapping and error recording 2026-02-23 16:07:44 -08:00
Joseph Savona
cebe42e245 [compiler] Add fault tolerance test fixtures (#35879)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35879).
* #35888
* #35884
* #35883
* #35882
* #35881
* #35880
* __->__ #35879
2026-02-23 16:06:39 -08:00
6 changed files with 46 additions and 30 deletions

View File

@@ -328,4 +328,6 @@ Walk through `runWithEnvironment` and wrap each pass call site. This is the inte
* **Partial HIR can trigger downstream invariants.** When lowering skips or partially handles constructs (e.g., unreachable hoisted functions, `var` declarations before the fix), downstream passes like `InferMutationAliasingEffects` may encounter uninitialized identifiers and throw invariants. This is acceptable since the function still correctly bails out of compilation, but error messages may be less specific. The fix for `var` (treating as `let`) demonstrates how to avoid this: continue lowering with a best-effort representation rather than skipping entirely.
* **Errors accumulated on `env` are lost when an invariant propagates out of the pipeline.** Since invariant CompilerErrors always re-throw through `tryRecord()`, they exit the pipeline as exceptions. The caller only sees the invariant error, not any errors previously recorded on `env`. This is a design limitation that could be addressed by aggregating env errors with caught exceptions in `tryCompileFunction()`.
* **Dedicated fault tolerance test fixtures** were added in `__tests__/fixtures/compiler/fault-tolerance/`. Each fixture combines two or more errors from different passes to verify the compiler reports all of them rather than short-circuiting on the first. Coverage includes: `var`+props mutation (BuildHIR→InferMutationAliasingEffects), `var`+ref access (BuildHIR→ValidateNoRefAccessInRender), `try/finally`+props mutation (BuildHIR→InferMutationAliasingEffects), `try/finally`+ref access (BuildHIR→ValidateNoRefAccessInRender), and a 3-error test combining try/finally+ref access+props mutation.
* **Cleanup: consistent `tryRecord()` wrapping in Pipeline.ts.** All validation passes and inference passes are now wrapped in `env.tryRecord()` for defense-in-depth, consistent with the approach used for transform passes. Previously only transform passes were wrapped. Merged duplicate `env.enableValidations` guard blocks. Pattern B lint-only passes (`env.logErrors()`) were intentionally not wrapped since they use a different error recording strategy.
* **Cleanup: normalized validation error recording pattern.** Four validation passes (`ValidateNoDerivedComputationsInEffects`, `ValidateMemoizedEffectDependencies`, `ValidatePreservedManualMemoization`, `ValidateSourceLocations`) were using `for (const detail of errors.details) { env.recordError(detail); }` instead of the simpler `env.recordErrors(errors)`. Normalized to use the batch method.

View File

@@ -161,8 +161,12 @@ function runWithEnvironment(
pruneMaybeThrows(hir);
log({kind: 'hir', name: 'PruneMaybeThrows', value: hir});
validateContextVariableLValues(hir);
validateUseMemo(hir);
env.tryRecord(() => {
validateContextVariableLValues(hir);
});
env.tryRecord(() => {
validateUseMemo(hir);
});
if (env.enableDropManualMemoization) {
dropManualMemoization(hir);
@@ -198,10 +202,14 @@ function runWithEnvironment(
if (env.enableValidations) {
if (env.config.validateHooksUsage) {
validateHooksUsage(hir);
env.tryRecord(() => {
validateHooksUsage(hir);
});
}
if (env.config.validateNoCapitalizedCalls) {
validateNoCapitalizedCalls(hir);
env.tryRecord(() => {
validateNoCapitalizedCalls(hir);
});
}
}
@@ -211,7 +219,9 @@ function runWithEnvironment(
analyseFunctions(hir);
log({kind: 'hir', name: 'AnalyseFunctions', value: hir});
inferMutationAliasingEffects(hir);
env.tryRecord(() => {
inferMutationAliasingEffects(hir);
});
log({kind: 'hir', name: 'InferMutationAliasingEffects', value: hir});
if (env.outputMode === 'ssr') {
@@ -225,25 +235,31 @@ function runWithEnvironment(
pruneMaybeThrows(hir);
log({kind: 'hir', name: 'PruneMaybeThrows', value: hir});
inferMutationAliasingRanges(hir, {
isFunctionExpression: false,
env.tryRecord(() => {
inferMutationAliasingRanges(hir, {
isFunctionExpression: false,
});
});
log({kind: 'hir', name: 'InferMutationAliasingRanges', value: hir});
if (env.enableValidations) {
validateLocalsNotReassignedAfterRender(hir);
}
env.tryRecord(() => {
validateLocalsNotReassignedAfterRender(hir);
});
if (env.enableValidations) {
if (env.config.assertValidMutableRanges) {
assertValidMutableRanges(hir);
}
if (env.config.validateRefAccessDuringRender) {
validateNoRefAccessInRender(hir);
env.tryRecord(() => {
validateNoRefAccessInRender(hir);
});
}
if (env.config.validateNoSetStateInRender) {
validateNoSetStateInRender(hir);
env.tryRecord(() => {
validateNoSetStateInRender(hir);
});
}
if (
@@ -252,7 +268,9 @@ function runWithEnvironment(
) {
env.logErrors(validateNoDerivedComputationsInEffects_exp(hir));
} else if (env.config.validateNoDerivedComputationsInEffects) {
validateNoDerivedComputationsInEffects(hir);
env.tryRecord(() => {
validateNoDerivedComputationsInEffects(hir);
});
}
if (env.config.validateNoSetStateInEffects && env.outputMode === 'lint') {
@@ -277,7 +295,9 @@ function runWithEnvironment(
env.config.validateExhaustiveEffectDependencies
) {
// NOTE: this relies on reactivity inference running first
validateExhaustiveDependencies(hir);
env.tryRecord(() => {
validateExhaustiveDependencies(hir);
});
}
}
@@ -506,7 +526,9 @@ function runWithEnvironment(
env.config.enablePreserveExistingMemoizationGuarantees ||
env.config.validatePreserveExistingMemoizationGuarantees
) {
validatePreservedManualMemoization(reactiveFunction);
env.tryRecord(() => {
validatePreservedManualMemoization(reactiveFunction);
});
}
const ast = codegenFunction(reactiveFunction, {
@@ -519,7 +541,9 @@ function runWithEnvironment(
}
if (env.config.validateSourceLocations) {
validateSourceLocations(func, ast, env);
env.tryRecord(() => {
validateSourceLocations(func, ast, env);
});
}
/**

View File

@@ -217,11 +217,7 @@ export function lower(
if (err instanceof CompilerError) {
// Re-throw invariant errors immediately
for (const detail of err.details) {
if (
(detail instanceof CompilerDiagnostic
? detail.category
: detail.category) === ErrorCategory.Invariant
) {
if (detail.category === ErrorCategory.Invariant) {
throw err;
}
}

View File

@@ -97,9 +97,7 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
}
}
}
for (const detail of errors.details) {
fn.env.recordError(detail);
}
fn.env.recordErrors(errors);
}
function validateEffect(

View File

@@ -52,9 +52,7 @@ export function validatePreservedManualMemoization(fn: ReactiveFunction): void {
manualMemoState: null,
};
visitReactiveFunction(fn, new Visitor(), state);
for (const detail of state.errors.details) {
fn.env.recordError(detail);
}
fn.env.recordErrors(state.errors);
}
const DEBUG = false;

View File

@@ -310,7 +310,5 @@ export function validateSourceLocations(
}
}
for (const detail of errors.details) {
env.recordError(detail);
}
env.recordErrors(errors);
}