-
Notifications
You must be signed in to change notification settings - Fork 18
Open
Description
Description
The incremental compiler (salsa-based compile_project_incremental) panics when processing certain models, most notably C-LEARN. Multiple call sites work around this by wrapping calls in std::panic::catch_unwind:
benches/compiler.rs:71-80-- theis_simulatablefunction explicitly documents that "some models (e.g. C-LEARN) panic in the incremental compiler rather than returning a clean error"tests/simulate.rs:1296-- the cross-validation test loop wraps incremental compilation incatch_unwindto allow the test to continue past panicking modelssrc/analysis.rs:122-124-- LTM analysis wraps the pipeline for "graceful degradation"src/layout/mod.rs:1874, 2041, 2061, 2129-- multiple layout functions wrap incremental/monolithic compilation
The monolithic compiler handled these same models without panicking, so the issue is specific to the incremental compilation path.
Why it matters
- Correctness: Panics indicate unhandled states that should be proper errors
- WASM safety:
catch_unwindis ineffective underpanic = "abort"(used in WASM release builds), so these models would crash the entire WASM runtime rather than failing gracefully - Maintainability:
catch_unwindis not a substitute for proper error handling and makes it harder to reason about failure modes
Components affected
src/simlin-engine/src/db.rs(incremental compilation entry points)src/simlin-engine/benches/compiler.rssrc/simlin-engine/tests/simulate.rssrc/simlin-engine/src/analysis.rssrc/simlin-engine/src/layout/mod.rs
Possible approaches
- Run the incremental compiler against C-LEARN (and any other panicking models) under a debug build to capture the panic backtrace and identify the root cause
- Convert the panic site(s) to return
Result::Errwith proper diagnostics - Once fixed, remove the
catch_unwindwrappers in benchmarks and tests (layout/analysiscatch_unwindmay still be justified for defense-in-depth against malformed user models)
Context
Identified during review of the salsa consolidation branch (salsa-consolidation), where the benchmark and test code were updated to use the incremental path.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels