Skip to content

fix(Reporter): avoid redundant effect handlers - #203

Merged
favonia merged 3 commits into
mainfrom
fix/202-avoid-noop-handlers
Oct 19, 2026
Merged

favonia merged 3 commits into
mainfrom
fix/202-avoid-noop-handlers

Conversation

@favonia

@favonia favonia commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

When ~emit is omitted, Reporter.try_with and Structured_reporter.try_with now catch fatal exceptions without installing an effect handler. Explicit emit handlers retain their existing behavior. The implementation also adopts OCaml 5.3's effect-handler syntax, raising the minimum supported OCaml version from 5.2 to 5.3.

Related: #202 (Option 3).

Skip the effect handler when try_with is called without emit. Use OCaml 5.3 effect-handler syntax and raise the minimum compiler version accordingly.

Add regression coverage for both reporters and test OCaml 5.3.0 and 5.4.1 in CI.

Fixes #202
@favonia favonia changed the title fix(Reporter)!: avoid redundant effect handlers fix(Reporter): avoid redundant effect handlers Oct 19, 2026
@favonia
favonia requested review from mikeshulman and removed request for mikeshulman October 19, 2026 05:56
@favonia
favonia marked this pull request as draft October 19, 2026 05:57

@mikeshulman mikeshulman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good!

Describe explicit emit_diagnostic forwarding as semantically equivalent but less efficient. Remove the changelog additions from this change.
@favonia
favonia marked this pull request as ready for review October 19, 2026 06:41
@favonia

favonia commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

@mikeshulman I made some minor documentation tweaks after your approval.

@favonia
favonia merged commit 46fdafc into main Oct 19, 2026
2 checks passed
@favonia
favonia deleted the fix/202-avoid-noop-handlers branch October 19, 2026 06:44
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.

2 participants