optional parameter for ppx_bin_prot

[pbiggar]

ppx_deriving supports an optional flag. From their docs:

It's possible to make deriving ignore a missing plugin rather than raising an error by passing an optional = true option, for example, to enable conditional compilation:

type addr = string * int
[@@deriving yojson { optional = true }]

This works well for their builtins, but doesn't seem to work for bin_io - which is making writing platform independent code difficult. The bin_io folks suggested that this would need to be implemented here.

Is this the place to support optional for all ppx derivers?

[ghost]

What is the use case for this? I mean, how can a ppx rewriter be present or not?

[pbiggar]

What is the use case for this? I mean, how can a ppx rewriter be present or not?

My main use case is sharing code between our two apps, one in bucklescript/ReasonML, and the other using native OCaml/dune. Though we use several PPXes in bucklescript, however some we only use in native (ppx_deriving_yojson which already supports optional which made this easy to handle; and ppx_bin_prot)

[ghost]

I see. That seems like a valid use case to me, thanks for explaining. That is indeed the right place to support optional for all ppx derivers. Do you want to give it a go?

[pbiggar]

Do you want to give it a go?

I can try. Can you give me a rough outline of what you'd want a solution or implementation to look like?

[ghost]

Sure, so I don't have the code in mind right now and little time to dig in on this subject, so I will only give you high level pointers and will let you do a bit of research for now :)

Essentially, at some point we resolve the string "ppx_bin_prot" into a Deriver.t, and we fail if we cannot find it. That were you want to patch the code: if we can't find it, you should like if there is optional:true in the list of arguments. To find out where this code is, you can start by grepping for the error message and walk your way up to the lookup function.

[sim642]

This is related to https://github.com/ocaml-ppx/ppx_deriving/issues/247, which really is just this ppxlib issue. if ppxlib's deriving transformation handled the special optional argument as well, then both issues would be solved.

It would be entirely possible to do this in ppxlib, I just wonder whether this is the most appropriate way to support optional derivers. It kind of hijacks one argument from the derivers' declared argument space. Maybe something separate like [@@deriving_optional ...] would be nicer? Of course that doesn't provide the seamless compatibility between ppx_deriving and ppxlib.

Or given the extremely rare (desired) use of optional derivers, maybe it's fine to not even provide anything for it out of the box, but just require it to be handled on the user's side. I think it would be fairly simple to define a custom in-project deriver that's conditionally just Ppxlib.Deriving.add_alias for the desired one (or not). I guess that would require #124 though.