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 Ppx_deriving.register_external #126

Merged
merged 1 commit into from Mar 21, 2017
Merged

Add Ppx_deriving.register_external #126

merged 1 commit into from Mar 21, 2017

Conversation

ghost
Copy link

@ghost ghost commented Mar 20, 2017

This PR adds a new function allowing one to declare a deriver that is interpret by another ppx rewriter:

(** [register_external name] registers [name] as an external deriver that is
    not interpreted by ppx_deriving. Ppx_deriving will simply ignore [name]
    when interpreting [[\@\@deriving]] annotations and leave it to be
    expanded by another rewriter. *)
val register_external : string -> unit

The aim is to allow ppx_deriver and ppx_type_conv to live in harmony when linked as part of the same ocaml-migrate-parsetree driver once #123 is merged.

Current situation

Currently ppx_type_conv does the following in order to interoperate with ppx_deriving:

  • it exports its own derivers using the Ppx_deriving API
  • it imports ppx_deriving derivers using Ppx_deriving.{add_register_hook,derivers}

If ppx_deriving and ppx_type_conv were linked as part of the same omp driver, the result might not be what one would expect. In practice, since ppx_type_conv uses the 4.03 AST and ppx_deriving (with #123) uses the 4.05 one, ppx_type_conv would be executed first and because it removes [@@deriving ...] annotations, ppx_deriving would see nothing. This would be a bit surprising for users since as soon as they start using a ppx_type_conv based rewriter such as ppx_sexp_conv, they would have to change the option syntax of their [@@deriving ...] annotations. Moreover it is a bit fragile.

Goal

With this PR and #123, ppx_type_conv will register its own deriviers with Ppx_deriving.register_external and ignore ppx_deriving's derivers.

This is better than the current situation for two reasons:

  • ppx_type_conv and ppx_deriving don't work exactly the same way and with this change they'll both get to interpret their own derivers in the way they were supposed to be interpreted
  • the import/export code in ppx_type_conv is big and tedious and this change will allow us to eventually get rid of it

@whitequark whitequark merged commit cb978e8 into ocaml-ppx:master Mar 21, 2017
@whitequark
Copy link
Collaborator

Thanks!

@ghost ghost mentioned this pull request Apr 24, 2017
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.

None yet

1 participant