Skip to content

Conversation

@johannes-wolf
Copy link
Collaborator

@johannes-wolf johannes-wolf commented Sep 14, 2025

It is still easy to accidentally "crash" the WASM runtime with queries – so let's replace runtime exceptions with tl::expected.

Missing:

  • Division by zero still throws
  • The model still throws (4 exceptions left)
  • Function arg-parsing (at runtime) still throws

On main:

benchmark name                       samples       iterations    est run time
                                     mean          low mean      high mean
                                     std dev       low std dev   high std dev
-------------------------------------------------------------------------------
Query typeof Recursive                         100             1     1.73898 s 
                                        17.6426 ms    17.5999 ms     17.695 ms 
                                        241.428 us    204.215 us    290.245 us 
                                                                               
Query field id Recursive                       100             1     1.36372 s 
                                        13.9724 ms    13.8896 ms    14.0689 ms 
                                        456.841 us    402.926 us    551.696 us 
                                                                               
Query field id                                 100             1    28.5413 ms 
                                        286.066 us    284.083 us     289.23 us 
                                        12.5867 us     8.7692 us    17.7284 us

This branch:

benchmark name                       samples       iterations    est run time  
                                     mean          low mean      high mean     
                                     std dev       low std dev   high std dev  
-------------------------------------------------------------------------------
Query typeof Recursive                         100             1     1.56551 s 
                                        15.1817 ms    15.1624 ms    15.2042 ms 
                                        106.495 us    89.1097 us    127.841 us 

Query field id Recursive                       100             1    940.211 ms 
                                         9.4754 ms     9.4566 ms    9.49996 ms 
                                        109.514 us    87.3406 us    139.911 us 

Query field id                                 100             1    23.6667 ms 
                                        233.751 us    231.936 us    240.035 us 
                                        15.4148 us    4.62786 us    34.9265 us 

Query field keys recursive                     100             1     1.26274 s 
                                        12.5335 ms    12.5022 ms    12.5729 ms 
                                        178.346 us    147.985 us    225.614 us

Note

Switches the engine from throwing exceptions to returning errors via tl::expected across expressions, functions, model/value, I/O, and tooling, updating APIs and tests accordingly.

  • Error handling (core):
    • Replace runtime exceptions with tl::expected throughout (Expr::eval/ieval, Function::eval, ResultFn, MetaType, operators dispatch, etc.).
    • Expand simfil::Error::Type and add equality for Error/SourceLocation.
  • Expressions/Operators:
    • All eval paths now return tl::expected<Result, Error>; propagate errors and remove exception-based control flow (e.g., division by zero -> Error::DivisionByZero).
    • Add move-optimized eval overloads; debug paths adjusted.
  • Functions:
    • Function interface returns expected; argument parsing reworked to validate/count/type-check via ArgParser without throwing.
  • Value/Model:
    • Refactor Value to carry ModelNode::Ptr in its variant; add helpers (nodePtr, node, asBool).
    • Model/ModelPool APIs (resolve, root, validate, read/write, setStrings) now return expected; error messages standardized.
    • StringPool::emplace/read/write return expected; handle pool overflow as Error.
  • JSON/Serialization:
    • json::parse and internal build use expected; Bitsery traits handle arena access errors.
  • Arena:
    • ArrayArena::at now returns expected<reference_wrapper<...>, Error>; iterator deref asserts; out-of-range returns Error::IndexOutOfRange.
  • Overlay/Completion/Parser:
    • Adjust to new eval and model APIs; non-cloneable completion exprs unchanged but error-aware.
  • Examples/REPL:
    • Update to unwrap expected (root/eval/parse) and print errors instead of throwing.
  • Tests:
    • Update expectations to check expected results and new error cases; add performance/diagnostics tweaks.

Written by Cursor Bugbot for commit f1a26a5. This will update automatically on new commits. Configure here.

@github-actions
Copy link

github-actions bot commented Sep 14, 2025

Test Results

 1 files  ± 0   1 suites  ±0   6m 53s ⏱️ + 4m 2s
86 tests +31  86 ✅ +31  0 💤 ±0  0 ❌ ±0 
91 runs  +31  91 ✅ +31  0 💤 ±0  0 ❌ ±0 

Results for commit 90615c1. ± Comparison against base commit 1a2b998.

This pull request removes 2 and adds 33 tests. Note that renamed tests count towards both.
CompleteFieldOrString ‑ CompleteFieldOrString
CompleteSorted ‑ CompleteSorted
Binary arithmetic operators ‑ Binary arithmetic operators
Binary comparison operators ‑ Binary comparison operators
Complete And/Or ‑ Complete And/Or
Complete Field with Special Characters ‑ Complete Field with Special Characters
Complete Function ‑ Complete Function
Complete SmartCas ‑ Complete SmartCas
Complete Wildcard Comparison ‑ Complete Wildcard Comparison
Complete Wildcard Hint ‑ Complete Wildcard Hint
Complete in Expression ‑ Complete in Expression
Complete in unclosed expression ‑ Complete in unclosed expression
…

♻️ This comment has been updated with latest results.

@johannes-wolf johannes-wolf force-pushed the make-simfil-even-more-exception-free branch from 22da9a0 to e0c84f6 Compare September 14, 2025 20:44
@johannes-wolf johannes-wolf force-pushed the make-simfil-even-more-exception-free branch from 7fe9fba to aa0eb8a Compare September 22, 2025 23:33
@johannes-wolf
Copy link
Collaborator Author

johannes-wolf commented Sep 22, 2025

Outside of constructors there are 4 calls to raise<...> left in the codebase with this PR.
(Plus some unreachable throws for unimplemented virtual functions that must never get called.)

@johannes-wolf johannes-wolf marked this pull request as ready for review September 22, 2025 23:36
@johannes-wolf johannes-wolf force-pushed the make-simfil-even-more-exception-free branch 2 times, most recently from 69a7acc to 223d3cd Compare September 22, 2025 23:39
cursor[bot]

This comment was marked as outdated.

@johannes-wolf johannes-wolf changed the title Removing Runtime Exceptions Remove Runtime Exceptions Sep 23, 2025
@johannes-wolf johannes-wolf force-pushed the make-simfil-even-more-exception-free branch from 223d3cd to fded898 Compare September 23, 2025 07:14
cursor[bot]

This comment was marked as outdated.

@johannes-wolf johannes-wolf force-pushed the make-simfil-even-more-exception-free branch 2 times, most recently from 2fe6c66 to 812f9a7 Compare September 23, 2025 07:25
cursor[bot]

This comment was marked as outdated.

@johannes-wolf johannes-wolf force-pushed the make-simfil-even-more-exception-free branch from 0a2d79c to 5a110e0 Compare September 23, 2025 16:27
cursor[bot]

This comment was marked as outdated.

@josephbirkner
Copy link
Collaborator

Looks good! We could probably improve performance even more by moving from result callbacks to coroutines. Should I create a ticket for this?

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@johannes-wolf johannes-wolf force-pushed the make-simfil-even-more-exception-free branch from c81bb65 to 368badb Compare September 25, 2025 18:15
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@cursor
Copy link

cursor bot commented Oct 23, 2025

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@johannes-wolf johannes-wolf force-pushed the make-simfil-even-more-exception-free branch from a09ad91 to a6bba7b Compare October 26, 2025 21:22
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@johannes-wolf johannes-wolf force-pushed the make-simfil-even-more-exception-free branch from 5813c2c to e39cb9d Compare October 26, 2025 23:44
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@johannes-wolf johannes-wolf force-pushed the make-simfil-even-more-exception-free branch from 96dc420 to 1d2ddae Compare October 27, 2025 08:08
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@josephbirkner
Copy link
Collaborator

Looks like a release 🥳

Might we think about doing this?

if (!result) {      // GCOVR_EXCL_START
  return Err{ENull};
}                   // GCOVR_EXCL_STOP

@johannes-wolf
Copy link
Collaborator Author

johannes-wolf commented Oct 29, 2025

I've added the gcovr option --exclude-lines-by-pattern TRY_EXPECTED and replaced most of the if (!res) ... blocks with the macro. EDIT: Does not seem to work 🙃.

See: https://gcovr.com/en/stable/manpage.html#cmdoption-gcovr-exclude-lines-by-pattern

@johannes-wolf johannes-wolf force-pushed the make-simfil-even-more-exception-free branch from 822cd59 to f597714 Compare October 29, 2025 17:06
@sonarqubecloud
Copy link

@github-actions
Copy link

Package Line Rate Branch Rate Health
include.simfil 27% 11%
include.simfil.model 91% 57%
src 74% 46%
src.model 80% 45%
Summary 45% (5783 / 12752) 27% (3880 / 14313)

Copy link
Collaborator

@josephbirkner josephbirkner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍰

@josephbirkner josephbirkner merged commit 6591b8e into main Oct 31, 2025
9 checks passed
@josephbirkner josephbirkner deleted the make-simfil-even-more-exception-free branch October 31, 2025 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants