bs.optional + bs.deriving abstract shows warning 69

[jchavarri]

@anmonteiro I ran into this issue and thought on sharing it in case it can be fixed on Melange side.

As Melange in Dune is using OCaml byte default flags, and warning 69 was introduced in 4.13.0, these warnings that did not happen with ReScript are raised now on Melange.

Maybe the warning could be silenced at Melange ppx level? In bs.optional attrs?

[anmonteiro]

I'm tempted not to fix this one. It aligns with dune, generally.

I tried this in native, and for the following dune file and scratch.ml file, this is the output:

;; dune
(executable
 (name scratch))


;; scratch.ml

type config = { leading : bool; trailing : bool }

$ dune build ./scratch.exe
    ocamldep .scratch.eobjs/scratch.ml.d
    ocamlopt .scratch.eobjs/native/dune__exe__Scratch.{cmx,o} (exit 2)
File "scratch.ml", line 1, characters 0-49:
1 | type config = { leading : bool; trailing : bool }
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error (warning 34 [unused-type-declaration]): unused type config.
File "scratch.ml", line 1, characters 16-31:
1 | type config = { leading : bool; trailing : bool }
                    ^^^^^^^^^^^^^^^
Error (warning 69 [unused-field]): unused record field leading.
File "scratch.ml", line 1, characters 32-47:
1 | type config = { leading : bool; trailing : bool }
                                    ^^^^^^^^^^^^^^^
Error (warning 69 [unused-field]): unused record field trailing.

[anmonteiro]

There are 2 solutions downstream:

1. actually use your record fields. Or delete the unused ones
2. add (flags :standard -w -69) to your library stanza

Any reason preventing you from doing 1. or 2.?

[jchavarri]

I am unsure the issue can be solved by actually using the fields, I see it more related to the way bs.deriving abstract is generating code. I refined the test in fcc6998 to show what I mean, you can see how defining a regular record does not trigger any 69 warning.

Maybe users can fix the problem by following your recommendations, but I think deriving abstract is still useful in some cases (like with optional fields), especially now that is confirmed that Melange will not include experimental optional record fields. And it might be a bit annoying to silence warnings every time it is used. Wdyt?

[anmonteiro]

Thanks for clarifying the test and the difference when [@@bs.deriving abstract] is present. Let me look at it again.

[anmonteiro]

This is, in fact, a problem related to code generated by bs.deriving abstract. Fix incoming.

[jchavarri]

Thanks!