Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flag -raise-embedded-errors to Driver #559

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

jaymody
Copy link
Contributor

@jaymody jaymody commented Feb 5, 2025

Adds a flag -raise-embedded-errors to the shared args in Driver, which will raise the first ocaml.error node found in the processed AST.

Motivation

It's useful to be able to run a standalone ppx driver for it's side affects (e.g. registering correction files), without actually making use of it's output directly.

For example, given the program program.ml:

type t = int [@@deriving_inline my_deriver]

One might run:

$ my_ppx.exe program.ml -null

With the objective of registering a correction file while ignoring any output. However, since errors are embedded into the AST, the error [%%ocaml.error "ppxlib: [@@@deriving.end] attribute missing"] never gets surfaced to the users. Now it can be, with -raise-embedded-errors.

@jaymody jaymody force-pushed the raise-embedded-errors branch 2 times, most recently from 5e4a42e to 6fcb29a Compare February 5, 2025 21:31
Copy link
Collaborator

@NathanReb NathanReb left a comment

Choose a reason for hiding this comment

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

It's a bit annoying to have to add yet another flag for this but I guess for now there's no other way around it.

I'd like to have a unique way to produce errors via ppxlib that would let the driver decide whether it embeds them or raise them straight away but that would require the whole ecosystem to update so it won't happen overnight.

@NathanReb
Copy link
Collaborator

Could you rebase and add a changelog entry?

The changelog check is not picking it up because the PR is based on a version where I had not re-introduced the unreleased changelog section.

@jaymody jaymody force-pushed the raise-embedded-errors branch from 1124e82 to 6fcb29a Compare February 12, 2025 16:53
…edded ocaml.error in the processed AST.

Signed-off-by: Jay Mody <[email protected]>
@jaymody jaymody force-pushed the raise-embedded-errors branch from 6fcb29a to dfa1f63 Compare February 12, 2025 16:54
@jaymody jaymody force-pushed the raise-embedded-errors branch from fc60ad7 to fa89724 Compare February 12, 2025 17:00
Copy link
Collaborator

@NathanReb NathanReb left a comment

Choose a reason for hiding this comment

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

Perfect, thanks!

@NathanReb NathanReb merged commit aa0afff into ocaml-ppx:main Feb 13, 2025
5 of 6 checks passed
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