ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.57k stars 396 forks source link

Documentation is not clear about (kind ppx_*) #1327

Open pkel opened 5 years ago

pkel commented 5 years ago

The current documentation reads:

(kind <kind>) is the kind of the library. The default is normal, other available choices are ppx_rewriter and ppx_deriver and must be set when the library is intended to be used as a ppx rewriter or a [@@deriving ...] plugin. The reason why ppx_rewriter and ppx_deriver are split is historical and hopefully we won’t need two options soon

I'm using Ppxlib.Deriving to build a [@@deriving my] plugin. I have to set ppx_rewriter, otherwise utop complains about missing ppx_deriving on #require "ppx_my". Should the description better read like the following or am I observing a different problem?

(kind <kind>) is the kind of the library. The default is normal, other available choices are ppx_rewriter and ppx_deriver and must be set when the library is intended to be used as a ppx. Use ppx_deriver for rewriters build on the ppx_deriving package and ppx_rewriter if you use ppxlib. The reason why ppx_rewriter and ppx_deriver are split is historical and hopefully we won’t need two options soon

emillon commented 5 years ago

Hi, I agree that this is not very clear. The wording you suggest is a step in the correct direction. I think that we can be a bit less specific, e.g. "for ppx preprocessors related to [@@deriving]" and "for other types of ppx preprecessors" (the reason is that you can use derivers without ppx_deriving, e.g. ppx_sexp_conv, and rewriters without ppxlib, e.g. using ocaml-migrate-parsetree - I don't think it's worth going into that much detail). Also, the part about historical reasons can probably dropped.

Can you open a PR? You'll just need to git commit --signoff if you're OK with our contributing guidelines. Thanks! :pray:

pkel commented 5 years ago

a bit less specific, e.g. "for ppx preprocessors related to [@@deriving]"

Using [@@deriving] to describe things related to the ppx_deriving package caused all the confusion on my side. At other places it seems that [@@deriving ...] (with or without dots) is used to describe derivers no matter how they are built.

ghost commented 5 years ago

BTW, the underlying story is the following: [@@deriving ..] plugins, using either ppx_deriving or ppxlib use a common global registry. So when several [@@deriving ..] plugins are used together, they must be grouped into a single process. When using Dune, all ppx rewriters are always grouped into a single one, so the distinction doesn't matter. However, they are cases where ppx rewriters are not grouped together by default:

In these two cases, the ppx rewriters are split into several processes and applied one by one except for the [@@deriving ..] plugins, that are dynlinked into a single process to ensure they can all access the global registry. ppx_deriving provides the executable that dynlink other [@@deriving ...] plugins. That's why when you use (kind ppx_deriving), Dune will arrange things so that in the toplevel or with other build systems your ppx automatically depend on ppx_deriving.

pkel commented 5 years ago

This implies, that [@@deriving ..] plugins built with ppxlib should set (kind ppx_deriver) and depend on ppx_deriving, correct? So the documentation of Dune is okay, but I was confused by the one of ppxlib.

ghost commented 5 years ago

Yes. it's a bit unfortunate to add an extra dependency just for the toplevel. It should be possible to adapt utop so that this isn't necessary anymore

pitag-ha commented 2 years ago

To follow up on this: Is the situation described here still as it was back then in 2018? Or has some part changed in this context in the meanwhile? From what I've been told, by now ppx_deriving registers its derivers via ppxlib. So at first sight, it seems like they might all be grouped into a single process by ppxlib anyways, but I might very easily be missing something here.