Use ocamlrep rather than ocamlvalue in rust_to_ocaml
commitb25fbcd9a01378233ebd89da3541ac43c04c865e
authorJake Bailey (Hacklang) <jakebailey@fb.com>
Sun, 27 Oct 2019 00:20:58 +0000 (26 17:20 -0700)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Sun, 27 Oct 2019 00:23:06 +0000 (26 17:23 -0700)
tree396950804b08b749374bfd440de5fe96be2708e5
parent53af5b93b799e8acb1817a0f3c734a47ef73be79
Use ocamlrep rather than ocamlvalue in rust_to_ocaml

Summary:
We currently have four methods of converting between Rust and OCaml values (that I know of):

- the `ocaml` crate, which provides a few limited wrappers around OCaml runtime allocation functions. Since it calls into the OCaml runtime, the garbage collector may run, and we must be careful to keep values rooted while using it.
- `ToOcaml`, which was implemented for passing the CST from the Rust parser to OCaml. It passes around a SerializationContext containing a pointer to the OCaml source text, since we chose not to write those pointers into positioned tokens and trivia on the Rust side, but those fields still exist on the OCaml side. It is backed by `ocamlpool`, a library which allows us to allocate values on the OCaml heap without running the garbage collector (so that we need not root our values).
- `OcamlRep`, which was originally conceived as a means of allocating OCaml values into Rust-managed memory (coupled with generation of implementations using a procedural macro). It is now (after {D17864960}) a generic interface for allocating OCaml values and deserializing OCaml values into Rust values.
- `Ocamlvalue`, which used a similar approach to OcamlRep (i.e., a trait and a procedural macro) to allocate values using `ocamlpool`.

Now that `ocamlrep` can allocate values using `ocamlpool` (after {D17864957}), I think it would be nice to bring this number from four down to three. Replacing the Ocamlvalue trait with OcamlRep will also allow us to use types which implement OcamlRep in Rust binaries. This was difficult before because implementations of Ocamlvalue linked against the OCaml runtime (via `ocamlpool`).

This diff replaces `Ocamlvalue` with `OcamlRep` in the `rust_to_ocaml` module.

The `ocamlrep_ocamlpool` library models the ocamlpool section (i.e., the region of code between invocations of `ocamlpool_enter()` and `ocamlpool_leave()`) using a `Pool` struct (which invokes `ocamlpool_enter()` when created and `ocamlpool_leave()` when dropped). A `Pool` doesn't play very nicely with `SerializationContext`, which does not currently borrow anything, and is borrowed immutably in `ToOcaml`--to put a Pool in, we'd need to parameterize SerializationContext with the lifetime of the Pool (and possibly also the lifetime of the context's mutable borrow of the pool) and change `ToOcaml` to borrow the context mutably.

That all seems like a needlessly involved refactoring. I think it's reasonable to provide an (unsafe) API for going around the `Pool` abstraction. If this API is used incorrectly, a segfault will result (the ocamlpool allocation functions will return a null pointer), but this was already true for `ToOcaml` anyway. This diff adds such an API in `add_to_ambient_pool`.

Reviewed By: losvald

Differential Revision: D18076617

fbshipit-source-id: 4a0eeb504119eb63dedd2eb764fc0954bae50f01
hphp/hack/Cargo.lock
hphp/hack/src/generate_full_fidelity.ml
hphp/hack/src/ocamlrep_ocamlpool/lib.rs
hphp/hack/src/parser/cargo/rust_to_ocaml/Cargo.toml
hphp/hack/src/parser/minimal_syntax.rs
hphp/hack/src/parser/minimal_token.rs
hphp/hack/src/parser/rust_to_ocaml.rs
hphp/hack/src/parser/smart_constructors.rs
hphp/hack/src/parser/syntax_error.rs
hphp/hack/src/parser/syntax_kind.rs
hphp/hack/src/utils/ocaml_ffi_mock/ocaml.c