ppxlib icon indicating copy to clipboard operation
ppxlib copied to clipboard

Ppx_deriving utility functions

Open sim642 opened this issue 3 years ago • 2 comments

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:

  • [x] Quoting. (#95)
  • [x] Mangling (#370).
  • [ ] Polymorphism handling.

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: https://github.com/ocaml-ppx/ppx_deriving/pull/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 (https://github.com/ocaml-ppx/ppx_deriving/pull/252).

sim642 avatar Jan 18 '22 10:01 sim642

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.

pitag-ha avatar Sep 08 '22 17:09 pitag-ha

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.

sim642 avatar Sep 08 '22 18:09 sim642