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

Ppx_deriving utility functions #317

Open
2 of 3 tasks
sim642 opened this issue Jan 18, 2022 · 2 comments
Open
2 of 3 tasks

Ppx_deriving utility functions #317

sim642 opened this issue Jan 18, 2022 · 2 comments

Comments

@sim642
Copy link
Contributor

sim642 commented Jan 18, 2022

Even though ppxlib is recommended for implementing new ppx derivers, ppxlib itself doesn't have a bunch of utility functions that are provided by ppx-deriving.api: https://github.com/ocaml-ppx/ppx_deriving/blob/master/src/api/ppx_deriving.cppo.mli.
In particular:

I would say that most nontrivial ppx derivers need that functionality anyway. Currently there's two options:

  1. Just depend on ppx-deriving.api in addition to ppxlib. This is done by, e.g., ppx_deriving_yojson. This is not ideal, since the API package doesn't just contain the utilities, but also ppx-deriving's own registration system, which is replaced by ppxlib. Moreover, it prevents deprecation of the API in favor of ppxlib: [WIP] Deprecate the ppx_deriving API in favour of ppxlib ppx_deriving#250.
  2. Copy the utility functions' implementations to own ppx deriver project. This is done by, e.g., ppx_show. This avoids the dependency but involves copying code, which isn't ideal either. Moreover, fixing or optimizing those utilities is much easier in a central place (Optimize forwarding in eq and ord plugins ppx_deriving#252).
@pitag-ha
Copy link
Member

pitag-ha commented Sep 8, 2022

Thanks @sim642 for opening and working on this!

Are you also working or would like to work on the missing one, i.e. polymorphism handling? I was just ask by someone what could be a good issue to work on for someone new working on ppxlib and this could be a fit - unless you wanted to implement it yourself of course.

@sim642
Copy link
Contributor Author

sim642 commented Sep 8, 2022

Feel free to let them do it! I won't rush to it then.

Once we fix the submodule name for all these utilities in #370, then it should be very straightforward to bring any remaining ones over from ppx_deriving. If I recall correctly, Ppx_deriving contains a bunch of utility functions that don't belong to any of the categories I initially listed, but might be worth bringing over nevertheless.

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

No branches or pull requests

2 participants