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

[Robert Widmann via Phabricator via llvm-commits]

CodaFi added inline comments.


================
Comment at: lib/Linker/LinkModules.cpp:617
+  std::unique_ptr<Module> M(unwrap(Src));
+  return L->linkInModule(std::move(M), Linker::Flags::None, {});
+}
----------------
kren1 wrote:
> CodaFi wrote:
> > Can you expose these two defaulted arguments in the bindings as well?  
> Exposing them is not trivial, because I would have to introduce new types for Flag and a trampoline for the callback. I removed them to be implicit and in line with `LLVMLinkModules2`. If you really want me to add them, I would prefer to do it as a separate diff.
I realize it’s non-trivial, but our commitment to ABI compatibility makes revising these public interfaces much more difficult after the fact than getting it right up front.  

FWIW, the result of the internalize bindings patch you have apply here as well, and you need only provide a thunk to translate the C binding flags to the few linker flags LLVM currently supports.  If you’d like, I can generate that binding and tag you in a review, then you can rebase the OCaml bindings on top.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65070