[RFC] Always using the diagnostic handler in the LLVMContext

[Mehdi Amini via llvm-commits]

> On Dec 10, 2015, at 8:06 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
> 
> On 4 December 2015 at 18:31, Mehdi Amini <mehdi.amini at apple.com> wrote:
>> 
>>> On Dec 4, 2015, at 2:39 PM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>>> 
>>> The attached patch converts code that has access to a LLVMContext to
>>> not take a diagnostic handler.
>>> 
>>> This has a few advantages
>>> 
>>> * It is easier to use a consistent diagnostic handler in a single program.
>>> * Less clutter since we are not passing a handler around.
>> 
>> I like it :)
>> 
>>> 
>>> It also has a few problems:
>>> 
>>> * The code that creates the context may be far from the code that
>>> wants no see the diagnostic and we probably don't want to have a lot
>>> of places changing the diagnostic handler in flight.
>> 
>> I only see three places where you needed to call setDiagnosticHandler() on the Context, two of them in the C API to support “soon deprecated” API ;)
>> and the third one in llvm-link.cpp:main() which seems good the me.
>> 
>> Do you had other places where it was not nice?
> 
> I was afraid of the use in clang, but it was not too bad in the end.
> 
> Updated patches attached.
> 
> Cheers,
> Rafael
> <llvm.diff><clang.diff>


@@ -77,6 +59,8 @@ static void linkerDiagnosticHandler(const DiagnosticInfo &DI,
     SmallVector<std::pair<unsigned, std::unique_ptr<llvm::Module>>, 4>
         LinkModules;
 
+    llvm::Module *CurLinkModule = nullptr;
+


This is not really nice, but probably unavoidable, at least document.


@@ -579,6 +559,12 @@ void BackendConsumer::DiagnosticHandlerImpl(const DiagnosticInfo &DI) {
       return;
     ComputeDiagID(Severity, backend_frame_larger_than, DiagID);
     break;
+  case DK_Linker:


assert(CurLinkModule)




@@ -35,17 +42,19 @@ LLVMBool LLVMParseBitcodeInContext(LLVMContextRef ContextRef,
   MemoryBufferRef Buf = unwrap(MemBuf)->getMemBufferRef();
   LLVMContext &Ctx = *unwrap(ContextRef);
 
+  LLVMContext::DiagnosticHandlerTy OldDiagnosticHandler =
+      Ctx.getDiagnosticHandler();
+  void *OldDiagnosticContext = Ctx.getDiagnosticContext();
   std::string Message;
-  raw_string_ostream Stream(Message);
-  DiagnosticPrinterRawOStream DP(Stream);
+  Ctx.setDiagnosticHandler(diagnosticHandler, &Message, true);
+
+  ErrorOr<std::unique_ptr<Module>> ModuleOrErr = parseBitcodeFile(Buf, Ctx);
+
+  Ctx.setDiagnosticHandler(OldDiagnosticHandler, OldDiagnosticContext, true);



If it is something that needs to be done in other places, some RAII handler would be nice, if it is the only place, OK.


--- a/unittests/Linker/LinkModulesTest.cpp
+++ b/unittests/Linker/LinkModulesTest.cpp
@@ -71,8 +71,6 @@ protected:
   BasicBlock *ExitBB;
 };
 
-static void expectNoDiags(const DiagnosticInfo &DI) { EXPECT_TRUE(false); }
-


Is there the same one already present on the Context?


— 
Mehdi