ppxlib icon indicating copy to clipboard operation
ppxlib copied to clipboard

"Corrected code doesn't round-trip" with deriving_inline

Open sim642 opened this issue 2 years ago • 7 comments

I have defined a deriver named "leq" with ppxlib, but I guess any ppxlib-based deriver would have the same issue. First, I use it in a test file as follows:

type t = L1.t [@@deriving_inline leq][@@@end]

Second, running dune's build and promote changes it to the following:

type t = L1.t [@@deriving_inline leq]
let _ = fun (_ : t) -> ()
let rec (leq : t -> t -> bool) =
  let __0 () = L1.leq in
  ((let open! ((Ppx_deriving_runtime)[@ocaml.warning "-A"]) in __0 ())
    [@ocaml.warning "-A"])[@@ocaml.warning "-39"]
let _ = leq
[@@@end]

This promoted file now refuses to build by giving the following error:

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_f87c44_dune/ppxlibb4e945: No such file or directory
diff: /tmp/build_f87c44_dune/ppxlib38b9eb: No such file or directory

Third, I discovered that removing the let _ definitions and manually changed the file to the following:

type t = L1.t [@@deriving_inline leq]
let rec (leq : t -> t -> bool) =
  let __0 () = L1.leq in
  ((let open! ((Ppx_deriving_runtime)[@ocaml.warning "-A"]) in __0 ())
    [@ocaml.warning "-A"])[@@ocaml.warning "-39"]
[@@@end]

Then the round-tripping error disappears, but dune build again suggests a promotion to add the two definitions back, leading back to the second code snippet above.

Therefore I cannot get deriving_inline to a stable state, where it builds without round-trip errors and doesn't keep suggesting additional promotions.

sim642 avatar Apr 30 '22 09:04 sim642

Thanks for the report!

Time allocated to ppxlib has become a scarce ressource, however I'll do my best to investigate this issue. Sorry in advance for the dealy!

panglesd avatar May 12 '22 13:05 panglesd

Hey, thanks for reporting and sorry for the delay! I've just tried to reproduce the problem and am not able to reproduce it. Are you using a mix between ppx_deriving and ppxlib to define your leq deriver? I'm wondering because of the open of the ppx_deriving runtime (which might be related to the round-trip problem, but I'm not sure).

pitag-ha avatar May 30 '22 15:05 pitag-ha

Indeed, I am using ppx_deriving, but only for some utilities (#317). I'll have to see, if I can minimize what I have.

sim642 avatar May 31 '22 09:05 sim642

Removing the usage of ppx_deriving's quoter, the original error disappears, but it still doesn't seem to work right.

Instead of the above, second dune build and promote changes it to:

type t = L1.t [@@deriving_inline leq]
let _ = fun (_ : t) -> ()
let rec (leq : t -> t -> bool) = L1.leq[@@ocaml.warning "-39"]
let _ = leq
[@@@end]

Building that fails with the following diff:

----- ppx_lattice_test.ml
++++++ ppx_lattice_test.ml.ppx-corrected
File "ppx_lattice_test.ml", line 21, characters 0-1:
 |type t = L1.t [@@deriving_inline leq]
-|let _ = fun (_ : t) -> ()
 |let rec (leq : t -> t -> bool) = L1.leq[@@ocaml.warning "-39"]
-|let _ = leq
 |[@@@end]
 |end

That again wants to get rid of those let _-s, but actually manages to print the diff instead of crashing. But whatever that diff is, dune promote doesn't promote anything.

Manually removing to

type t = L1.t [@@deriving_inline leq]
let rec (leq : t -> t -> bool) = L1.leq[@@ocaml.warning "-39"]
[@@@end]

finally builds successfully and doesn't print any diff.

Therefore it seems to me that there's still some inconsistency, because initially @@deriving_inline suggests those and then suggests removing them.

The round-tripping error might be orthogonal then and due to some of that ppx_deriving quoter wrapping expression.

sim642 avatar Jun 01 '22 15:06 sim642