ocaml-ppx
Bump AST to OCaml 5.2.0
This is a draft PR to bump the AST of ppxlib to OCaml's 5.2 AST. The changes to the internal representation of functions are the main challenge, regardless of what we do we will need to patch quite a few ppxes that directly pattern-match on the constructors of the AST for functions.
There are a lot of changes to the pretty-printing too which I copied mostly from upstream -- it is unclear to me if this file is supposed to be copied across or updated with ppxlib specific changes ? @NathanReb or @pitag-ha maybe you could shed some light here. This is currently breaking this PR on older compiler versions but I'm not 100% sure what is causing that or where the check is ?
Some code is not round-tripping here and I think it might be caused by (although I could be wrong) function arity issues. See the 4.12.1 build in the CI (there are other issues too I know). But it seems something like
type t = { x : int; y : int }
let f = fun z -> fun { x; y } -> x + y + z
and this roundtrip to
let f = fun z { x; y } -> x + y + z
Though I thought the latter is just syntactic sugar for the former, but then I haven't read the new arity change PR in full yet!
The actual code causing this is:
method include_infos :
'a . ('a -> 'a) -> 'a include_infos -> 'a include_infos=
fun _a ->
fun { pincl_mod; pincl_loc; pincl_attributes } ->
let pincl_mod = _a pincl_mod in
let pincl_loc = self#location pincl_loc in
let pincl_attributes = self#attributes pincl_attributes in
{ pincl_mod; pincl_loc; pincl_attributes }
Although it is unclear given the diff presented. Would it make more sense to write a structural equality checker ourselves that could output the problematic AST nodes when they don't compare correctly rather than using Poly.( <> ) + s-expression diff?
Having a proper comparison function for this and a proper printer would be great yes. I guess the printer is a bit more important in the short term as it would help us solve this particular issue.
We can probably work out a decent enough printer using Ast_traverse.lift.
From a quick look I think it's the other way around, our migrations are bugged in a way that will turn fun x y -> x + y into fun x -> fun y -> x + y.
I'm looking into testing and fixing this!
Another tricky corner case that has appeared during the 5.2 AST bump is that local opens in types make it very hard (perhaps impossible) to know if a type is recursive or not using really_recursive/type_is_recursive.
module M = struct
type t = A
end
type t = M.(t)
When asked if type t = M.(t) is recursive or not are tests say yes. Of course, it could be the case that it actually is recursive. We just don't know without going to look at the module and working out if there's actually a t in there if it is or not.
Of course this was already the case with global opens too I suppose -- just thought I'd add that here.