From 079f71c7cdfa566a1965e66116c8b9f4495c460f Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Mon, 16 Dec 2024 13:07:23 +0100 Subject: [PATCH 1/4] Document limitation --- lib/vcs/src/dune | 7 +++++- lib/vcs/src/graph.ml | 53 +++++++++++++++++++++++++++----------------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/lib/vcs/src/dune b/lib/vcs/src/dune index c7c685f..edcad04 100644 --- a/lib/vcs/src/dune +++ b/lib/vcs/src/dune @@ -17,7 +17,12 @@ (instrumentation (backend bisect_ppx)) (lint - (pps ppx_js_style -allow-let-operators -check-doc-comments)) + (pps + -unused-code-warnings=force + ppx_compare + ppx_js_style + -allow-let-operators + -check-doc-comments)) (modules_without_implementation trait_add trait_branch diff --git a/lib/vcs/src/graph.ml b/lib/vcs/src/graph.ml index 78f004d..9fdd878 100644 --- a/lib/vcs/src/graph.ml +++ b/lib/vcs/src/graph.ml @@ -48,26 +48,39 @@ module Node_kind = struct } [@@deriving sexp_of] - let equal = - (fun a__001_ b__002_ -> - if a__001_ == b__002_ - then true - else ( - match a__001_, b__002_ with - | Root _a__003_, Root _b__004_ -> Rev.equal _a__003_.rev _b__004_.rev - | Root _, _ -> false - | _, Root _ -> false - | Commit _a__005_, Commit _b__006_ -> - Rev.equal _a__005_.rev _b__006_.rev - && Node.equal _a__005_.parent _b__006_.parent - | Commit _, _ -> false - | _, Commit _ -> false - | Merge _a__007_, Merge _b__008_ -> - Rev.equal _a__007_.rev _b__008_.rev - && Node.equal _a__007_.parent1 _b__008_.parent1 - && Node.equal _a__007_.parent2 _b__008_.parent2) - : t -> t -> bool) - ;; + (* CR mbarbin: I wish to be able to use `deriving_inline` on additional ppx + without adding then as build dependency, nor adding a runtime dependency + into their runtime lib. + + Currently, I cannot do that, without disabling ppx entirely for this + directory. I don't want to do that, because I want to keep the [ppx] for + the other ppx that are used in this directory, such as [sexp_of]. Also, I + need the ppx for constructions such as `[%sexp]`, for which there doesn't + exist a `deriving_inline` equivalent. *) + + include ( + struct + type nonrec t = t = + | Root of { rev : Rev.t } + | Commit of + { rev : Rev.t + ; parent : Node.t + } + | Merge of + { rev : Rev.t + ; parent1 : Node.t + ; parent2 : Node.t + } + [@@deriving_inline equal] + + [%%ocaml.error + "Ppxlib.Deriving: 'equal' is not a supported type deriving generator"] + + [@@@deriving.end] + end : + sig + val equal : t -> t -> bool + end) end include T From d9126d54d6baf241272518cd2f7f49f8f3c996ea Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Mon, 21 Oct 2024 17:47:35 +0200 Subject: [PATCH 2/4] Steps to trigger some build issues related to deriving_inline --- lib/vcs/src/dune | 1 + lib/vcs/src/graph.ml | 82 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/lib/vcs/src/dune b/lib/vcs/src/dune index edcad04..8607ede 100644 --- a/lib/vcs/src/dune +++ b/lib/vcs/src/dune @@ -43,6 +43,7 @@ (preprocess (pps -unused-code-warnings=force + ppx_compare ppx_enumerate ppx_sexp_conv ppx_sexp_value))) diff --git a/lib/vcs/src/graph.ml b/lib/vcs/src/graph.ml index 9fdd878..077e8dc 100644 --- a/lib/vcs/src/graph.ml +++ b/lib/vcs/src/graph.ml @@ -60,6 +60,63 @@ module Node_kind = struct include ( struct + (* CR mbarbin: Here is a sequence that produces some issue for me: + + 1, Starting from a build in watch mode: `dune build @all @lint -w` + + 2. After everything stabilizes, I get the "Success, waiting for + filesystem changes..." output from dune. + + Then I try the following: + + 3. Replace the `@@deriving` by a `@@deriving_inline`, and add an additional + line after it: `[@@@deriving.end]`. + + 4. Save the file. + + This triggers a rebuild, and a promotion error that shows the code to be inserted. + + 5. I run: `dune promote`. + + This creates the following error: + + {v + Error: ppxlib: the corrected code doesn't round-trip. + This is probably a bug in the OCaml printer: + + diff: /tmp/build_2c6e2f_dune/ppxlib6f4ca1: No such file or directory + diff: /tmp/build_2c6e2f_dune/ppxlibaf673d: No such file or directory + Had 2 errors, waiting for filesystem changes... + v} + + 6. If I save the file again, the code gets reformatted but the error + does not go away. + + {v + Error: ppxlib: the corrected code doesn't round-trip. + This is probably a bug in the OCaml printer: + + Had 1 error, waiting for filesystem changes... + v} + + 7. If I kill the build, and restart it the error is now different (looks worse): + + {v +Error: ppxlib: the corrected code doesn't round-trip. +This is probably a bug in the OCaml printer: + +Uncaught exception: + + (Failure + "Both files, /tmp/build_f345a9_dune/ppxlib7c84c6 and /tmp/build_f345a9_dune/ppxlibe07aed, do not exist") + +Raised at Stdlib.failwith in file "stdlib.ml", line 29, characters 17-33 +Called from Dune__exe__Compare.compare_main in file "bin/compare.ml", line 129, characters 7-77 +Called from Dune__exe__Compare.main in file "bin/compare.ml", line 174, characters 13-38 +Called from Command.For_unix.run.(fun) in file "command/src/command.ml", lines 3388-3399, characters 8-31 +Called from Base__Exn.handle_uncaught_aux in file "src/exn.ml", line 126, characters 6-10 + v} + *) type nonrec t = t = | Root of { rev : Rev.t } | Commit of @@ -73,8 +130,29 @@ module Node_kind = struct } [@@deriving_inline equal] - [%%ocaml.error - "Ppxlib.Deriving: 'equal' is not a supported type deriving generator"] + let equal = + (fun a__016_ b__017_ -> + if Stdlib.( == ) a__016_ b__017_ + then true + else ( + match a__016_, b__017_ with + | Root _a__018_, Root _b__019_ -> Rev.equal _a__018_.rev _b__019_.rev + | Root _, _ -> false + | _, Root _ -> false + | Commit _a__020_, Commit _b__021_ -> + Stdlib.( && ) + (Rev.equal _a__020_.rev _b__021_.rev) + (Node.equal _a__020_.parent _b__021_.parent) + | Commit _, _ -> false + | _, Commit _ -> false + | Merge _a__022_, Merge _b__023_ -> + Stdlib.( && ) + (Rev.equal _a__022_.rev _b__023_.rev) + (Stdlib.( && ) + (Node.equal _a__022_.parent1 _b__023_.parent1) + (Node.equal _a__022_.parent2 _b__023_.parent2))) + : t -> t -> bool) + ;; [@@@deriving.end] end : From bc6c267ef5832b696fdbc3cbb9276b67d69064e3 Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Mon, 16 Dec 2024 13:14:44 +0100 Subject: [PATCH 3/4] Update using NathanReb/improve-deriving-inline --- lib/vcs/src/dune | 2 +- lib/vcs/src/graph.ml | 23 ++++++++++++----------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/vcs/src/dune b/lib/vcs/src/dune index 8607ede..1f985f6 100644 --- a/lib/vcs/src/dune +++ b/lib/vcs/src/dune @@ -43,7 +43,7 @@ (preprocess (pps -unused-code-warnings=force - ppx_compare + -no-corrections ppx_enumerate ppx_sexp_conv ppx_sexp_value))) diff --git a/lib/vcs/src/graph.ml b/lib/vcs/src/graph.ml index 077e8dc..1b010de 100644 --- a/lib/vcs/src/graph.ml +++ b/lib/vcs/src/graph.ml @@ -131,26 +131,27 @@ Called from Base__Exn.handle_uncaught_aux in file "src/exn.ml", line 126, charac [@@deriving_inline equal] let equal = - (fun a__016_ b__017_ -> - if Stdlib.( == ) a__016_ b__017_ + (fun a__001_ -> + fun b__002_ -> + if Stdlib.( == ) a__001_ b__002_ then true else ( - match a__016_, b__017_ with - | Root _a__018_, Root _b__019_ -> Rev.equal _a__018_.rev _b__019_.rev + match a__001_, b__002_ with + | Root _a__003_, Root _b__004_ -> Rev.equal _a__003_.rev _b__004_.rev | Root _, _ -> false | _, Root _ -> false - | Commit _a__020_, Commit _b__021_ -> + | Commit _a__005_, Commit _b__006_ -> Stdlib.( && ) - (Rev.equal _a__020_.rev _b__021_.rev) - (Node.equal _a__020_.parent _b__021_.parent) + (Rev.equal _a__005_.rev _b__006_.rev) + (Node.equal _a__005_.parent _b__006_.parent) | Commit _, _ -> false | _, Commit _ -> false - | Merge _a__022_, Merge _b__023_ -> + | Merge _a__007_, Merge _b__008_ -> Stdlib.( && ) - (Rev.equal _a__022_.rev _b__023_.rev) + (Rev.equal _a__007_.rev _b__008_.rev) (Stdlib.( && ) - (Node.equal _a__022_.parent1 _b__023_.parent1) - (Node.equal _a__022_.parent2 _b__023_.parent2))) + (Node.equal _a__007_.parent1 _b__008_.parent1) + (Node.equal _a__007_.parent2 _b__008_.parent2))) : t -> t -> bool) ;; From 1ada1736302fe094540dafd1fdd5374ac569fe72 Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Mon, 16 Dec 2024 13:23:47 +0100 Subject: [PATCH 4/4] Update CRs --- lib/vcs/src/graph.ml | 81 ++++++++++---------------------------------- 1 file changed, 17 insertions(+), 64 deletions(-) diff --git a/lib/vcs/src/graph.ml b/lib/vcs/src/graph.ml index 1b010de..9cfdd5f 100644 --- a/lib/vcs/src/graph.ml +++ b/lib/vcs/src/graph.ml @@ -48,75 +48,28 @@ module Node_kind = struct } [@@deriving sexp_of] - (* CR mbarbin: I wish to be able to use `deriving_inline` on additional ppx - without adding then as build dependency, nor adding a runtime dependency - into their runtime lib. + (* CR mbarbin: This almost works now with [NathanReb/improve-deriving-inline]. - Currently, I cannot do that, without disabling ppx entirely for this - directory. I don't want to do that, because I want to keep the [ppx] for - the other ppx that are used in this directory, such as [sexp_of]. Also, I - need the ppx for constructions such as `[%sexp]`, for which there doesn't - exist a `deriving_inline` equivalent. *) + Building Works! - include ( - struct - (* CR mbarbin: Here is a sequence that produces some issue for me: - - 1, Starting from a build in watch mode: `dune build @all @lint -w` - - 2. After everything stabilizes, I get the "Success, waiting for - filesystem changes..." output from dune. - - Then I try the following: - - 3. Replace the `@@deriving` by a `@@deriving_inline`, and add an additional - line after it: `[@@@deriving.end]`. - - 4. Save the file. - - This triggers a rebuild, and a promotion error that shows the code to be inserted. + {v + $ dune build @all + [0] + v} - 5. I run: `dune promote`. + However, the linter shows small mismatch in ocamlformatting. For example, + it wants to add parenthesis around certain tuple below: - This creates the following error: + {[ + match (a__001_, b__002_) with + | (Root _a__003_, Root _b__004_) -> Rev.equal _a__003_.rev _b__004_.rev + | (Root _, _) -> false + | (_, Root _) -> false + ]} + *) - {v - Error: ppxlib: the corrected code doesn't round-trip. - This is probably a bug in the OCaml printer: - - diff: /tmp/build_2c6e2f_dune/ppxlib6f4ca1: No such file or directory - diff: /tmp/build_2c6e2f_dune/ppxlibaf673d: No such file or directory - Had 2 errors, waiting for filesystem changes... - v} - - 6. If I save the file again, the code gets reformatted but the error - does not go away. - - {v - Error: ppxlib: the corrected code doesn't round-trip. - This is probably a bug in the OCaml printer: - - Had 1 error, waiting for filesystem changes... - v} - - 7. If I kill the build, and restart it the error is now different (looks worse): - - {v -Error: ppxlib: the corrected code doesn't round-trip. -This is probably a bug in the OCaml printer: - -Uncaught exception: - - (Failure - "Both files, /tmp/build_f345a9_dune/ppxlib7c84c6 and /tmp/build_f345a9_dune/ppxlibe07aed, do not exist") - -Raised at Stdlib.failwith in file "stdlib.ml", line 29, characters 17-33 -Called from Dune__exe__Compare.compare_main in file "bin/compare.ml", line 129, characters 7-77 -Called from Dune__exe__Compare.main in file "bin/compare.ml", line 174, characters 13-38 -Called from Command.For_unix.run.(fun) in file "command/src/command.ml", lines 3388-3399, characters 8-31 -Called from Base__Exn.handle_uncaught_aux in file "src/exn.ml", line 126, characters 6-10 - v} - *) + include ( + struct type nonrec t = t = | Root of { rev : Rev.t } | Commit of