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

[sim642]

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.

[panglesd]

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!

[pitag-ha]

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).

[sim642]

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]

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.

[mbarbin]

I have been experimenting with @@deriving_inline today, with other ppx than mentioned here, and been hitting this "doesn't round-trip" issue too. Except in my case, there's no flickering in the generation. After I fmt the code, the error goes away sometimes.

However I haven't managed to reproduce the issue clearly. This is probably hard to test if this is specific to dune running in watch mode. If there's interest I could try to spending more time on this. For now I just wanted to mention that there are still some issues in this area with recent versions of ppxlib (this is with 0.33).

[NathanReb]

I'll have to get a bit more familiar with the original error but if you manage to get a reproducible example I'll happily look into it!

[mbarbin]

Thanks a lot @NathanReb for offering to help!

I added a new commit to the Vcs PR where I discuss some related deriving_inline topics. I wrote down the detailed steps of what I do and the errors that I get. It's this commit: a02920b5a13c487e932e104256ffecbe2fe2ebfa Permalink

If you try going through the same steps and let me know what this does for you, with a bit of luck, you might get similar errors!

[NathanReb]

Sorry for the long wait, I was on vacation. I'll look into this shortly!

[NathanReb]

I managed to reproduce it locally, I'll try to see what's going on!

[NathanReb]

I tried it with the latest ppxlib and I'm getting different output from @all and @lint builds which can't be reconciled but no more code does not roundtrip anymore.

I'll look into it.

[NathanReb]

This is due to the quoter returning different IDs in one case and the other. The lint build only generates the deriving_inline code and therefore gets different IDs than the regular builds that also run the rest of the PPX-es.

I need to refresh my memory on how deriving_inline is supposed to work but it's a little odd that it's ran buy the regular builds AND the lint. I would expect it to only be ran by the lint builds so that it's not a dependency of the regular builds anymore.

[NathanReb]

Circling back to the quoter issue, I think it can easily be solved by adding the entire set of ppx-es to the (lint) field as well as then the IDs allocated by the quoter would match. I'll have to give this a try just to ensure that there are no more "no round-trip" errors.

That being said, I agree this won't solve your main issue which is that you can't properly use [@@deriving_inline ...] to remove a build dependency on a subset of your ppx-es. For that I'm currently working on adding a flag!

[mbarbin]

Circling back to the quoter issue, I think it can easily be solved by adding the entire set of ppx-es to the (lint) field as well as then the IDs allocated by the quoter would match.

You are talking about something inside the ppxlib implementation, or a change for the user to do in the dune config files?

you can't properly use [@@deriving_inline ...] to remove a build dependency on a subset of your ppx-es. For that I'm currently working on adding a flag!

I'm quite happy with a flag, and can justify allocating time for the testing of it if this helps down the road. Thanks a lot!