From 69bdb0d73d2c6e4242b3e6cfae297f37e13c44f1 Mon Sep 17 00:00:00 2001 From: Nathan Rebours Date: Mon, 13 Jan 2025 14:31:46 +0100 Subject: [PATCH 1/3] Add tests for 5.3 effects roundtrip Signed-off-by: Nathan Rebours --- .../preserve-effect-downward/driver.ml | 18 +++++++++++++ .../preserve-effect-downward/dune | 10 ++++++++ .../preserve-effect-downward/run.t | 25 +++++++++++++++++++ 3 files changed, 53 insertions(+) create mode 100644 test/503_migrations/preserve-effect-downward/driver.ml create mode 100644 test/503_migrations/preserve-effect-downward/dune create mode 100644 test/503_migrations/preserve-effect-downward/run.t diff --git a/test/503_migrations/preserve-effect-downward/driver.ml b/test/503_migrations/preserve-effect-downward/driver.ml new file mode 100644 index 00000000..fb80f19b --- /dev/null +++ b/test/503_migrations/preserve-effect-downward/driver.ml @@ -0,0 +1,18 @@ +module To_before_503 = + Ppxlib_ast.Convert (Ppxlib_ast.Js) (Ppxlib_ast__.Versions.OCaml_502) + +module From_before_503 = + Ppxlib_ast.Convert + (Ppxlib_ast__.Versions.OCaml_502) + (Ppxlib_ast.Compiler_version) + +let impl _ctxt str = + (* This manual migration is here to ensure the test still works even once our + internal AST has been bumped past 5.3 *) + let before_503_ast = To_before_503.copy_structure str in + let roundtrip = From_before_503.copy_structure before_503_ast in + Ocaml_common.Pprintast.structure Format.std_formatter roundtrip; + str + +let () = Ppxlib.Driver.V2.register_transformation ~impl "503-downward-roundtrip" +let () = Ppxlib.Driver.standalone () diff --git a/test/503_migrations/preserve-effect-downward/dune b/test/503_migrations/preserve-effect-downward/dune new file mode 100644 index 00000000..35d09cfe --- /dev/null +++ b/test/503_migrations/preserve-effect-downward/dune @@ -0,0 +1,10 @@ +(executable + (name driver) + (enabled_if + (>= %{ocaml_version} "5.3")) + (libraries ppxlib ocaml-compiler-libs.common compiler-libs.common)) + +(cram + (enabled_if + (>= %{ocaml_version} "5.3")) + (deps driver.exe)) diff --git a/test/503_migrations/preserve-effect-downward/run.t b/test/503_migrations/preserve-effect-downward/run.t new file mode 100644 index 00000000..a70441fe --- /dev/null +++ b/test/503_migrations/preserve-effect-downward/run.t @@ -0,0 +1,25 @@ +We should be able to migrate the effect syntax downward and back. This allows users +to run ppx-es on 5.3 on source files that use the effect syntax until we update our +internal AST to fully support it. + +We have a custom driver that will force migration of the AST down to 5.2 and back to +the compiler's version and print it as source code using the compiler's printer, +regardless of ppxlib's internal AST version. + +If we run the driver on the following source file: + + $ cat > test.ml << EOF + > let handler f = + > match f () with + > | x -> x + > | effect Random_bits, k -> Effect.Deep.continue k (Random.bits ()) + > EOF + +it should successfully roundtrip to 5.2 and print the source code unchanged: + + $ ./driver.exe test.ml -o test.pp.ml + File "test.ml", line 4, characters 2-23: + 4 | | effect Random_bits, k -> Effect.Deep.continue k (Random.bits ()) + ^^^^^^^^^^^^^^^^^^^^^ + Error: migration error: effect pattern is not supported before OCaml 5.03 + [1] From f4ba4c197356bbbf4a6f0c2997d2d5620ad36663 Mon Sep 17 00:00:00 2001 From: Hugo Heuzard Date: Mon, 13 Jan 2025 12:38:25 +0100 Subject: [PATCH 2/3] 5.3 effect syntax Signed-off-by: Nathan Rebours --- astlib/migrate_502_503.ml | 4 ++++ astlib/migrate_503_502.ml | 16 +++++++++++----- .../preserve-effect-downward/run.t | 9 ++++----- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/astlib/migrate_502_503.ml b/astlib/migrate_502_503.ml index 94179a0b..05c30ee8 100644 --- a/astlib/migrate_502_503.ml +++ b/astlib/migrate_502_503.ml @@ -325,6 +325,10 @@ and copy_pattern_desc : (copy_loc (fun x -> Option.map (fun x -> x) x) x0) | Ast_502.Parsetree.Ppat_exception x0 -> Ast_503.Parsetree.Ppat_exception (copy_pattern x0) + | Ast_502.Parsetree.Ppat_extension + ( { txt = "ppxlib.effect_syntax"; _ }, + PPat ({ ppat_desc = Ppat_tuple [ e; c ]; _ }, None) ) -> + Ast_503.Parsetree.Ppat_effect (copy_pattern e, copy_pattern c) | Ast_502.Parsetree.Ppat_extension x0 -> Ast_503.Parsetree.Ppat_extension (copy_extension x0) | Ast_502.Parsetree.Ppat_open (x0, x1) -> diff --git a/astlib/migrate_503_502.ml b/astlib/migrate_503_502.ml index cf11c888..d3be09e7 100644 --- a/astlib/migrate_503_502.ml +++ b/astlib/migrate_503_502.ml @@ -2,10 +2,6 @@ open Stdlib0 module From = Ast_503 module To = Ast_502 -let migration_error loc missing_feature = - Location.raise_errorf ~loc - "migration error: %s is not supported before OCaml 5.03" missing_feature - let rec copy_toplevel_phrase : Ast_503.Parsetree.toplevel_phrase -> Ast_502.Parsetree.toplevel_phrase = function @@ -330,7 +326,17 @@ and copy_pattern_desc loc : (copy_loc (fun x -> Option.map (fun x -> x) x) x0) | Ast_503.Parsetree.Ppat_exception x0 -> Ast_502.Parsetree.Ppat_exception (copy_pattern x0) - | Ast_503.Parsetree.Ppat_effect _ -> migration_error loc "effect pattern" + | Ast_503.Parsetree.Ppat_effect (e, c) -> + Ast_502.Parsetree.Ppat_extension + ( Location.{ txt = "ppxlib.effect_syntax"; loc = Location.none }, + Ast_502.Parsetree.PPat + ( { + ppat_desc = Ppat_tuple [ copy_pattern e; copy_pattern c ]; + ppat_attributes = []; + ppat_loc_stack = []; + ppat_loc = Location.none; + }, + None ) ) | Ast_503.Parsetree.Ppat_extension x0 -> Ast_502.Parsetree.Ppat_extension (copy_extension x0) | Ast_503.Parsetree.Ppat_open (x0, x1) -> diff --git a/test/503_migrations/preserve-effect-downward/run.t b/test/503_migrations/preserve-effect-downward/run.t index a70441fe..a69f6e95 100644 --- a/test/503_migrations/preserve-effect-downward/run.t +++ b/test/503_migrations/preserve-effect-downward/run.t @@ -18,8 +18,7 @@ If we run the driver on the following source file: it should successfully roundtrip to 5.2 and print the source code unchanged: $ ./driver.exe test.ml -o test.pp.ml - File "test.ml", line 4, characters 2-23: - 4 | | effect Random_bits, k -> Effect.Deep.continue k (Random.bits ()) - ^^^^^^^^^^^^^^^^^^^^^ - Error: migration error: effect pattern is not supported before OCaml 5.03 - [1] + let handler f = + match f () with + | x -> x + | effect Random_bits, k -> Effect.Deep.continue k (Random.bits ()) From a72b54342dc49c50c86e3f48b13adfda02a07872 Mon Sep 17 00:00:00 2001 From: Nathan Rebours Date: Mon, 13 Jan 2025 15:32:11 +0100 Subject: [PATCH 3/3] Use ppxlib.migration namespace and attach loc to ppxlib.effects_syntax Signed-off-by: Nathan Rebours --- astlib/migrate_502_503.ml | 2 +- astlib/migrate_503_502.ml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/astlib/migrate_502_503.ml b/astlib/migrate_502_503.ml index 05c30ee8..0b4c99a6 100644 --- a/astlib/migrate_502_503.ml +++ b/astlib/migrate_502_503.ml @@ -326,7 +326,7 @@ and copy_pattern_desc : | Ast_502.Parsetree.Ppat_exception x0 -> Ast_503.Parsetree.Ppat_exception (copy_pattern x0) | Ast_502.Parsetree.Ppat_extension - ( { txt = "ppxlib.effect_syntax"; _ }, + ( { txt = "ppxlib.migration.ppat_effect"; _ }, PPat ({ ppat_desc = Ppat_tuple [ e; c ]; _ }, None) ) -> Ast_503.Parsetree.Ppat_effect (copy_pattern e, copy_pattern c) | Ast_502.Parsetree.Ppat_extension x0 -> diff --git a/astlib/migrate_503_502.ml b/astlib/migrate_503_502.ml index d3be09e7..3f55081d 100644 --- a/astlib/migrate_503_502.ml +++ b/astlib/migrate_503_502.ml @@ -328,13 +328,13 @@ and copy_pattern_desc loc : Ast_502.Parsetree.Ppat_exception (copy_pattern x0) | Ast_503.Parsetree.Ppat_effect (e, c) -> Ast_502.Parsetree.Ppat_extension - ( Location.{ txt = "ppxlib.effect_syntax"; loc = Location.none }, + ( Location.{ txt = "ppxlib.migration.ppat_effect"; loc }, Ast_502.Parsetree.PPat ( { ppat_desc = Ppat_tuple [ copy_pattern e; copy_pattern c ]; ppat_attributes = []; ppat_loc_stack = []; - ppat_loc = Location.none; + ppat_loc = loc; }, None ) ) | Ast_503.Parsetree.Ppat_extension x0 ->