ocaml-ppx / ppx_tools

Tools for authors of ppx rewriters
MIT License
134 stars 39 forks source link

Plan for 4.03 #31

Closed Drup closed 8 years ago

Drup commented 8 years ago

The main point of breakage for ppxs in 4.03 is going to be the new type for labeled arguments. I think we could ease a bit the transition with some additions to ppx_tools, in both the 4.02.3 versions and the 4.03 version:

type label = 
  string (* in 4.02 *)
  Asttypes.arg_label (* in 4.03 *)

type label_desc = (* = Asttypes.arg_label for 4.03 only *)
    Nolabel
  | Labelled of string 
  | Optional of string

val explode : label -> label_desc

val nolabel : label
val labelled : string -> label
val optional : string -> label

By using only these functions, it should be possible to do almost all manipulations on functions with 4.02 and 4.03 compatibility.

@alainfrisch @diml opinions ?

alainfrisch commented 8 years ago

This sounds like a good idea (perhaps putting all those in a sub-module with a short name, or use something more explicit than explode at least).

Drup commented 8 years ago

@alainfrisch Yes, I was planning on putting that in a submodule. could you create a 4.02 branch based on the last release ?

alainfrisch commented 8 years ago

I've granted you push rights on the repo. Feel free to create the branch and push your suggestion to it and master.

Drup commented 8 years ago

@diml Do you already have a similar plan for ppx_core ?

ghost commented 8 years ago

I didn't make plan for 4.02/4.03 compatilibility of ppx_core. I was thinking of leaving the compatibility question for after 4.03, as we have a lot of breakage in our ppx rewriters with 4.03. But if you add this to ppx_tools I can use it now in ppx_core.

BTW the current version of ppx_tools in opam for 4.02 is based on this branch: https://github.com/diml/ppx_tools/tree/4.02. Make sure to import the commits if you plan to do more changes for 4.02

Drup commented 8 years ago

Ah, you backported the various changes. Great.

Drup commented 8 years ago

Ok, all done now. The final diff between 4.02 and master is here and it's rather small. Should I use the Label module inside genlifter too ?

The Label module is enough to make lwt's ppx compatible with both version easily. I will see if I need other modules for other ppxs.

whitequark commented 8 years ago

@Drup Time to make a release with this? There should be no more AST changes for 4.03 for sure, and ppx_tools blocks all downstream software, e.g. https://github.com/whitequark/ppx_deriving/issues/80.

whitequark commented 8 years ago

Also I'm not sure if the Label module really should be in Ast_convenience. It's more of a necessity...

Drup commented 8 years ago

@whitequark Label is in Ast_convenience because that's (almost) the only module in ppx_tools. ;)

I agree we should do a release. I have very little time right now. @alainfrisch ?

alainfrisch commented 8 years ago

I agree we should do a release. I have very little time right now. @alainfrisch ?

Same for me. I'm leaving tonight for a few days of vacations...

alainfrisch commented 8 years ago

ppx_tools 4.03.0 is now released. I've switched to a versioning scheme that follows OCaml's.

Drup commented 8 years ago

@alainfrisch You need to release the version for 4.02.3 too.

alainfrisch commented 8 years ago

Ok, done as well. Both submitted to OPAM: https://github.com/ocaml/opam-repository/pull/5942

Drup commented 8 years ago

Thanks! I think we can close now.