[PATCH] D65070: [LLVM-C][OCaml] Add a fast linker binding

Sat Jul 27 05:35:39 PDT 2019

whitequark added inline comments.

================
Comment at: bindings/ocaml/linker/linker_ocaml.c:40
+
+static value *internalize_trampoline_callback_value = NULL;
+
----------------
kren1 wrote:
> whitequark wrote:
> > I think the proliferation of non-reentrant/non-threadsafe functions in the API is an issue. (Right now OCaml has a global lock, but a multicore runtime is in works.) The approach in D65138 is better, but still seems error-prone to me; I'll need to take a closer look at the OCaml runtime feature to see if perhaps a better one is possible.
> I can't do the D65138 approach, because the callback doesn't pass a context, but it would be easier in this case because there is no need to worry about life times.
I would personally consider any C API function that does not take a context a (usability) bug, since the LLVM C API is primarily useful for writing bindings from other languages, most of which have some kind of closure mechanism. I think in this case it is clearly justifiable to add an `LLVMLinkInModule2` that fixes this.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65070/new/

https://reviews.llvm.org/D65070