search

yallop / ocaml-ctypes

Library for binding to C libraries using pure OCaml
MIT License
369 stars 95 forks source link

Fix warning 9 [missing-record-field-pattern] in generated OCaml code #700

Closed MisterDA closed 2 years ago

MisterDA commented 2 years ago

Example problem:

File "lib/bindings_stubs_field_info.ml", line 17, characters 9-15:
17 |   | View { ty } -> seal ty
              ^^^^^^
Error (warning 9 [missing-record-field-pattern]): the following labels are not bound in this record pattern:
read, write, format_typ, format
Either bind these labels explicitly or add '; _' to the pattern.
yallop commented 2 years ago

Thank you for the contribution, @MisterDA!

MisterDA commented 2 years ago

Emergency bugfix release? :angel:

yallop commented 2 years ago

It'll go in the next release. It's not a bug, because warning 9 is off by default.

MisterDA commented 2 years ago

It's not a bug, because warning 9 is off by default.

I've encountered the error fiddling with ocaml-gobject-introspection. I'm using setup-ocaml in GitHub Actions for the CI. The project uses dune, and setup-ocaml builds it in debug mode, where dune turns warnings into errors. The generated code has this warning, and when dune builds it, dune emits an error failing the build. This is the same scenario than in my developer workflow if I don't pin-depends ocaml-ctypes with this patch or build the project in release-mode. Even if one could argue that this is a problem of dune (and iirc it has been argued) it does mean that the project I'm working on cannot be built "out-of-the-box". I think there's a case for not generating code that has compilation warnings.

Leonidas-from-XIV commented 2 years ago

I ran into the exact same case when building with dune locally and while searching whether this has been reported I stumbled over this PR. I agree with @MisterDA here - while the dune defaults might be debatable, there is definitely a point for generating code that doesn't trigger warnings, which helps to be confident that the stub generator generates the right code and does not accidentally miss to generate stubs for something.

It would be nice to have a point release, especially since some fixes were merged and also OCaml 4.14 came out which is now tested in CI.

yallop commented 2 years ago

Okay, I'm convinced. There's a new release on its way: https://github.com/ocaml/opam-repository/pull/21068