ocaml-ppx / ocaml-migrate-parsetree

DEPRECATED. See https://ocaml.org/changelog/2023-10-23-omp-deprecation. Convert OCaml parsetrees between different major versions
Other
87 stars 43 forks source link

gencopy fails to build on OCaml 4.13.1 #118

Closed rwmjones closed 2 years ago

rwmjones commented 2 years ago
+ dune build -j48
File "tools/gencopy.ml", line 79, characters 43-74:
79 |     Pat.construct ?loc ?attrs (lid ?loc s) (may_tuple ?loc Pat.tuple args)
                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: This expression has type Parsetree.pattern option
       but an expression was expected of type
         (str list * Parsetree.pattern) option
       Type Parsetree.pattern is not compatible with type
         str list * Parsetree.pattern 
rwmjones commented 2 years ago

This patch is what we're carrying in Fedora. It allows compilation but I've no idea if it's correct:

https://src.fedoraproject.org/rpms/ocaml-migrate-parsetree/blob/rawhide/f/0001-tools-gencopy.ml-Fix-compilation.patch

pitag-ha commented 2 years ago

Ok, thanks for reporting!

It would still be good to keep on using may_tuple since the variant constructor might have no or only one parameter (whereas if you use Pat.tuple always, you're assuming that it has more than one parameter), so I think something like

Pat.construct ?loc ?attrs (lid ?loc s) (Option.map (fun tuple -> ([], tuple)) (may_tuple ?loc Pat.tuple args))

would work.

If this isn't urgent now, I'll look into it in more detail when we need to generate the migration modules for 4.14. We'll need to generate them for ppxlib directly, not for OMP anymore, but we'll still use this same tooling.

rwmjones commented 2 years ago

Sure, no problem at the moment. I have enough of a workaround for now.

pitag-ha commented 2 years ago

Cool! By the way, out of curiosity: What do you need the gencopy for?

rwmjones commented 2 years ago

We just package it in Fedora. I don't have any insight into whether anyone uses it.

pitag-ha commented 2 years ago

This has been fixed in #119.