ppxlib icon indicating copy to clipboard operation
ppxlib copied to clipboard

Unclear update path for Ast_pattern.pexp_function in 0.36

Open tobil4sk opened this issue 1 month ago • 3 comments

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.

tobil4sk avatar Nov 26 '25 21:11 tobil4sk

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

I'll take a look shortly!

NathanReb avatar Nov 27 '25 09:11 NathanReb

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 ?

patricoferris avatar Dec 04 '25 14:12 patricoferris

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

tobil4sk avatar Dec 04 '25 15:12 tobil4sk