[llvm-c] Expose LLVMContextGetDiagnostic{Handler,Context}

[whitequark]

LGTM

[whitequark]

Actually, can you please add a test? The main function of llvm-c-test/echo.cpp seems to be a suitable place to set and then retrieve a diagnostic handler.

[jketema]

I was wondering what a suitable place for a test was. Thanks for the pointer, I'll update. Once this is in I should have a first version of a patch for the OCaml bindings exposing the diagnostic handler.

[jketema]

Added test for diagnostic handler

[whitequark]

deadalnix, does the test look OK to you?

[deadalnix]

Overall I'm good with it. I'm not sure test belongs in the echo thing. I would add a new flag to llvm-c-test and just create a new kind of test. That way it would be possible to actually trigger the handler and see if it does what is expected.

[jketema]

I've split out the diagnostic testing and improved it a bit as suggested. Let me know what you think.

While here, I also fixed some issues with the help text printed by llvm-c-test. I hope that's fine.

[jketema]

Remove some unnecessary duplication introduced in the previous version of this patch.

[deadalnix]

Small nits, but looks good overall.

tools/llvm-c-test/diagnostic.c

40 ↗

(On Diff #52894)

Remove empty line

tools/llvm-c-test/llvm-c-test.h

53 ↗

(On Diff #52894)

The name is a bit confusing, because this isn't the handler. Maybe llvm_test_diagnostic_handler or something alike.

[jketema]

Addressed the little nitpicks.

@whitequark Is this ok with you?

include/llvm-c/Core.h

 /**
  * Set the diagnostic handler for this context.
  */
 void LLVMContextSetDiagnosticHandler(LLVMContextRef C,
                                      LLVMDiagnosticHandler Handler,
                                      void *DiagnosticContext);
 
 /**
+ * Get the diagnostic handler of this context.
+ */
+LLVMDiagnosticHandler LLVMContextGetDiagnosticHandler(LLVMContextRef C);
+
+/**
+ * Get the diagnostic context of this context.
+ */
+void *LLVMContextGetDiagnosticContext(LLVMContextRef C);
+
+/**
  * Set the yield callback function for this context.
  *
  * @see LLVMContext::setYieldCallback()
  */
 void LLVMContextSetYieldCallback(LLVMContextRef C, LLVMYieldCallback Callback,
                                  void *OpaqueHandle);
 
 /**

lib/IR/Core.cpp

 LLVMContextRef LLVMGetGlobalContext() {
   return wrap(&getGlobalContext());
 }
 
 void LLVMContextSetDiagnosticHandler(LLVMContextRef C,
                                      LLVMDiagnosticHandler Handler,
                                      void *DiagnosticContext) {
   unwrap(C)->setDiagnosticHandler(
-      LLVM_EXTENSION reinterpret_cast<LLVMContext::DiagnosticHandlerTy>(Handler),
+      LLVM_EXTENSION reinterpret_cast<LLVMContext::DiagnosticHandlerTy>(
+          Handler),
       DiagnosticContext);
 }
 
+LLVMDiagnosticHandler LLVMContextGetDiagnosticHandler(LLVMContextRef C) {
+  return LLVM_EXTENSION reinterpret_cast<LLVMDiagnosticHandler>(
+      unwrap(C)->getDiagnosticHandler());
+}
+
+void *LLVMContextGetDiagnosticContext(LLVMContextRef C) {
+  return unwrap(C)->getDiagnosticContext();
+}
+
 void LLVMContextSetYieldCallback(LLVMContextRef C, LLVMYieldCallback Callback,
                                  void *OpaqueHandle) {
   auto YieldCallback =
     LLVM_EXTENSION reinterpret_cast<LLVMContext::YieldCallbackTy>(Callback);
   unwrap(C)->setYieldCallback(YieldCallback, OpaqueHandle);
 }
 
 void LLVMContextDispose(LLVMContextRef C) {

tools/llvm-c-test/echo.cpp

while (true) {
     LLVMValueRef Prev = LLVMGetPreviousFunction(Next);
     if (Prev != Cur)
       report_fatal_error("Next.Previous function is not Current");
 
     Cur = Next;
   }
 }
 
+static void diagnostic_handler(LLVMDiagnosticInfoRef DI, void * C) {}
+
+static int diagnostic_context = 42;
+
+static void test_diagnostic_handler(LLVMContextRef Ctx) {
+  LLVMDiagnosticHandler OldHandler = LLVMContextGetDiagnosticHandler(Ctx);
+  void *OldContext = LLVMContextGetDiagnosticContext(Ctx);
+
+  LLVMContextSetDiagnosticHandler(Ctx, diagnostic_handler, &diagnostic_context);
+
+  if (LLVMContextGetDiagnosticHandler(Ctx) != diagnostic_handler)
+    report_fatal_error("LLVMContext{Set,Get}DiagnosticHandler failed");
+
+  int *Context = (int *)LLVMContextGetDiagnosticContext(Ctx);
+  if (Context != &diagnostic_context || *Context != 42)
+    report_fatal_error("LLVMContextGetDiagnosticContext failed");
+
+  // Restore the original handler
+  LLVMContextSetDiagnosticHandler(Ctx, OldHandler, OldContext);
+}
+
 int llvm_echo(void) {
   LLVMEnablePrettyStackTrace();
 
   LLVMModuleRef Src = llvm_load_module(false, true);
   size_t Len;
   const char *ModuleName = LLVMGetModuleIdentifier(Src, &Len);
   LLVMContextRef Ctx = LLVMContextCreate();
   LLVMModuleRef M = LLVMModuleCreateWithNameInContext(ModuleName, Ctx);
 
+  test_diagnostic_handler(Ctx);
+
   // This whole switcharound is done because the C API has no way to
   // set the source_filename
   LLVMSetModuleIdentifier(M, "", 0);
   LLVMGetModuleIdentifier(M, &Len);
   if (Len != 0)
       report_fatal_error("LLVM{Set,Get}ModuleIdentifier failed");
   LLVMSetModuleIdentifier(M, ModuleName, strlen(ModuleName));