Make this part of a bundle of standard derivers on ocaml-ppx

[pitag-ha]

Hi @bn-d, I've just come across this project and it looks really nice! Do you know about our efforts of writing a bundle of Ppxlib.Deriving standard derivers similar to the old ppx_deriving.std? With "us" I mean the ppxlib team (I'm one of the maintainers). And the person who's doing the work of implementing the derivers is @ayc9 who is doing an Outreachy internship with the OCaml community.

Unfortunately, we didn't know about your ppx_make project some weeks ago when @ayc9 started the internship and started implementing something quite similar to this. So, since this is on me: apologies to both of you!! Now that I know about this project, I have a couple of questions for you:

Would you mind listing the difference(s) with ppx_deriving.make from a user's perspective? E.g. it seems like ppx_make also supports tuples. Are there also some breaking differences?

And what do you think about the idea of joining efforts? @ayc9's work so far has been really good so we for sure want to use it. But still, parts of the implementation of ppx_make would probably still be helpful for us. Of course, if we join efforts, we'll give you credits on our README and let you review @ayc9's version (if you want to) etc.

[bn-d]

Hi @pitag-ha , that sounds like a very interesting project! Where can I read more about this Ppxlib.Deriving standard derivers project?

I had listed the differences here previously.

• add support for option: val make_t : ?value:t -> unit -> t
• add support for tuple: val make_t : ?v0 -> ?v1 -> ... -> unit -> t
• add support for variant, including embedded record: make_choice_name_of_t : ... -> unit -> t
• add support for multiple [@main] in record
• unit is required for all make function unless [@main] presents. This is to ensure forward-compatibility
  • i.e. the use of the make of { a:int; b:int } will work with the make of { a:int; b:int; c:int option }
• ditch the [@split]

This is mostly unchanged. I think the only addition is support for polymorphic types (ie result).

The last two point are breaking differences. The unit change is to make the generated codes less ambiguous and more friendly in case of the modification of original types. For split, I simply do not understand its use case and purpose, therefore I did not add it. But I believe those two behaviors could be easily altered.

I'd love to help and answer any question regarding ppx_make. But could you please elaborate more on what joining effort means? Thanks.

[bn-d]

And if there is a project for standard rewriter, please check out https://github.com/bn-d/ppx_pyformat . It is a rewriter that is inspired by format function in Python. I just finished it recently and I am working on releasing to opam.

[pitag-ha]

Thanks a lot for your quick and nice answer @bn-d!

Where can I read more about this Ppxlib.Deriving standard derivers project?

You can have a look at the README of the repo where @ayc9 will add the derivers. I also announced the internship together with the other two internships this round on discuss, but only with one brief sentence.

Thanks also for listing the differences!

The unit change is to make the generated codes less ambiguous and more friendly in case of the modification of original types.

Yes, I agree that that's more coherent. However, on a big record without optional fields it might be inconvenient to have the unit at last parameter. To be honest, I don't have a technical opinion on that decision myself, but we're trying to break as little as possible with the ppx_deriving ones for portability, so we'll probably go for how it's done there.

For split, I simply do not understand its use case and purpose

I could think of two possible use cases: partial function application stopping somewhere in the middle of a split field; and when splitting a field field e.g. (int * int list) having the second entry optional. However, I agree that those are really not common use cases, so we'll have to think about if it's worth implementing.

I'd love to help and answer any question regarding ppx_make.

That's awesome! Thank you :)

But could you please elaborate more on what joining effort means?

What I meant was: @ayc9 partially taking code of yours (which I think with your license should be ok anyway, but still I wanted to ask first); us discussing the project with you to inter-change ideas and understanding (as we've started doing already); possibly you also having a look at @ayc9's deriver (but only if you want to take the time); and at the end think together if we want this deriver to keep on co-existing or not anymore. To be honest, it seems like we'll end up supporting fewer features, in which case it probably should keep on co-existing. So in that case maybe we can mutually add links with explanations to each other on each other's README. How does that all sound to you?

[bn-d]

Please consider the following scenario. When you provide types to clients through library, you can tag the types as private in .mli and force the clients to only use the make function to construct them. In the case of

type blob = private {
  encoding : encoding;
  data : string;
}[@@deriving make]

The client will do

(* in ppx_deriving.make *)
let my_blob = make_blob ~encoding ~data

(* in ppx_make *)
let my_blob = make_blob ~encoding ~data ()

When you try to enhance the type after release, ie adding a new optional field

type blob = private {
  encoding : encoding;
  data : string;
  encoding_config : encoding_config option;
}[@@deriving make]

The ppx_deriving.make code will break but the ppx_make code will continue to work.

The philosophy is that as long as the added field is optional (option /list types or provided with default value), the previous code should not break. This can make continuous integration a little bit easier when sharing types across libs in OCaml.

However, I agree that those are really not common use cases, so we'll have to think about if it's worth implementing.

Just something came to my mind.

type my_type = {
  arg : int;
  args : (int * int list); [@split]
}

This is a corner case where split does not work with a type. Some special validation is required here, else the code will give some very confusing error messages at compile time or work incorrectly.

but still I wanted to ask first

That is very kind.

possibly you also having a look at @ayc9's deriver (but only if you want to take the time)

Would do if you see any benefit to it.

To be honest, it seems like we'll end up supporting fewer features, in which case it probably should keep on co-existing.

I agree. If the project's goal is to "break as little as possible with the ppx_deriving ones for portability", the two libraries will not be compatible with each other in some cases and co-existing is a good solution. But the examples I give could be pretty rare. I am also wondering how many code would break in opam if we just replace ppx_deriving.make with ppx_make.

[pitag-ha]

Thanks for your input! And sorry for the delay in answering.

Re ppx_make vs ppx_deriving.make unit convention in the context of private types:

The ppx_deriving.make code will break but the ppx_make code will continue to work.

Note that even if you declare your record as private, it can still be de-structured. So also with ppx_make, adding a new field to your private record would be a breaking change. To avoid that, the type would need to be abstract, but on abstract types ppx_deriving.make doesn't work and I'd assume that's the same for ppx_make, isn't it?

Re split:

This is a corner case where split does not work with a type. Some special validation is required here, else the code will give some very confusing error messages at compile time or work incorrectly.

When using ppx_deriving.make on that, there's no compile error. The derived value has type

val make_my_type : arg:'a -> arg:int -> ?args:int list -> unit -> my_type

Could you give more detail on what validation is required?

[bn-d]

Note that even if you declare your record as private, it can still be de-structured. So also with ppx_make, adding a new field to your private record would be a breaking change. To avoid that, the type would need to be abstract, but on abstract types ppx_deriving.make doesn't work and I'd assume that's the same for ppx_make, isn't it?

You're absolutely correct. That why I said a little bit easier. My solution is just add {...;_ } everywhere and just suppress the warning. (Also there is the problem of match etc...

When using ppx_deriving.make on that, there's no compile error. The derived value has type

Interesting. I don't know you can have two labeled args with the same name. But without some special handling, the second arg will shadow the first. I see no such thing in the original ppx_deriving code.

https://github.com/ocaml-ppx/ppx_deriving/blob/6198c82aa38b023b4402f4085cbfda195571d8ed/src_plugins/make/ppx_deriving_make.cppo.ml#L74 https://github.com/ocaml-ppx/ppx_deriving/blob/6198c82aa38b023b4402f4085cbfda195571d8ed/src_plugins/make/ppx_deriving_make.cppo.ml#L83

In my test case,

type my_type = {
  arg : int;
  args : (int * int list) [@split];
} [@@deriving make]

let _ =
  let a = make_my_type ~arg:1 ~arg:2 () in
  let args_fst, _ = a.args in
  Printf.printf "%d %d" a.arg args_fst

It suppose to print 1 2 but print 2 2 instead. (I might test this OCaml behavior a little bit more and submit an issue to ask for an additional warning for this case

If I changed the type of arg to string, then I will get compiler error

File "deriving_test.ml", lines 1-4, characters 0-19:
1 | type my_type = {
2 |   arg : string;
3 |   args : (int * int list) [@split];
4 | } [@@deriving make]
Error: This expression has type string * 'a list
       but an expression was expected of type int * int list
       Type string is not compatible with type int

[pitag-ha]

Ooh, good point! I hadn't even noticed that the two parameters have the same name. So yes, you've convinced me that the split feature isn't a hygienic approach. Thanks!