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

[whitequark via llvm-commits]

whitequark added inline comments.

================
Comment at: bindings/ocaml/llvm/llvm.ml:317
@@ +316,3 @@
+(*===-- Context error handling --------------------------------------------===*)
+module DiagnosticInfo = struct
+  type t
----------------
Just `module Diagnostic`, I think.

================
Comment at: bindings/ocaml/llvm/llvm_ocaml.c:135
@@ +134,3 @@
+                         sizeof(LLVMDiagnosticInfoRef), 0, 1);
+  DiagnosticInfo_val(V) = DI;
+  return V;
----------------
You don't need all this ceremony. LLVMDiagnosticInfoRef is a `void*` and so is a `value` and OCaml's runtime is defined to not do anything special with pointers that it doesn't control. So just cast `LLVMDiagnosticInfoRef` to `value`.

================
Comment at: bindings/ocaml/llvm/llvm_ocaml.c:178
@@ +177,3 @@
+/* llcontext -> (DiagnosticInfo.t -> unit) -> unit */
+CAMLprim value llvm_set_diagnostic_handler(LLVMContextRef C, value Handler) {
+  llvm_remove_diagnostic_handler(C);
----------------
I would merge these two functions into just `llvm_set_diagnostic_handler`; there are a few examples of how to handle incoming `option` values in this codebase.

================
Comment at: test/Bindings/OCaml/diagnostic_handler.ml:31
@@ +30,3 @@
+      let m = begin try
+        (* CHECK: Diagnostic handler called
+         *)
----------------
No check for the proper message and severity? This can hide a bug where the severity or message are mishandled.


http://reviews.llvm.org/D18891