[PATCH] D18891: [OCaml] Expose the LLVM diagnostic handler

[whitequark via llvm-commits]

whitequark added inline comments.

================
Comment at: bindings/ocaml/llvm/llvm.ml:324
@@ +323,3 @@
+
+  external get_description : t -> string = "llvm_get_diagnostic_description"
+  external get_severity : t -> DiagnosticSeverity.t
----------------
Just `description` and `severity`.

================
Comment at: bindings/ocaml/llvm/llvm.mli:428
@@ +427,3 @@
+val set_diagnostic_handler
+  : llcontext -> (DiagnosticInfo.t -> 'a -> unit) -> 'a -> unit
+
----------------
There is no need to have an `'a` here since OCaml already has closures; the `context` argument in C is closure emulation in essence.

================
Comment at: bindings/ocaml/llvm/llvm.mli:431
@@ +430,3 @@
+(** [unset_diagnostic_handler c] unsets the diagnostic handler of [c]. *)
+val unset_diagnostic_handler : llcontext -> unit
+
----------------
Accept an option in `set_diagnostic_handler` instead of using two functions.

================
Comment at: bindings/ocaml/llvm/llvm_ocaml.c:187
@@ +186,3 @@
+  if (wrapper == NULL) {
+    llvm_raise(*caml_named_value("Llvm.MemoryError"),
+               LLVMCreateMessage(
----------------
There is `caml_raise_out_of_memory` and I think we should use it.

================
Comment at: test/Bindings/OCaml/linker.ml:21
@@ +20,3 @@
+  Printf.printf
+    "Diagnostic handler called: %s\n" (Llvm.DiagnosticInfo.get_description di)
+
----------------
Is this (and elsewhere in `test/`) actually used in tests?


http://reviews.llvm.org/D18891