[compiler] Add fault tolerance plan document by everettbu · Pull Request #560 · greptileai/react-mirror

everettbu · 2026-02-21T03:59:15Z

Mirror of facebook/react#35840
Original author: josephsavona

Add detailed plan for making the React Compiler fault-tolerant by
accumulating errors across all passes instead of stopping at the first
error. This enables reporting multiple compilation errors at once.

Stack created with Sapling. Best reviewed with ReviewStack.

#35853
#35852
#35851
#35850
#35849
#35848
#35847
#35845
#35844
#35843
#35842
#35841
-> #35840

Add detailed plan for making the React Compiler fault-tolerant by accumulating errors across all passes instead of stopping at the first error. This enables reporting multiple compilation errors at once.

greptile-apps · 2026-02-21T04:02:23Z

Greptile Summary

This PR adds a detailed plan document (compiler/fault-tolerance-overview.md) describing how to make the React Compiler fault-tolerant by accumulating errors across all compilation passes instead of halting at the first error. The document is well-organized into 8 phases covering infrastructure, pipeline changes, HIR lowering, validation passes, inference passes, codegen, integration, and testing.

The overall architecture and approach (error accumulation on Environment, tryRecord wrapper, incremental migration path) is sound and aligns with the existing codebase patterns.
Multiple Pipeline.ts line number references are inaccurate (off by ~10-60 lines), which could cause confusion during implementation. For example, lower() is cited at line 163 but is at 156; codegenFunction is cited at lines 575-578 but is at 517-520; validatePreservedManualMemoization is cited at line 572 but is at 514.
Minor typo on line 20: "construct record the todo error" should be "record the todo error".

Confidence Score: 4/5

This PR is safe to merge — it is a documentation-only change with no runtime impact.
This is a plan document with no code changes, so there is zero risk to runtime behavior. The score is 4 instead of 5 because the document contains inaccurate line number references that could mislead implementers.
compiler/fault-tolerance-overview.md has inaccurate line references that should be corrected before using this document to guide implementation.

Important Files Changed

Filename	Overview
compiler/fault-tolerance-overview.md	New plan document for compiler fault tolerance. Well-structured with 8 phases, but contains numerous inaccurate line number references to Pipeline.ts (off by ~10-60 lines) and a minor typo in the Detailed Design section.

_{Last reviewed commit: 056a8e1}

greptile-apps

_{1 file reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-21T04:02:27Z

+- [ ] **4.1 `validateHooksUsage`** (`src/Validation/ValidateHooksUsage.ts`)
+  - Change signature from `(fn: HIRFunction): Result<void, CompilerError>` to `(fn: HIRFunction): void`
+  - Record errors on `fn.env` instead of returning `errors.asResult()`
+  - Update Pipeline.ts call site (line 211): remove `.unwrap()`
+
+- [ ] **4.2 `validateNoCapitalizedCalls`** (`src/Validation/ValidateNoCapitalizedCalls.ts`)
+  - Change signature to return void
+  - Fix the hybrid pattern: the direct `CallExpression` path currently throws via `CompilerError.throwInvalidReact()` — change to record on env
+  - The `MethodCall` path already accumulates — change to record on env
+  - Update Pipeline.ts call site (line 214): remove `.unwrap()`
+
+- [ ] **4.3 `validateUseMemo`** (`src/Validation/ValidateUseMemo.ts`)
+  - Change signature to return void
+  - Record hard errors on env instead of returning `errors.asResult()`
+  - The soft `voidMemoErrors` path already uses `env.logErrors()` — keep as-is or also record
+  - Update Pipeline.ts call site (line 170): remove `.unwrap()`
+
+- [ ] **4.4 `dropManualMemoization`** (`src/Inference/DropManualMemoization.ts`)
+  - Change signature to return void
+  - Record errors on env instead of returning `errors.asResult()`
+  - Update Pipeline.ts call site (line 178): remove `.unwrap()`
+
+- [ ] **4.5 `validateNoRefAccessInRender`** (`src/Validation/ValidateNoRefAccessInRender.ts`)
+  - Change signature to return void
+  - Record errors on env instead of returning Result
+  - Update Pipeline.ts call site (line 275): remove `.unwrap()`
+
+- [ ] **4.6 `validateNoSetStateInRender`** (`src/Validation/ValidateNoSetStateInRender.ts`)
+  - Change signature to return void
+  - Record errors on env
+  - Update Pipeline.ts call site (line 279): remove `.unwrap()`
+
+- [ ] **4.7 `validateNoImpureFunctionsInRender`** (`src/Validation/ValidateNoImpureFunctionsInRender.ts`)
+  - Change signature to return void
+  - Record errors on env
+  - Update Pipeline.ts call site (line 300): remove `.unwrap()`
+
+- [ ] **4.8 `validateNoFreezingKnownMutableFunctions`** (`src/Validation/ValidateNoFreezingKnownMutableFunctions.ts`)
+  - Change signature to return void
+  - Record errors on env
+  - Update Pipeline.ts call site (line 303): remove `.unwrap()`
+
+- [ ] **4.9 `validateExhaustiveDependencies`** (`src/Validation/ValidateExhaustiveDependencies.ts`)
+  - Change signature to return void
+  - Record errors on env
+  - Update Pipeline.ts call site (line 315): remove `.unwrap()`
+
+- [ ] **4.10 `validateMemoizedEffectDependencies`** (`src/Validation/ValidateMemoizedEffectDependencies.ts`)
+  - Change signature to return void (note: operates on `ReactiveFunction`)
+  - Record errors on the function's env
+  - Update Pipeline.ts call site (line 565): remove `.unwrap()`
+
+- [ ] **4.11 `validatePreservedManualMemoization`** (`src/Validation/ValidatePreservedManualMemoization.ts`)
+  - Change signature to return void (note: operates on `ReactiveFunction`)
+  - Record errors on the function's env
+  - Update Pipeline.ts call site (line 572): remove `.unwrap()`
+
+- [ ] **4.12 `validateSourceLocations`** (`src/Validation/ValidateSourceLocations.ts`)
+  - Change signature to return void
+  - Record errors on env
+  - Update Pipeline.ts call site (line 585): remove `.unwrap()`


Inaccurate line references for Pipeline.ts

Several line numbers cited for Pipeline.ts do not match the actual codebase. The plan says validateHooksUsage is at line 211 (it's at 199), validateNoCapitalizedCalls at line 214 (it's at 202), inferMutationAliasingEffects at lines 233-239 (it's at 212-218), inferMutationAliasingRanges at lines 258-267 (it's at 231-240), validateNoRefAccessInRender at line 275 (it's at 248), validateNoSetStateInRender at line 279 (it's at 252), validateNoImpureFunctionsInRender at line 300 (it's at 273), validateNoFreezingKnownMutableFunctions at line 303 (it's at 276), validateExhaustiveDependencies at line 315 (it's at 288), validateMemoizedEffectDependencies at line 565 (no such call exists at that line), validatePreservedManualMemoization at line 572 (it's at 514), validateSourceLocations at line 585 (it's at 527), codegenFunction at lines 575-578 (it's at 517-520), and env.logErrors() calls at lines 286-331 (actually around 259-304).

Since these line references are intended to guide implementation, the inaccuracies could cause confusion during development. Consider either correcting them or removing specific line numbers in favor of function/identifier-based references which are more resilient to code changes.

Prompt To Fix With AI

This is a comment left during a code review. Path: compiler/fault-tolerance-overview.md Line: 130-190 Comment: **Inaccurate line references for Pipeline.ts** Several line numbers cited for `Pipeline.ts` do not match the actual codebase. The plan says `validateHooksUsage` is at line 211 (it's at 199), `validateNoCapitalizedCalls` at line 214 (it's at 202), `inferMutationAliasingEffects` at lines 233-239 (it's at 212-218), `inferMutationAliasingRanges` at lines 258-267 (it's at 231-240), `validateNoRefAccessInRender` at line 275 (it's at 248), `validateNoSetStateInRender` at line 279 (it's at 252), `validateNoImpureFunctionsInRender` at line 300 (it's at 273), `validateNoFreezingKnownMutableFunctions` at line 303 (it's at 276), `validateExhaustiveDependencies` at line 315 (it's at 288), `validateMemoizedEffectDependencies` at line 565 (no such call exists at that line), `validatePreservedManualMemoization` at line 572 (it's at 514), `validateSourceLocations` at line 585 (it's at 527), `codegenFunction` at lines 575-578 (it's at 517-520), and `env.logErrors()` calls at lines 286-331 (actually around 259-304). Since these line references are intended to guide implementation, the inaccuracies could cause confusion during development. Consider either correcting them or removing specific line numbers in favor of function/identifier-based references which are more resilient to code changes. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-21T04:02:28Z

+## Detailed Design
+
+* The Environment needs a way to record errors as compilation proceeds. This should generally store the error (and log, if a logger is configured), but should immediately throw if the error is an invariant (see above).
+* BuildHIR should always produce an HIR without error. For syntax forms that are unsupported (currently throwing a Todo error), we should instead construct record the todo error on the environment, and construct a partial HIR. The exact form of the partial HIR can be situation specific:


Typo: "construct record" should be "construct, record" or "record"

This sentence reads: "we should instead construct record the todo error on the environment, and construct a partial HIR." It appears to have a duplicated/garbled verb. It likely should be "we should instead record the todo error on the environment, and construct a partial HIR."

Prompt To Fix With AI

This is a comment left during a code review. Path: compiler/fault-tolerance-overview.md Line: 20 Comment: **Typo: "construct record" should be "construct, record" or "record"** This sentence reads: "we should instead construct record the todo error on the environment, and construct a partial HIR." It appears to have a duplicated/garbled verb. It likely should be "we should instead **record** the todo error on the environment, and construct a partial HIR." How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-21T04:02:29Z

+- [ ] **4.3 `validateUseMemo`** (`src/Validation/ValidateUseMemo.ts`)
+  - Change signature to return void
+  - Record hard errors on env instead of returning `errors.asResult()`
+  - The soft `voidMemoErrors` path already uses `env.logErrors()` — keep as-is or also record
+  - Update Pipeline.ts call site (line 170): remove `.unwrap()`


validateUseMemo line reference is wrong (163 vs 170)

Step 4.3 says the validateUseMemo call site is at Pipeline.ts line 170, but it is actually at line 163 (validateUseMemo(hir).unwrap()). Line 170 in Pipeline.ts is inlineImmediatelyInvokedFunctionExpressions(hir), a completely different pass.

Prompt To Fix With AI

This is a comment left during a code review. Path: compiler/fault-tolerance-overview.md Line: 141-145 Comment: **`validateUseMemo` line reference is wrong (163 vs 170)** Step 4.3 says the `validateUseMemo` call site is at Pipeline.ts line 170, but it is actually at line 163 (`validateUseMemo(hir).unwrap()`). Line 170 in Pipeline.ts is `inlineImmediatelyInvokedFunctionExpressions(hir)`, a completely different pass. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-21T04:02:30Z

+- [ ] **4.4 `dropManualMemoization`** (`src/Inference/DropManualMemoization.ts`)
+  - Change signature to return void
+  - Record errors on env instead of returning `errors.asResult()`
+  - Update Pipeline.ts call site (line 178): remove `.unwrap()`


dropManualMemoization line reference is wrong (166 vs 178)

Step 4.4 says the dropManualMemoization call site is at Pipeline.ts line 178, but it is actually at line 166 (dropManualMemoization(hir).unwrap()). Line 178 is mergeConsecutiveBlocks.

Prompt To Fix With AI

This is a comment left during a code review. Path: compiler/fault-tolerance-overview.md Line: 147-150 Comment: **`dropManualMemoization` line reference is wrong (166 vs 178)** Step 4.4 says the `dropManualMemoization` call site is at Pipeline.ts line 178, but it is actually at line 166 (`dropManualMemoization(hir).unwrap()`). Line 178 is `mergeConsecutiveBlocks`. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-21T04:02:31Z

+- [ ] **7.1 Wrap `lower()` call** (line 163)
+  - Change from `lower(func, env).unwrap()` to `lower(func, env)` (direct return after Phase 3.1)


Pipeline.ts lower() call is at line 156, not 163

Step 7.1 says the lower() call is at line 163, but it is actually at line 156 (const hir = lower(func, env).unwrap()). Line 163 is validateUseMemo.

Prompt To Fix With AI

This is a comment left during a code review. Path: compiler/fault-tolerance-overview.md Line: 251-252 Comment: **Pipeline.ts `lower()` call is at line 156, not 163** Step 7.1 says the `lower()` call is at line 163, but it is actually at line 156 (`const hir = lower(func, env).unwrap()`). Line 163 is `validateUseMemo`. How can I resolve this? If you propose a fix, please make it concise.

everettbu · 2026-02-23T17:35:00Z

Upstream PR was closed or merged. Code is synced via branch mirror.

[compiler] Add fault tolerance plan document

056a8e1

Add detailed plan for making the React Compiler fault-tolerant by accumulating errors across all passes instead of stopping at the first error. This enables reporting multiple compilation errors at once.

everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 21, 2026

greptile-apps Bot reviewed Feb 21, 2026

View reviewed changes

everettbu closed this Feb 23, 2026

everettbu deleted the pr35840 branch February 23, 2026 17:35

everettbu restored the pr35840 branch February 23, 2026 18:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[compiler] Add fault tolerance plan document#560

[compiler] Add fault tolerance plan document#560
everettbu wants to merge 1 commit into
mainfrom
pr35840

everettbu commented Feb 21, 2026

Uh oh!

greptile-apps Bot commented Feb 21, 2026

Uh oh!

greptile-apps Bot left a comment

Uh oh!

greptile-apps Bot Feb 21, 2026

Uh oh!

greptile-apps Bot Feb 21, 2026

Uh oh!

greptile-apps Bot Feb 21, 2026

Uh oh!

greptile-apps Bot Feb 21, 2026

Uh oh!

greptile-apps Bot Feb 21, 2026

Uh oh!

everettbu commented Feb 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		- [ ] 7.1 Wrap `lower()` call (line 163)
		- Change from `lower(func, env).unwrap()` to `lower(func, env)` (direct return after Phase 3.1)

Uh oh!

Conversation

everettbu commented Feb 21, 2026

Uh oh!

greptile-apps Bot commented Feb 21, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

everettbu commented Feb 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants