Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document current limitation of deriving_inline #32

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion lib/vcs/src/dune
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,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
Expand All @@ -36,6 +41,7 @@
(preprocess
(pps
-unused-code-warnings=force
ppx_compare
ppx_enumerate
ppx_sexp_conv
ppx_sexp_value)))
134 changes: 111 additions & 23 deletions lib/vcs/src/graph.ml
Original file line number Diff line number Diff line change
Expand Up @@ -48,29 +48,117 @@ module Node_kind = struct
}
[@@deriving sexp_of]

let equal =
(fun a__001_ b__002_ ->
if Stdlib.( == ) 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_ ->
Stdlib.( && )
(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_ ->
Stdlib.( && )
(Rev.equal _a__007_.rev _b__008_.rev)
(Stdlib.( && )
(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
(* 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:
<no differences produced by diff>
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:
<no differences produced by diff>
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:
<no differences produced by diff>
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
{ rev : Rev.t
; parent : Node.t
}
| Merge of
{ rev : Rev.t
; parent1 : Node.t
; parent2 : Node.t
}
[@@deriving_inline equal]

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 :
sig
val equal : t -> t -> bool
end)
end

include T
Expand Down
Loading