Skip to content

Dev Meeting 2024 04 16

Jump to bottom
Nathan Rebours edited this page Apr 17, 2024 · 3 revisions

Agenda

  • How to deal with the release of 5.2 support
    • Can we safely release it ahead of the compiler?
    • Should we already merge trunk-support into main?
  • Updating trunk-support for 5.3
    • The trunk-support branch was lagging behind before our recent update following the 5.2.0~alpha releases of the compiler. Now is a good time to document and improve how we maintain this branch.
    • We lack documentation on how to add support for a new compiler version, that's something we might want to look into when adding support for the latest trunk.
    • It seems that @hhugo already has a branch with 5.3 support, probably worth looking into this.
  • Are we bumping the internal AST to 5.2
  • A quick update on ppx_deriving
    • 6.0.0 release ongoing with standard derivers ported to ppxlib
    • ppx_deriving_yojson should be migrated as well
    • We should start deprecating the ppx_deriving API for writing derivers as a next step
  • Refactoring the driver code that builds the list of AST transforms
    • Working on the dune driver mode brought to light that this part of the driver could use some simplification and clarification
  • Repo hygiene

Attendees

Notes

Release of 5.2 support and add 5.3 (trunk) support to ppxlib

We released 5.1 somewhere between alpha and beta release of the compiler last time. We were unlucky and the AST changed after that but this is very unlikely to happen again, especially considering we are in the beta phase of the compiler release already. We'll proceed to merge the trunk-support branch into main and cut a new release with the 5.2 support so we are ready ahead of the compiler final release. Worst case scenario: we will just have to update the upper bounds on opam-repo and cut another release.

For 5.3 we are considering dropping the trunk-support branch and instead add the 5.3 support and maintain it directly on the main branch. There is no arm in having partial support there. We'll drop the upper bound on ocaml (or move it to two version ahead of the last instead of one) on the main branch. We just have to be careful to adjust the bound before cutting a release between now and the 5.3 beta releases.

@hhugo mentioned they have a branch with 5.3 support so we will reach out to see how much of this we can reuse.

Even if we end up merging Hugo's work, we should take the time to gather all the documentation and tools that we use to maintain the versioned AST and migrations and organize them properly in ppxlib's repo as some of it at least will be part of the upstreaming work to the compiler. Sonja mentioned that some of the tools might still live in the old OMP repo. We lack good documentation on how to add new migrations.

Nathan and Patrick to distribute the work among them (doing the work and reviewing).

5.2 AST bump

This is still in discussion. It cannot happen before we released a 5.2 compatible version of ppxlib and the compiler has a stable 5.2 release.

ppx_deriving

The PR to port the standard derivers to ppxlib is merged.

There was a first attempt at a release but it was breaking too much rev deps for the following reasons:

  • ppx_deriving.show port removed an unused argument when the deriver was used on interfaces. Unfortunately it turned out that some users were setting that argument in that context sometimes even though it was not used there. We put it back for compatibility.
  • ppx_let had an extension name clash with [%map: ...] from ppx-deriving.map. The bug is actually on ppx_let's side as they are not namespacing there extension. This has been reported upstream. We also decided to remove this extension from ppx_deriving.map (same for iter and fold) as it was neither useful, nor actually used in opam and it simplified the release a lot.

We fixed a bug with ppx_deriving.make that was not handling sets of mutually recursive types properly. That also made us realize ppxlib's type deriver API is a bit surprising: in such a case derivers receive the full set of type declarations regardless of where the attribute is attached in the set. This allows the user to attach the deriving attribute only once for the set so it's convenient. We do not restrict it to be attached at the end of the set only so it can be confusing to users and ppx authors. We should probably make sure our documentation reflects that behaviour. Nathan will open an issue about that.

We attempted another release this week. There's one last thing to fix there but after that we should be good.

The next (and final) step is to deprecate the whole ppx_deriving API for writing derivers in favor of ppxlib's. Before doing that there are two important things to take care of:

  • Release a new version of ppx_deriving_yojson ported to ppxlib. There is an open PR already, Nathan will review it and cut a release.
  • Sonja mentioned that utop might be depending on ppx_deriving in some way for dynamic ppx linking. Nathan will reach out to utop maintainers to learn more about that.

ppx_deriving_yojson seems to violate the location invariants according to merlin but our location check doesn't seem to detect that. We'll need to investigate our location check and reproduce this error so we can actually fix it in ppx_deriving_yojson. We might be lucky and the ppxlib port might already fix this.

Refactoring of the driver's Transform module

Transform.t is a record type that represent individual ppx transformations as registered via Ppxlib.Driver.register_transformation, i.e. the entry point for ppx authors.

Part of the driver's work is to turn the list of registeredTransform.t into an ordered series of structure -> structure or signature -> signature functions that will ultimately apply all rewriting rules.

This is all done with that same Transform.t type even though it is not suited at all. We could really benefit from introducing a more concrete type for the different type of AST pass that we have and that must be applied in a specific order. That would mean changing the code so that it builds something of this new type from the list of Transform.t and would greatly simplify the code without changing the behaviour at all.

It should be transparent to users as this is all internal and private API of the driver.

Patrick will take a look at this as it is a good way for him to get more familiar with the driver.

Making the repositoring welcoming to new contributors

The repo is not very welcoming at the moment and it can be quite hard for new contributors to find their way across the codebase on their own at the moment or to even figure out how they can help.

We need to update our CONTRIBUTING.md, provide some kind of guided tour of the codebase and sort our issues better.

The issue tracker is pretty full atm so it will take some time to clean it up.

Moazzam volunteered to gave us some notes on the state of our documentation for newcomers so we can identify priorities there.

Clone this wiki locally