Compare commits

..

1 Commits

Author SHA1 Message Date
Joe Savona
9d81bee358 [compiler] Cleanup: consistent tryRecord() wrapping and error recording 2026-02-20 18:03:20 -08:00
7 changed files with 60 additions and 35 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

@@ -164,8 +164,12 @@ function runWithEnvironment(
});
log({kind: 'hir', name: 'PruneMaybeThrows', value: hir});
validateContextVariableLValues(hir);
validateUseMemo(hir);
env.tryRecord(() => {
validateContextVariableLValues(hir);
});
env.tryRecord(() => {
validateUseMemo(hir);
});
if (env.enableDropManualMemoization) {
dropManualMemoization(hir);
@@ -213,10 +217,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);
});
}
}
@@ -230,7 +238,9 @@ function runWithEnvironment(
});
log({kind: 'hir', name: 'AnalyseFunctions', value: hir});
inferMutationAliasingEffects(hir);
env.tryRecord(() => {
inferMutationAliasingEffects(hir);
});
log({kind: 'hir', name: 'InferMutationAliasingEffects', value: hir});
if (env.outputMode === 'ssr') {
@@ -250,25 +260,31 @@ function runWithEnvironment(
});
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 (
@@ -277,7 +293,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') {
@@ -289,10 +307,14 @@ function runWithEnvironment(
}
if (env.config.validateNoImpureFunctionsInRender) {
validateNoImpureFunctionsInRender(hir);
env.tryRecord(() => {
validateNoImpureFunctionsInRender(hir);
});
}
validateNoFreezingKnownMutableFunctions(hir);
env.tryRecord(() => {
validateNoFreezingKnownMutableFunctions(hir);
});
}
env.tryRecord(() => {
@@ -306,7 +328,9 @@ function runWithEnvironment(
env.config.validateExhaustiveEffectDependencies
) {
// NOTE: this relies on reactivity inference running first
validateExhaustiveDependencies(hir);
env.tryRecord(() => {
validateExhaustiveDependencies(hir);
});
}
}
@@ -594,7 +618,9 @@ function runWithEnvironment(
env.config.enablePreserveExistingMemoizationGuarantees ||
env.config.validatePreserveExistingMemoizationGuarantees
) {
validatePreservedManualMemoization(reactiveFunction);
env.tryRecord(() => {
validatePreservedManualMemoization(reactiveFunction);
});
}
const ast = codegenFunction(reactiveFunction, {
@@ -607,7 +633,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);
}

View File

@@ -18,7 +18,12 @@ function Component() {
// Error: reading ref during render
const value = ref.current;
return <div>{value}{items.length}</div>;
return (
<div>
{value}
{items.length}
</div>
);
}
```
@@ -50,8 +55,8 @@ error.var-declaration-and-ref-access.ts:15:16
> 15 | const value = ref.current;
| ^^^^^^^^^^^ Cannot access ref value during render
16 |
17 | return <div>{value}{items.length}</div>;
18 | }
17 | return (
18 | <div>
```