Unexpected migration error in OCaml 5.3

[johnridesabike]

I just installed a new OPAM switch for OCaml 5.3, and I'm getting the following error when I use the new effect syntax with a file that's preprocessed by the JSOO PPX. I'm not sure where the best place to report this is, so feel free to direct this issue elsewhere.

$ dune build
File "main.ml", line 6, characters 4-25:
6 |   | effect Random_bits, k -> Effect.Deep.continue k (Random.bits ())
        ^^^^^^^^^^^^^^^^^^^^^
Error: migration error: effect pattern is not supported before OCaml 5.03

Minimal example

dune-project

(lang dune 3.17)

dune

(executable
 (name main)
 (modes js)
 (js_of_ocaml
  (flags
   (--enable effects)))
 (preprocess
  (pps js_of_ocaml-ppx)))

main.ml

type _ Effect.t += Random_bits : int Effect.t

let handler f =
  match f () with
  | x -> x
  | effect Random_bits, k -> Effect.Deep.continue k (Random.bits ())

Versions:

$ opam show --field=version ocaml ppxlib js_of_ocaml-ppx
5.3.0
0.34.0
5.9.1

If I remove the preprocess stanza from the dune file, then dune build works with no errors.

[NathanReb]

This is unfortunately expected behaviour at the moment. The latest release is compatible with the 5.3 compiler but not with 5.3 language features.

To clarify, you can compile and preprocess with 5.3 if your project builds with 5.2. If you start using 5.3 language features, we can't migrate them to our internal AST. Updating our internal AST to 5.3, and therefore supporting effects will happen in a subsequent release.

I apologize for the inconvenience and hopefully this will be fixed shortly but bumping ppxlib's internal AST is quite a heavy process as we patch reverse dependencies to lighten the burden of using the unstable parsetree for ppx authors and maintainers.

[hhugo]

@NathanReb, couldn't we rewrite the effect syntax with an extension point during the downgrade and restore it after processors have run ?

Something like the following

let handler f =
  match f () with
  | x -> x
  | [%ppxlib.effect_syntax? Random_bits, k] -> Effect.Deep.continue k (Random.bits ())

[hhugo]

PoC #552

[Octachron]

Maybe [%ppxlib.migration.effect ... ] to make place for other potential ppxlib's migration extension.

Also since a pattern transformer should probably not recognize Random_bits, k as a tuple pattern maybe

let handler f =
  match f () with
  | x -> x
  | [%ppxlib.migration.eff? Random_bits][@ppxlib.migration.kont? k] -> Effect.Deep.continue k (Random.bits ())

?

[NathanReb]

Yes, we could definitely do something like that for now. Thanks for the POC, I'll take a look!

[NathanReb]

Also since a pattern transformer should probably not recognize Random_bits, k as a tuple pattern maybe

I guess we don't really expect anyone to interpret this extension's payload expect for ppxlib itself, especially since this is a temporary measure and hopefully we'll soon have full support for 5.3!

[johnridesabike]

In the event that a fix is not imminent, or in case it comes up again in another release, it would also be very nice if the error message was improved at a minimum. Since the error appears when the user is in fact using 5.3, the message text seems confusingly incorrect. It's also not clear to the average user what caused the error. (Of course, if the fix can be made soon then that would be best!)

[NathanReb]

I'm working on a fix right now as I agree in practice this simply makes the ppx experience very bad in 5.3 at the moment.

ppx-es still won't be able to consume or produce code using the effect syntax but at least you'll be able to use it.

I agree the error message is particularly confusing to people unaware of ppxlib's internals, that is: most people. We need to improve it. I'll open an issue to keep track of that specific problem!

[NathanReb]

I just merged #552 and will release it today. I'm closing this but feel free to re-open if the issue resurfaces somehow!