Unclear update path for Ast_pattern.pexp_function in 0.36

[tobil4sk]

ppx_parser has a pattern that uses Ast_pattern.pexp_function from ppxlib 0.35 and below:

https://github.com/NielsMommen/ppx_parser/blob/c1d8f379a05ddfd55306f58ed643066b07549b3f/rewriter/ppx_parser.ml#L3-L7

let ppx_parser_pat = Ast_pattern.(single_expr_payload
  (alt
    (pexp_function __ |> map1 ~f:(fun cases -> (None, cases)) )
    (pexp_match __ __ |> map2 ~f:(fun e cases -> (Some e, cases)) ))
  )

However, in ppxlib 0.36 and above, the signature has changed and this no longer compiles.

In the migration guide: https://github.com/ocaml-ppx/ppxlib/wiki/Upgrading-to-ppxlib-0.36.0:

Since Pexp_function now represents all functions and not just the special pattern-matching case, pexp_function now builds full functions and not just ones with cases. When upgrading, your easiest path is to replace your calls to pexp_function with calls to pexp_function_cases.

This is for Ast_builder. There is no equivalent Ast_pattern.pexp_function_cases, is this intentional or an oversight?

From the documentation: https://ocaml.org/p/ppxlib/0.37.0/doc/matching-code.html#building-patterns

All constructors from Ast_builder have a "deconstructor" in Ast_pattern with the same name.

[NathanReb]

It indeed looks like an oversight, thanks for reporting this!

I'll take a look shortly!

[patricoferris]

Thanks for spotting this @tobil4sk. I think this was indeed an oversight.

I believe you can write your own version without needing to wait for this to be added (and the subsequent release):

  let ppx_parser_pat =
    let open Ast_pattern in
    single_expr_payload @@
    alt
      (pexp_function drop drop (pfunction_cases __ drop drop) |> map1 ~f:(fun cases -> (None, cases)))
      (pexp_match __ __ |> map2 ~f:(fun e cases -> (Some e, cases)))

I think this has the same semantics as the old version, @NathanReb ?

[tobil4sk]

Thank you @patricoferris! That compiles and seems fine with the ppx_parser test suite.

[patricoferris]

I have updated the documentation for migrating to include this information. I think, given how few requests there are for this, I am going to close this for now. Providing a pexp_function_cases is a little tricky and might make too many assumptions compared to the old pexp_function (e.g. what to do with any arguments that are in the new Pexp_function node?). I think users are better guided towards pexp_function and ensuring they write the logic they want.

If anyone feels differently please do comment here.