Add error handling plumbing - #86
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #86 +/- ##
==========================================
+ Coverage 92.42% 92.79% +0.37%
==========================================
Files 26 27 +1
Lines 4606 4888 +282
==========================================
+ Hits 4257 4536 +279
- Misses 349 352 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6459cc1ed5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| check_quickr_error_message_continuable(msg) | ||
| msg_literal <- fortran_string_literal(msg) | ||
| c(glue("call {quickr_error_setter_name()}({msg_literal})"), "return") |
There was a problem hiding this comment.
The helper always emits a bare return after setting the error message. If a user calls stop() (or triggers the Lapack error helpers that reuse this) inside a parallel() loop, the generated code places that return inside a !$omp parallel do region, which OpenMP forbids as a branch out of a parallel region and typically fails to compile. Consider suppressing the return when OpenMP is active and rely on !$omp cancel parallel plus the post-loop error check, or emit a loop-local cycle instead.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14eba24768
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| error_check_inner <- if (is.null(parallel)) { | ||
| quickr_error_return_if_set(scope) | ||
| } else { | ||
| "" |
There was a problem hiding this comment.
When parallel is set, error_check_inner is deliberately empty, so the OpenMP sapply loop never checks quickr_err_msg inside the do region. Because compile_sapply_assignment() never calls enter_openmp_scope(), a stop() in the closure only calls quickr_set_error_msg and returns, and the parallel loop continues to run until completion; the error is only surfaced after the loop. This means OpenMP sapply() can keep executing with known-invalid inputs or side effects long after an error is raised, unlike parallel for loops where !$omp cancel do is triggered promptly. Consider inserting a cancellation check inside the loop for the parallel case or marking the OpenMP scope so stop() emits !$omp cancel do.
Useful? React with 👍 / 👎.
- Keep post-loop error checks inside enclosing OpenMP region
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
quickr_err_msg+quickr_set_error_msg) and raiseRf_error()when setstop()to the Fortran error setter and propagate early returns across closure calls andsapply()loopssolve(),qr.solve(),chol(),chol2inv()via quickr errorsMotivation
Generated code had no structured way to signal errors back to R, and LAPACK failures could be silent. This adds a minimal error channel so quickr code can raise meaningful errors and callers see them immediately, including inside closures and OpenMP loops.
New API examples