Always pass a diagnostic handler to the linker.

Before this patch the diagnostic handler was optional. If it was not
passed, the one in the LLVMContext was used.

That is probably not a pattern we want to follow. If each area has an
optional callback, there is a sea of callbacks and it is hard to follow
which one is called.

Doing this also found cases where the callback is a nice addition, like
testing that no errors or warnings are reported.

The other option is to always use the diagnostic handler in the
LLVMContext. That has a few problems

* To implement the C API we would have to set the diag handler and then
  set it back to the original value.
* Code that creates the context might be far away from code that wants
  the diagnostics.

I do have a patch that implements the second option and will send that as
an RFC.

llvm-svn: 254777

llvm/include/llvm/Linker/Linker.h

@@ -69,7 +69,6 @@ public:
   };
 
   Linker(Module &M, DiagnosticHandlerFunction DiagnosticHandler);
-  Linker(Module &M);
 
   /// \brief Link \p Src into the composite. The source is destroyed.
   ///
@@ -88,8 +87,9 @@ public:
                           DiagnosticHandlerFunction DiagnosticHandler,
                           unsigned Flags = Flags::None);
 
-  static bool linkModules(Module &Dest, Module &Src,
-                          unsigned Flags = Flags::None);
+  DiagnosticHandlerFunction getDiagnosticHandler() const {
+    return DiagnosticHandler;
+  }
 
 private:
   Module &Composite;

llvm/lib/LTO/LTOCodeGenerator.cpp

@@ -65,9 +65,10 @@ const char* LTOCodeGenerator::getVersionString() {
 }
 
 LTOCodeGenerator::LTOCodeGenerator(LLVMContext &Context)
-    : Context(Context),
-      MergedModule(new Module("ld-temp.o", Context)),
-      IRLinker(new Linker(*MergedModule)) {
+    : Context(Context), MergedModule(new Module("ld-temp.o", Context)),
+      IRLinker(new Linker(*MergedModule, [this](const DiagnosticInfo &DI) {
+        MergedModule->getContext().diagnose(DI);
+      })) {
   initializeLTOPasses();
 }
 
@@ -123,7 +124,8 @@ void LTOCodeGenerator::setModule(std::unique_ptr<LTOModule> Mod) {
   AsmUndefinedRefs.clear();
 
   MergedModule = Mod->takeModule();
-  IRLinker = make_unique<Linker>(*MergedModule);
+  IRLinker =
+      make_unique<Linker>(*MergedModule, IRLinker->getDiagnosticHandler());
 
   const std::vector<const char*> &Undefs = Mod->getAsmUndefinedRefs();
   for (int I = 0, E = Undefs.size(); I != E; ++I)

llvm/lib/Linker/LinkModules.cpp

@@ -2030,11 +2030,6 @@ Linker::Linker(Module &M, DiagnosticHandlerFunction DiagnosticHandler)
   }
 }
 
-Linker::Linker(Module &M)
-    : Linker(M, [this](const DiagnosticInfo &DI) {
-        Composite.getContext().diagnose(DI);
-      }) {}
-
 bool Linker::linkInModule(Module &Src, unsigned Flags,
                           const FunctionInfoIndex *Index,
                           DenseSet<const GlobalValue *> *FunctionsToImport) {
@@ -2061,11 +2056,6 @@ bool Linker::linkModules(Module &Dest, Module &Src,
   return L.linkInModule(Src, Flags);
 }
 
-bool Linker::linkModules(Module &Dest, Module &Src, unsigned Flags) {
-  Linker L(Dest);
-  return L.linkInModule(Src, Flags);
-}
-
 //===----------------------------------------------------------------------===//
 // C API.
 //===----------------------------------------------------------------------===//

llvm/tools/bugpoint/BugDriver.cpp

@@ -15,6 +15,7 @@
 
 #include "BugDriver.h"
 #include "ToolRunner.h"
+#include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/IRReader/IRReader.h"
@@ -112,6 +113,12 @@ std::unique_ptr<Module> llvm::parseInputFile(StringRef Filename,
   return Result;
 }
 
+static void diagnosticHandler(const DiagnosticInfo &DI) {
+  DiagnosticPrinterRawOStream DP(errs());
+  DI.print(DP);
+  errs() << '\n';
+}
+
 // This method takes the specified list of LLVM input files, attempts to load
 // them, either as assembly or bitcode, then link them together. It returns
 // true on failure (if, for example, an input bitcode file could not be
@@ -132,7 +139,7 @@ bool BugDriver::addSources(const std::vector<std::string> &Filenames) {
     if (!M.get()) return true;
 
     outs() << "Linking in input file: '" << Filenames[i] << "'\n";
-    if (Linker::linkModules(*Program, *M))
+    if (Linker::linkModules(*Program, *M, diagnosticHandler))
       return true;
   }
 

llvm/tools/bugpoint/Miscompilation.cpp

@@ -18,6 +18,7 @@
 #include "llvm/Config/config.h"   // for HAVE_LINK_R
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
@@ -207,6 +208,14 @@ namespace {
   };
 }
 
+static void diagnosticHandler(const DiagnosticInfo &DI) {
+  DiagnosticPrinterRawOStream DP(errs());
+  DI.print(DP);
+  errs() << '\n';
+  if (DI.getSeverity() == DS_Error)
+    exit(1);
+}
+
 /// TestMergedProgram - Given two modules, link them together and run the
 /// program, checking to see if the program matches the diff. If there is
 /// an error, return NULL. If not, return the merged module. The Broken argument
@@ -222,7 +231,7 @@ static Module *TestMergedProgram(const BugDriver &BD, Module *M1, Module *M2,
     M1 = CloneModule(M1);
     M2 = CloneModule(M2);
   }
-  if (Linker::linkModules(*M1, *M2))
+  if (Linker::linkModules(*M1, *M2, diagnosticHandler))
     exit(1);
   delete M2;   // We are done with this module.
 
@@ -390,7 +399,8 @@ static bool ExtractLoops(BugDriver &BD,
         MisCompFunctions.emplace_back(F->getName(), F->getFunctionType());
       }
 
-      if (Linker::linkModules(*ToNotOptimize, *ToOptimizeLoopExtracted))
+      if (Linker::linkModules(*ToNotOptimize, *ToOptimizeLoopExtracted,
+                              diagnosticHandler))
         exit(1);
 
       MiscompiledFunctions.clear();
@@ -418,7 +428,8 @@ static bool ExtractLoops(BugDriver &BD,
     // extraction both didn't break the program, and didn't mask the problem.
     // Replace the current program with the loop extracted version, and try to
     // extract another loop.
-    if (Linker::linkModules(*ToNotOptimize, *ToOptimizeLoopExtracted))
+    if (Linker::linkModules(*ToNotOptimize, *ToOptimizeLoopExtracted,
+                            diagnosticHandler))
       exit(1);
 
     delete ToOptimizeLoopExtracted;
@@ -594,7 +605,7 @@ static bool ExtractBlocks(BugDriver &BD,
     if (!I->isDeclaration())
       MisCompFunctions.emplace_back(I->getName(), I->getFunctionType());
 
-  if (Linker::linkModules(*ProgClone, *Extracted))
+  if (Linker::linkModules(*ProgClone, *Extracted, diagnosticHandler))
     exit(1);
 
   // Set the new program and delete the old one.

llvm/tools/gold/gold-plugin.cpp

@@ -938,7 +938,7 @@ static ld_plugin_status allSymbolsReadHook(raw_fd_ostream *ApiFile) {
   }
 
   std::unique_ptr<Module> Combined(new Module("ld-temp.o", Context));
-  Linker L(*Combined);
+  Linker L(*Combined, diagnosticHandler);
 
   std::string DefaultTriple = sys::getDefaultTargetTriple();
 

llvm/unittests/Linker/LinkModulesTest.cpp

@@ -71,6 +71,8 @@ protected:
   BasicBlock *ExitBB;
 };
 
+static void expectNoDiags(const DiagnosticInfo &DI) { EXPECT_TRUE(false); }
+
 TEST_F(LinkModuleTest, BlockAddress) {
   IRBuilder<> Builder(EntryBB);
 
@@ -93,7 +95,7 @@ TEST_F(LinkModuleTest, BlockAddress) {
   Builder.CreateRet(ConstantPointerNull::get(Type::getInt8PtrTy(Ctx)));
 
   Module *LinkedModule = new Module("MyModuleLinked", Ctx);
-  Linker::linkModules(*LinkedModule, *M);
+  Linker::linkModules(*LinkedModule, *M, expectNoDiags);
 
   // Delete the original module.
   M.reset();
@@ -169,13 +171,13 @@ static Module *getInternal(LLVMContext &Ctx) {
 TEST_F(LinkModuleTest, EmptyModule) {
   std::unique_ptr<Module> InternalM(getInternal(Ctx));
   std::unique_ptr<Module> EmptyM(new Module("EmptyModule1", Ctx));
-  Linker::linkModules(*EmptyM, *InternalM);
+  Linker::linkModules(*EmptyM, *InternalM, expectNoDiags);
 }
 
 TEST_F(LinkModuleTest, EmptyModule2) {
   std::unique_ptr<Module> InternalM(getInternal(Ctx));
   std::unique_ptr<Module> EmptyM(new Module("EmptyModule1", Ctx));
-  Linker::linkModules(*InternalM, *EmptyM);
+  Linker::linkModules(*InternalM, *EmptyM, expectNoDiags);
 }
 
 TEST_F(LinkModuleTest, TypeMerge) {