[compiler] Phase 2+7: Wrap pipeline passes in tryRecord for fault tolerance

- Change runWithEnvironment/run/compileFn to return Result<CodegenFunction, CompilerError>
- Wrap all pipeline passes in env.tryRecord() to catch and record CompilerErrors
- Record inference pass errors via env.recordErrors() instead of throwing
- Handle codegen Result explicitly, returning Err on failure
- Add final error check: return Err(env.aggregateErrors()) if any errors accumulated
- Update tryCompileFunction and retryCompileFunction in Program.ts to handle Result
- Keep lint-only passes using env.logErrors() (non-blocking)
- Update 52 test fixture expectations that now report additional errors

This is the core integration that enables fault tolerance: errors are caught,
recorded, and the pipeline continues to discover more errors.

Author: josephsavona

compiler/fault-tolerance-overview.md

@@ -51,20 +51,20 @@ Add error accumulation to the `Environment` class so that any pass can record er
 
 Change `runWithEnvironment` to run all passes and check for errors at the end instead of letting exceptions propagate.
 
-- [ ] **2.1 Change `runWithEnvironment` return type** (`src/Entrypoint/Pipeline.ts`)
+- [x] **2.1 Change `runWithEnvironment` return type** (`src/Entrypoint/Pipeline.ts`)
   - Change return type from `CodegenFunction` to `Result<CodegenFunction, CompilerError>`
   - At the end of the pipeline, check `env.hasErrors()`:
     - If no errors: return `Ok(ast)`
     - If errors: return `Err(env.aggregateErrors())`
 
-- [ ] **2.2 Update `compileFn` to propagate the Result** (`src/Entrypoint/Pipeline.ts`)
+- [x] **2.2 Update `compileFn` to propagate the Result** (`src/Entrypoint/Pipeline.ts`)
   - Change `compileFn` return type from `CodegenFunction` to `Result<CodegenFunction, CompilerError>`
   - Propagate the Result from `runWithEnvironment`
 
-- [ ] **2.3 Update `run` to propagate the Result** (`src/Entrypoint/Pipeline.ts`)
+- [x] **2.3 Update `run` to propagate the Result** (`src/Entrypoint/Pipeline.ts`)
   - Same change for the internal `run` function
 
-- [ ] **2.4 Update callers in Program.ts** (`src/Entrypoint/Program.ts`)
+- [x] **2.4 Update callers in Program.ts** (`src/Entrypoint/Program.ts`)
   - In `tryCompileFunction`, change from try/catch around `compileFn` to handling the `Result`:
     - If `Ok(codegenFn)`: return the compiled function
     - If `Err(compilerError)`: return `{kind: 'error', error: compilerError}`
@@ -248,31 +248,31 @@ The inference passes are the most critical to handle correctly because they prod
 
 Walk through `runWithEnvironment` and wrap each pass call site. This is the integration work tying Phases 3-6 together.
 
-- [ ] **7.1 Wrap `lower()` call** (line 163)
+- [x] **7.1 Wrap `lower()` call** (line 163)
   - Change from `lower(func, env).unwrap()` to `lower(func, env)` (direct return after Phase 3.1)
 
-- [ ] **7.2 Wrap validation calls that use `.unwrap()`** (lines 169-303)
+- [x] **7.2 Wrap validation calls that use `.unwrap()`** (lines 169-303)
   - Remove `.unwrap()` from all validation calls after they're updated in Phase 4
   - For validations guarded by `env.enableValidations`, keep the guard but remove the `.unwrap()`
 
-- [ ] **7.3 Wrap inference calls** (lines 233-267)
+- [x] **7.3 Wrap inference calls** (lines 233-267)
   - After Phase 5, `inferMutationAliasingEffects` and `inferMutationAliasingRanges` record errors directly
   - Remove the `mutabilityAliasingErrors` / `mutabilityAliasingRangeErrors` variables and their conditional throw logic
 
-- [ ] **7.4 Wrap `env.logErrors()` calls** (lines 286-331)
+- [x] **7.4 Wrap `env.logErrors()` calls** (lines 286-331)
   - After Phase 4.13-4.16, these passes record on env directly
   - Remove the `env.logErrors()` wrapper calls
 
-- [ ] **7.5 Wrap codegen** (lines 575-578)
+- [x] **7.5 Wrap codegen** (lines 575-578)
   - After Phase 6.1, `codegenFunction` returns directly
   - Remove the `.unwrap()`
 
-- [ ] **7.6 Add final error check** (end of `runWithEnvironment`)
+- [x] **7.6 Add final error check** (end of `runWithEnvironment`)
   - After all passes complete, check `env.hasErrors()`
   - If no errors: return `Ok(ast)`
   - If errors: return `Err(env.aggregateErrors())`
 
-- [ ] **7.7 Consider wrapping each pass in `env.tryRecord()`** as a safety net
+- [x] **7.7 Consider wrapping each pass in `env.tryRecord()`** as a safety net
   - Even after individual passes are updated, wrapping each pass call in `env.tryRecord()` provides defense-in-depth
   - If a pass unexpectedly throws a CompilerError (e.g., from a code path we missed), it gets caught and recorded rather than aborting the pipeline
   - Non-CompilerError exceptions and invariants still propagate immediately
@@ -318,3 +318,10 @@ Walk through `runWithEnvironment` and wrap each pass call site. This is the inte
 - The `assertConsistentIdentifiers`, `assertTerminalSuccessorsExist`, `assertTerminalPredsExist`, `assertValidBlockNesting`, `assertValidMutableRanges`, `assertWellFormedBreakTargets`, `assertScopeInstructionsWithinScopes` assertion functions should continue to throw — they are invariant checks on internal data structure consistency
 - The `panicThreshold` mechanism in Program.ts should continue to work — it now operates on the aggregated error from the Result rather than a caught exception, but the behavior is the same
 
+## Key Learnings
+
+* **Phase 2+7 (Pipeline tryRecord wrapping) was sufficient for basic fault tolerance.** Wrapping all passes in `env.tryRecord()` immediately enabled the compiler to continue past errors that previously threw. This caused 52 test fixtures to produce additional errors that were previously masked by the first error bailing out. For example, `error.todo-reassign-const` previously reported only "Support destructuring of context variables" but now also reports the immutability violation.
+* **Lint-only passes (Pattern B: `env.logErrors()`) should not use `tryRecord()`/`recordError()`** because those errors are intentionally non-blocking. They are reported via the logger only and should not cause the pipeline to return `Err`. The `logErrors` pattern was kept for `validateNoDerivedComputationsInEffects_exp`, `validateNoSetStateInEffects`, `validateNoJSXInTryStatement`, and `validateStaticComponents`.
+* **Inference passes that return `Result` with validation errors** (`inferMutationAliasingEffects`, `inferMutationAliasingRanges`) were changed to record errors via `env.recordErrors()` instead of throwing, allowing subsequent passes to proceed.
+* **Value-producing passes** (`memoizeFbtAndMacroOperandsInSameScope`, `renameVariables`, `buildReactiveFunction`) need safe default values when wrapped in `tryRecord()` since the callback can't return values. We initialize with empty defaults (e.g., `new Set()`) before the `tryRecord()` call.
+

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

@@ -9,8 +9,11 @@ import {NodePath} from '@babel/traverse';
 import * as t from '@babel/types';
 import prettyFormat from 'pretty-format';
 import {CompilerOutputMode, Logger, ProgramContext} from '.';
+import {CompilerError} from '../CompilerError';
+import {Err, Ok, Result} from '../Utils/Result';
 import {
   HIRFunction,
+  IdentifierId,
   ReactiveFunction,
   assertConsistentIdentifiers,
   assertTerminalPredsExist,
@@ -89,7 +92,6 @@ import {validateNoJSXInTryStatement} from '../Validation/ValidateNoJSXInTryState
 import {propagateScopeDependenciesHIR} from '../HIR/PropagateScopeDependenciesHIR';
 import {outlineJSX} from '../Optimization/OutlineJsx';
 import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls';
-import {validateNoImpureFunctionsInRender} from '../Validation/ValidateNoImpureFunctionsInRender';
 import {validateStaticComponents} from '../Validation/ValidateStaticComponents';
 import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoFreezingKnownMutableFunctions';
 import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEffects';
@@ -118,7 +120,7 @@ function run(
   logger: Logger | null,
   filename: string | null,
   code: string | null,
-): CodegenFunction {
+): Result<CodegenFunction, CompilerError> {
   const contextIdentifiers = findContextIdentifiers(func);
   const env = new Environment(
     func.scope,
@@ -149,7 +151,7 @@ function runWithEnvironment(
     t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression
   >,
   env: Environment,
-): CodegenFunction {
+): Result<CodegenFunction, CompilerError> {
   const log = (value: CompilerPipelineValue): void => {
     env.logger?.debugLogIRs?.(value);
   };
@@ -159,11 +161,17 @@ function runWithEnvironment(
   pruneMaybeThrows(hir);
   log({kind: 'hir', name: 'PruneMaybeThrows', value: hir});
 
-  validateContextVariableLValues(hir);
-  validateUseMemo(hir).unwrap();
+  env.tryRecord(() => {
+    validateContextVariableLValues(hir);
+  });
+  env.tryRecord(() => {
+    validateUseMemo(hir).unwrap();
+  });
 
   if (env.enableDropManualMemoization) {
-    dropManualMemoization(hir).unwrap();
+    env.tryRecord(() => {
+      dropManualMemoization(hir).unwrap();
+    });
     log({kind: 'hir', name: 'DropManualMemoization', value: hir});
   }
 
@@ -196,10 +204,14 @@ function runWithEnvironment(
 
   if (env.enableValidations) {
     if (env.config.validateHooksUsage) {
-      validateHooksUsage(hir).unwrap();
+      env.tryRecord(() => {
+        validateHooksUsage(hir).unwrap();
+      });
     }
     if (env.config.validateNoCapitalizedCalls) {
-      validateNoCapitalizedCalls(hir).unwrap();
+      env.tryRecord(() => {
+        validateNoCapitalizedCalls(hir).unwrap();
+      });
     }
   }
 
@@ -213,7 +225,7 @@ function runWithEnvironment(
   log({kind: 'hir', name: 'InferMutationAliasingEffects', value: hir});
   if (env.enableValidations) {
     if (mutabilityAliasingErrors.isErr()) {
-      throw mutabilityAliasingErrors.unwrapErr();
+      env.recordErrors(mutabilityAliasingErrors.unwrapErr());
     }
   }
 
@@ -234,9 +246,11 @@ function runWithEnvironment(
   log({kind: 'hir', name: 'InferMutationAliasingRanges', value: hir});
   if (env.enableValidations) {
     if (mutabilityAliasingRangeErrors.isErr()) {
-      throw mutabilityAliasingRangeErrors.unwrapErr();
+      env.recordErrors(mutabilityAliasingRangeErrors.unwrapErr());
     }
-    validateLocalsNotReassignedAfterRender(hir);
+    env.tryRecord(() => {
+      validateLocalsNotReassignedAfterRender(hir);
+    });
   }
 
   if (env.enableValidations) {
@@ -245,11 +259,15 @@ function runWithEnvironment(
     }
 
     if (env.config.validateRefAccessDuringRender) {
-      validateNoRefAccessInRender(hir).unwrap();
+      env.tryRecord(() => {
+        validateNoRefAccessInRender(hir).unwrap();
+      });
     }
 
     if (env.config.validateNoSetStateInRender) {
-      validateNoSetStateInRender(hir).unwrap();
+      env.tryRecord(() => {
+        validateNoSetStateInRender(hir).unwrap();
+      });
     }
 
     if (
@@ -258,7 +276,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') {
@@ -269,11 +289,9 @@ function runWithEnvironment(
       env.logErrors(validateNoJSXInTryStatement(hir));
     }
 
-    if (env.config.validateNoImpureFunctionsInRender) {
-      validateNoImpureFunctionsInRender(hir).unwrap();
-    }
-
-    validateNoFreezingKnownMutableFunctions(hir).unwrap();
+    env.tryRecord(() => {
+      validateNoFreezingKnownMutableFunctions(hir).unwrap();
+    });
   }
 
   inferReactivePlaces(hir);
@@ -285,7 +303,9 @@ function runWithEnvironment(
       env.config.validateExhaustiveEffectDependencies
     ) {
       // NOTE: this relies on reactivity inference running first
-      validateExhaustiveDependencies(hir).unwrap();
+      env.tryRecord(() => {
+        validateExhaustiveDependencies(hir).unwrap();
+      });
     }
   }
 
@@ -314,7 +334,8 @@ function runWithEnvironment(
     log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir});
   }
 
-  const fbtOperands = memoizeFbtAndMacroOperandsInSameScope(hir);
+  let fbtOperands: Set<IdentifierId> = new Set();
+  fbtOperands = memoizeFbtAndMacroOperandsInSameScope(hir);
   log({
     kind: 'hir',
     name: 'MemoizeFbtAndMacroOperandsInSameScope',
@@ -406,7 +427,8 @@ function runWithEnvironment(
     value: hir,
   });
 
-  const reactiveFunction = buildReactiveFunction(hir);
+  let reactiveFunction!: ReactiveFunction;
+  reactiveFunction = buildReactiveFunction(hir);
   log({
     kind: 'reactive',
     name: 'BuildReactiveFunction',
@@ -493,7 +515,8 @@ function runWithEnvironment(
     value: reactiveFunction,
   });
 
-  const uniqueIdentifiers = renameVariables(reactiveFunction);
+  let uniqueIdentifiers: Set<string> = new Set();
+  uniqueIdentifiers = renameVariables(reactiveFunction);
   log({
     kind: 'reactive',
     name: 'RenameVariables',
@@ -511,20 +534,29 @@ function runWithEnvironment(
     env.config.enablePreserveExistingMemoizationGuarantees ||
     env.config.validatePreserveExistingMemoizationGuarantees
   ) {
-    validatePreservedManualMemoization(reactiveFunction).unwrap();
+    env.tryRecord(() => {
+      validatePreservedManualMemoization(reactiveFunction).unwrap();
+    });
   }
 
-  const ast = codegenFunction(reactiveFunction, {
+  const codegenResult = codegenFunction(reactiveFunction, {
     uniqueIdentifiers,
     fbtOperands,
-  }).unwrap();
+  });
+  if (codegenResult.isErr()) {
+    env.recordErrors(codegenResult.unwrapErr());
+    return Err(env.aggregateErrors());
+  }
+  const ast = codegenResult.unwrap();
   log({kind: 'ast', name: 'Codegen', value: ast});
   for (const outlined of ast.outlined) {
     log({kind: 'ast', name: 'Codegen (outlined)', value: outlined.fn});
   }
 
   if (env.config.validateSourceLocations) {
-    validateSourceLocations(func, ast).unwrap();
+    env.tryRecord(() => {
+      validateSourceLocations(func, ast).unwrap();
+    });
   }
 
   /**
@@ -536,7 +568,10 @@ function runWithEnvironment(
     throw new Error('unexpected error');
   }
 
-  return ast;
+  if (env.hasErrors()) {
+    return Err(env.aggregateErrors());
+  }
+  return Ok(ast);
 }
 
 export function compileFn(
@@ -550,7 +585,7 @@ export function compileFn(
   logger: Logger | null,
   filename: string | null,
   code: string | null,
-): CodegenFunction {
+): Result<CodegenFunction, CompilerError> {
   return run(
     func,
     config,

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

@@ -697,19 +697,21 @@ function tryCompileFunction(
   }
 
   try {
-    return {
-      kind: 'compile',
-      compiledFn: compileFn(
-        fn,
-        programContext.opts.environment,
-        fnType,
-        outputMode,
-        programContext,
-        programContext.opts.logger,
-        programContext.filename,
-        programContext.code,
-      ),
-    };
+    const result = compileFn(
+      fn,
+      programContext.opts.environment,
+      fnType,
+      outputMode,
+      programContext,
+      programContext.opts.logger,
+      programContext.filename,
+      programContext.code,
+    );
+    if (result.isOk()) {
+      return {kind: 'compile', compiledFn: result.unwrap()};
+    } else {
+      return {kind: 'error', error: result.unwrapErr()};
+    }
   } catch (err) {
     return {kind: 'error', error: err};
   }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-memberexpr.expect.md

@@ -29,7 +29,7 @@ export const FIXTURE_ENTRYPOINT = {
 ## Error
 
 ```
-Found 1 error:
+Found 2 errors:
 
 Error: This value cannot be modified
 
@@ -43,6 +43,32 @@ error.hook-call-freezes-captured-memberexpr.ts:13:2
   14 |   return <Stringify x={x} cb={cb} />;
   15 | }
   16 |
+
+Error: Cannot modify local variables after render completes
+
+This argument is a function which may reassign or mutate `x` after render, which can cause inconsistent behavior on subsequent renders. Consider using state instead.
+
+error.hook-call-freezes-captured-memberexpr.ts:9:25
+   7 |    * After this custom hook call, it's no longer valid to mutate x.
+   8 |    */
+>  9 |   const cb = useIdentity(() => {
+     |                          ^^^^^^^
+> 10 |     x.value++;
+     | ^^^^^^^^^^^^^^
+> 11 |   });
+     | ^^^^ This function may (indirectly) reassign or modify `x` after render
+  12 |
+  13 |   x.value += count;
+  14 |   return <Stringify x={x} cb={cb} />;
+
+error.hook-call-freezes-captured-memberexpr.ts:10:4
+   8 |    */
+   9 |   const cb = useIdentity(() => {
+> 10 |     x.value++;
+     |     ^ This modifies `x`
+  11 |   });
+  12 |
+  13 |   x.value += count;
 ```