From b24b018071553491010f11be6ce60615193c7957 Mon Sep 17 00:00:00 2001 From: Patrick Ferris Date: Wed, 22 Apr 2026 12:54:33 +0100 Subject: [PATCH] Better migration and encoding errors The new error messages provide more context and make sure to mention that ppxlib raised the error. Signed-off-by: Patrick Ferris --- ast/ast.ml | 2 ++ astlib/encoding_502.ml | 3 +-- astlib/encoding_503.ml | 3 +-- astlib/encoding_504.ml | 3 +-- astlib/encoding_505.ml | 3 +-- astlib/error.ml | 24 ++++++++++++++++++++++++ astlib/migrate_410_409.ml | 3 +-- astlib/migrate_413_412.ml | 3 +-- astlib/migrate_414_413.ml | 3 +-- astlib/migrate_504_503.ml | 4 +--- astlib/migrate_505_504.ml | 4 +--- test/driver/compiler-pp/run.t | 16 ++++++++++++++-- test/encoding/503/migrations/run.t | 19 +++++++++++++++++++ test/encoding/504/migrations/run.t | 18 ++++++++++++++++++ 14 files changed, 86 insertions(+), 22 deletions(-) create mode 100644 astlib/error.ml diff --git a/ast/ast.ml b/ast/ast.ml index 1a034416b..90024f330 100644 --- a/ast/ast.ml +++ b/ast/ast.ml @@ -55,6 +55,8 @@ open Import (* Note that when bumping ppxlib's internal AST you will also need to update a few other files: - [astlib/pprintast.ml], details on how to update at the head of the file. + - [astlib/error.ml], change the error message to taken into account the new internal AST + version number. *) (* Source code locations (ranges of positions), used in parsetree. *) diff --git a/astlib/encoding_502.ml b/astlib/encoding_502.ml index e02f34782..425f581cd 100644 --- a/astlib/encoding_502.ml +++ b/astlib/encoding_502.ml @@ -2,8 +2,7 @@ module Ext_name = struct let ptyp_open = "ppxlib.migration.ptyp_open_502" end -let invalid_encoding ~loc name = - Location.raise_errorf ~loc "Invalid %s encoding" name +let invalid_encoding ~loc name = Error.invalid_encoding ~loc ~version:"5.2" name module To_501 = struct open Ast_501.Asttypes diff --git a/astlib/encoding_503.ml b/astlib/encoding_503.ml index b89efdd45..daed0a88f 100644 --- a/astlib/encoding_503.ml +++ b/astlib/encoding_503.ml @@ -2,8 +2,7 @@ module Ext_name = struct let ppat_effect = "ppxlib.migration.ppat_effect_503" end -let invalid_encoding ~loc name = - Location.raise_errorf ~loc "Invalid %s encoding" name +let invalid_encoding ~loc name = Error.invalid_encoding ~loc ~version:"5.3" name module To_502 = struct let encode_ppat_effect ~loc ~effect_ ~k = diff --git a/astlib/encoding_504.ml b/astlib/encoding_504.ml index 8d5bb9f8b..771e524f6 100644 --- a/astlib/encoding_504.ml +++ b/astlib/encoding_504.ml @@ -10,8 +10,7 @@ module Ext_name = struct let bivariant_pmty_with = "ppxlib.migration.bivariant_pmty_with_5_4" end -let invalid_encoding ~loc name = - Location.raise_errorf ~loc "Invalid %s encoding" name +let invalid_encoding ~loc name = Error.invalid_encoding ~loc ~version:"5.4" name module type AST = sig type payload diff --git a/astlib/encoding_505.ml b/astlib/encoding_505.ml index 9e31d1e4e..c553e8ed3 100644 --- a/astlib/encoding_505.ml +++ b/astlib/encoding_505.ml @@ -10,8 +10,7 @@ module Ext_name = struct let external_pmty_with = "ppxlib.migration.external_pmty_with_5_5" end -let invalid_encoding ~loc name = - Location.raise_errorf ~loc "Invalid %s encoding" name +let invalid_encoding ~loc name = Error.invalid_encoding ~loc ~version:"5.5" name module To_504 = struct open Ast_504.Asttypes diff --git a/astlib/error.ml b/astlib/error.ml new file mode 100644 index 000000000..bcd2265fa --- /dev/null +++ b/astlib/error.ml @@ -0,0 +1,24 @@ +let raise_error_txt ~loc = Location.raise_errorf ~loc "%a" Format.pp_print_text + +let migration_error ~loc ~from ~to_ msg = + let s = + Format.sprintf + "ppxlib migration error (migrating ocaml.%s to ocaml.%s): %s\n\n\ + Ppxlib was likely trying to migrate from %s to 5.2.0 and back again, \ + but encountered an AST node that could not be encoded. Either remove \ + the feature or open an issue at \ + https://github.com/ocaml-ppx/ppxlib/issues." + from to_ msg Sys.ocaml_version + in + raise_error_txt ~loc s + +let invalid_encoding ~loc ~version msg = + let s = + Format.sprintf + "ppxlib invalid encoding: %s\n\n\ + Ppxlib failed to decode a feature from the OCaml %s AST. If this does \ + not seem right, please do open an issue at \ + https://github.com/ocaml-ppx/ppxlib/issues." + msg version + in + raise_error_txt ~loc s diff --git a/astlib/migrate_410_409.ml b/astlib/migrate_410_409.ml index d1298e0c6..ae01351fc 100644 --- a/astlib/migrate_410_409.ml +++ b/astlib/migrate_410_409.ml @@ -2,8 +2,7 @@ module From = Ast_410 module To = Ast_409 let migration_error loc missing_feature = - Location.raise_errorf ~loc - "migration error: %s is not supported before OCaml 4.10" missing_feature + Error.migration_error ~loc ~from:"4.10" ~to_:"4.09" missing_feature let map_option f x = match x with None -> None | Some x -> Some (f x) diff --git a/astlib/migrate_413_412.ml b/astlib/migrate_413_412.ml index 350882ac6..7cc3203c7 100644 --- a/astlib/migrate_413_412.ml +++ b/astlib/migrate_413_412.ml @@ -3,8 +3,7 @@ module From = Ast_413 module To = Ast_412 let migration_error loc missing_feature = - Location.raise_errorf ~loc - "migration error: %s is not supported before OCaml 4.13" missing_feature + Error.migration_error ~loc ~from:"4.13" ~to_:"4.12" missing_feature let rec copy_toplevel_phrase : Ast_413.Parsetree.toplevel_phrase -> Ast_412.Parsetree.toplevel_phrase = diff --git a/astlib/migrate_414_413.ml b/astlib/migrate_414_413.ml index 90c268ba9..9cb093c57 100644 --- a/astlib/migrate_414_413.ml +++ b/astlib/migrate_414_413.ml @@ -3,8 +3,7 @@ module From = Ast_414 module To = Ast_413 let migration_error loc missing_feature = - Location.raise_errorf ~loc - "migration error: %s is not supported before OCaml 4.13" missing_feature + Error.migration_error ~loc ~from:"4.14" ~to_:"4.13" missing_feature let rec copy_toplevel_phrase : Ast_414.Parsetree.toplevel_phrase -> Ast_413.Parsetree.toplevel_phrase = diff --git a/astlib/migrate_504_503.ml b/astlib/migrate_504_503.ml index 5ff951e67..b93a06c8d 100644 --- a/astlib/migrate_504_503.ml +++ b/astlib/migrate_504_503.ml @@ -10,9 +10,7 @@ module Bivariant_param = struct end let bivariant_error ~loc = - Location.raise_errorf ~loc - "Ppxlib migration error: bivariant type parameters cannot be migrated from \ - OCaml 5.4 to 5.3" + Error.migration_error ~loc ~from:"5.4" ~to_:"5.3" "bivariant type parameters" let rec copy_toplevel_phrase : Ast_504.Parsetree.toplevel_phrase -> Ast_503.Parsetree.toplevel_phrase = diff --git a/astlib/migrate_505_504.ml b/astlib/migrate_505_504.ml index 45c9d530a..53591ffaa 100644 --- a/astlib/migrate_505_504.ml +++ b/astlib/migrate_505_504.ml @@ -33,9 +33,7 @@ module External_type = struct { v with value = List.rev v.value } let err ~loc = - Location.raise_errorf ~loc - "Ppxlib migration error: external type cannot be migrated from OCaml 5.5 \ - to 5.4" + Error.migration_error ~loc ~from:"5.5" ~to_:"5.4" "external types" end let copy_location x = x diff --git a/test/driver/compiler-pp/run.t b/test/driver/compiler-pp/run.t index 5ef61706b..dfc7b5c3d 100644 --- a/test/driver/compiler-pp/run.t +++ b/test/driver/compiler-pp/run.t @@ -31,7 +31,13 @@ Now if we run it with `--use-compiler-pp`, we should get the migration error: File "test.ml", line 1, characters 0-22: 1 | [%%named_existentials] ^^^^^^^^^^^^^^^^^^^^^^ - Error: migration error: existentials in pattern-matching is not supported before OCaml 4.13 + Error: ppxlib migration error (migrating ocaml.4.13 to ocaml.4.12): + existentials in pattern-matching + + Ppxlib was likely trying to migrate from 4.11.2 to 5.2.0 and back + again, but encountered an AST node that could not be encoded. Either + remove the feature or open an issue at + https://github.com/ocaml-ppx/ppxlib/issues. [1] This should also work for correction based code gen: @@ -61,5 +67,11 @@ and with the flag: File "test_inline.ml", lines 1-2, characters 0-38: 1 | type t = int 2 | [@@deriving_inline named_existentials] - Error: migration error: existentials in pattern-matching is not supported before OCaml 4.13 + Error: ppxlib migration error (migrating ocaml.4.13 to ocaml.4.12): + existentials in pattern-matching + + Ppxlib was likely trying to migrate from 4.11.2 to 5.2.0 and back + again, but encountered an AST node that could not be encoded. Either + remove the feature or open an issue at + https://github.com/ocaml-ppx/ppxlib/issues. [1] diff --git a/test/encoding/503/migrations/run.t b/test/encoding/503/migrations/run.t index a38377765..cd0342dd7 100644 --- a/test/encoding/503/migrations/run.t +++ b/test/encoding/503/migrations/run.t @@ -15,3 +15,22 @@ And that it is correctly decoded when migrated back up to 5.3+ ASTs: $ ./id_driver.exe effect_pattern.ml --use-compiler-pp let () = try comp () with | effect Xchg n, k -> continue k (n + 1) + +A test for the invalid encoding logic: + + $ cat > effect_pattern_encoded.ml << EOF + > let () = + > try comp () + > with + > | [%ppxlib.migration.ppat_effect_503 ? "Badly encoded ppat_effect!"] -> continue k (n + 1) + > EOF + $ ./id_driver.exe effect_pattern_encoded.ml --use-compiler-pp + File "effect_pattern_encoded.ml", line 4, characters 5-37: + 4 | | [%ppxlib.migration.ppat_effect_503 ? "Badly encoded ppat_effect!"] -> continue k (n + 1) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + Error: ppxlib invalid encoding: ppxlib.migration.ppat_effect_503 + + Ppxlib failed to decode a feature from the OCaml 5.3 AST. If this does + not seem right, please do open an issue at + https://github.com/ocaml-ppx/ppxlib/issues. + [1] diff --git a/test/encoding/504/migrations/run.t b/test/encoding/504/migrations/run.t index 4650d6f51..7ab50ef0f 100644 --- a/test/encoding/504/migrations/run.t +++ b/test/encoding/504/migrations/run.t @@ -43,6 +43,24 @@ And same for patterns: $ ./id_driver.exe pattern.ml --use-compiler-pp let (~a, ~b:_, c, ..) = x +We also check our invalid decoding logic: + $ cat > invalid_lbl_tuple_encoding.ml << EOF + > let x = + > [%ppxlib.migration.pexp_labeled_tuple_5_4 + > (((Some \`a), 0), ((Some \`b), 1), (None, "abc"))] + > EOF + + $ ./id_driver.exe invalid_lbl_tuple_encoding.ml --use-compiler-pp + File "invalid_lbl_tuple_encoding.ml", line 2, characters 3-42: + 2 | [%ppxlib.migration.pexp_labeled_tuple_5_4 + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + Error: ppxlib invalid encoding: ppxlib.migration.pexp_labeled_tuple_5_4 + + Ppxlib failed to decode a feature from the OCaml 5.4 AST. If this does + not seem right, please do open an issue at + https://github.com/ocaml-ppx/ppxlib/issues. + [1] + We also check that bivariant type parameters are correctly encoded and migrated: $ cat > bivariant.ml << EOF