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

[coq] [draft] Support for coqffi #4007

Closed
wants to merge 5 commits into from
Closed

Conversation

ejgallego
Copy link
Collaborator

@ejgallego ejgallego commented Dec 4, 2020

This is a draft PR to support @lthms coqffi rules that allows for Coq to call OCaml libraries https://github.com/coq-community/coqffi . This has been written in collaboration with @lthms .

As of today, we add a (ffi_modules ...) field to (coq.theory ...) and the corresponding rules for .cmi to .v generation. This is a pretty small patch and highlights how to add a simple rule to Coq's mode.

There are some open issues and todos before we can consider this ready:

  • blocker: fix code to locate a cmi from a lib + OCaml module name. As of today we just do a hack, however it doesn't work for wrapped modules; we couldn't find the right API for it.
  • syntax: there are several options, may need to specify flags, etc... so this is still work in progress
  • documentation / changelog
  • more tests [should be disabled by default until coqffi is stable]
  • version check for (coq.theory ...)

ejgallego and others added 4 commits December 4, 2020 10:32
Co-authored-by: Thomas Letan <thomas.letan@ssi.gouv.fr>
Signed-off-by: Emilio Jesus Gallego Arias <e+git@x80.org>
Co-authored-by: Thomas Letan <thomas.letan@ssi.gouv.fr>
Signed-off-by: Emilio Jesus Gallego Arias <e+git@x80.org>
Co-authored-by: Thomas Letan <thomas.letan@ssi.gouv.fr>
Signed-off-by: Emilio Jesus Gallego Arias <e+git@x80.org>
We are missing how to access the right cmi file name.

Co-authored-by: Thomas Letan <thomas.letan@ssi.gouv.fr>
Signed-off-by: Emilio Jesus Gallego Arias <e+git@x80.org>
(* Get the path to the cmi file *)
(* We need help here! The below fails with wrong dune *)
let expand_expr = String_with_vars.make_var Loc.none ~payload:ml_lib_name "cmi" in
let cmi_path = Expander.expand_path expander expand_expr in
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work, we also tried:

  let cmi_path =
    let open Result.O in
    let* ml_lib = Lib.DB.resolve lib_db (Loc.none, Lib_name.of_string ml_lib_name) in
    let info = Lib.info ml_lib in
    let obj_dir = Obj_dir.public_cmi_dir (Lib_info.obj_dir info) in
    Result.Ok (Path.relative obj_dir (coq_module_name ^ ".cmi"))

which works but it doesn't compute the right cmi path for wrapped libs.

@ejgallego ejgallego added the coq label Dec 4, 2020
This is broken for external libs, but maybe it is not too far from the
right path.

Co-authored-by: Thomas Letan <thomas.letan@ssi.gouv.fr>
Signed-off-by: Emilio Jesus Gallego Arias <e+git@x80.org>
@ejgallego
Copy link
Collaborator Author

Updated with a fresh attempt to locate a cmi path, but unfortunately it only works for local libs.

@lthms
Copy link
Contributor

lthms commented Dec 4, 2020

Updated with a fresh attempt to locate a cmi path, but unfortunately it only works for local libs.

This is already a pretty great improvement, congrats!

@ejgallego
Copy link
Collaborator Author

ejgallego commented Dec 7, 2020

Adding @rgrinberg 's comments on how to move forward with this:

The correct way is to find the Obj_dir.t that corresponds to this library and then use it to resolve the module. You can obtain the module by name by getting the Modules.t of the library.

For external libraries, the Modules.t should be available straight fromdune-package.

What might be problematic is obtaining Modules.t for external libraries. Because I recall that we only wrote this value to dune-package for special cases. That should be easy enough to fix.

@lthms
Copy link
Contributor

lthms commented Dec 7, 2020

Thanks @ejgallego (and @rgrinberg ) for the heads-up. I have tagged a first beta release for coqffi, so I will have some time now to work on this. I hope I will be able to have a first clean version for modules from local libraries later this week!

Base automatically changed from master to main January 14, 2021 17:09
@ejgallego
Copy link
Collaborator Author

Hi @lthms , I'm closing this PR due to inactivity, but I'd love to reopen and finish it if you still find it useful.

@ejgallego ejgallego closed this Sep 8, 2021
@lthms
Copy link
Contributor

lthms commented Sep 8, 2021

Yeah, sorry about that. I’ve been focused on other things. Yet, coqffi has become way more stable now, and I think that as soon as I release the first stable version, I will give this another try.

Thanks again a lot for taking the time to guide me the other time.

@ejgallego
Copy link
Collaborator Author

Great to hear, looking forward to completing this, and to using coqffi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants