-
Notifications
You must be signed in to change notification settings - Fork 98
Dev meeting 21 11 2023
- Deprecation of
Camlin ppxlib and how breaking that change is. -
Ast_builderandAst_patternhave a restricted API wrt attributes.Ast_helperseemed more flexible wrt attributes (see https://github.com/ocaml-ppx/ppxlib/issues/458 and https://discuss.ocaml.org/t/best-practices-for-attributes-in-ppx/13400) - We're stopping the efforts to manually keep ppxlib compatible with
trunk. - Ppxlib driver performance
- Sonja / @pitag-ha
- Carl / @ceastlund
In ppxlib, we've recently deprecated the moudle Caml which provided a way to qualify standard library functions/modules before Stdlib was there. Theoretically that's a breaking change, since Caml is exposed. We've discussed how breaking it is. It shouldn't be a problem: No matter whether PPXs use it as a compile time dependency or as a runtime dependency, they'd have to declare ppxlib.stdppx as such in their dune file. That's not the case for anyone, at least not for anyone in our (a bit outdated) current PPX universe.
People have been asking about why Ast_builder (and Ast_pattern) doesn't provide a way to handle attributes (https://github.com/ocaml-ppx/ppxlib/issues/458). Concretely, when creating a parsetree node with attributes, the current workflow is to use Ast_builder to create the node without attribute(s) and to functionally update that node to add the attribute(s). That's unnecessarily convoluted, bad in terms of stability, and bad in terms of minimazing heap allocations/optimizing performance.
Ast_helper, the old module to build parsetree nodes, used to be more flexible in that regard by providing attributes as an optional argument to its functions. However, Ast_helper is a lot messier than Ast_builder by being based on ref cells, even for default locations! We certainly want to avoid that people go back to using Ast_helpder.
Action-point Sonja: Answer on the issue that more flexibility in Ast_builder to handle attributes sounds very good.
We've also discussed the impact at Jane Street. Carl has pointed out that there people most likely construct nodes with attributes manually, at the cost of stability.
We've briefly discussed the effort of our recent efforts to keep ppxlib compatible with OCaml trunk by one of us allocating time to ppxlib to add the new migrations on every compiler parsetree change. We predicted this to be a lot less work than it actually is for the following reason: Before OCaml 5.1.0, all OCaml parsetree changes we remember were syntax additions. Syntax additions are very easy to capture in our parsetree migrations. However, both for OCaml 5.1.0 and for OCaml 5.2.0, several parsetree changes are parsing changes, such as the "type constraint on let bindings" change or the "differentiate between applicative and generative functors" change or the "have the parsetree support functions with arity > 1 directly" change. Those changes need a lot of attention to detail to get the migrations right.. It just isn't viable for us to do a very deep context switch from our usual work to write the migrations for those OCaml parsetree changes right away when introduced in the compiler. So we need to find a way that one ppxlib maintainer can allocate time to upstreaming Astlib to the compiler. That would mean having ppxlib contibuously co-installable with trunk.
We've briefly discussed why the ppxlib driver performance is worse than one would expect, and in particular why ppxlib parsetree traversals seem to have much more performance overhead than merlin or compiler parsetree traversals. Was that better when PPX parsetree traversals were based on Ast_mapper? The summary is: We don't know
We've also mentioned how the parsetree type is a pain in general due to being a mutual recursive type of ~20 types, but that's nothing we could change.