Merge DarwinLdDriver and Driver.

[ruiu]

Summary

Now that DarwinLdDriver is the only derived class of Driver.
This patch merges them and actually removed the class because
they can now just be non-member functions. This change simplifies
a common header, Driver.h.

[silvas]

lib/Driver/DarwinLdDriver.cpp

1260

drive-by nit: closing namespace comments

[ruiu]

• Address Sean's comment

lib/Driver/DarwinLdDriver.cpp

1260

Done.

[lhames]

LGTM.

As a side note: we plan to experiment with new Atom-based formats and drivers in the future, so any assumption that Atoms == MachO should be generally be avoided in refactors. That said, this looks like generically good clean-up.

[ruiu]

Thank you for reviewing!

[ruiu]

Submitted as r262502.

COFF/Driver.h

 class LinkerDriver;
 extern LinkerDriver *Driver;
 
 using llvm::COFF::MachineTypes;
 using llvm::COFF::WindowsSubsystem;
 using llvm::Optional;
 class InputFile;
 
-// Entry point of the COFF linker.
-bool link(llvm::ArrayRef<const char *> Args);
-
 // Implemented in MarkLive.cpp.
 void markLive(const std::vector<Chunk *> &Chunks);
 
 // Implemented in ICF.cpp.
 void doICF(const std::vector<Chunk *> &Chunks);
 
 class ArgParser {
 public:

COFF/Driver.cpp

 //===- Driver.cpp ---------------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Config.h"
 #include "Driver.h"
 #include "Error.h"
 #include "InputFiles.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "Writer.h"
+#include "lld/Driver/Driver.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/LibDriver/LibDriver.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"

ELF/Driver.h

 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace lld {
 namespace elf {
 
 extern class LinkerDriver *Driver;
 
-// Entry point of the ELF linker. Returns true on success.
-bool link(ArrayRef<const char *> Args, llvm::raw_ostream &Error = llvm::errs());
-
 class LinkerDriver {
 public:
   void main(ArrayRef<const char *> Args);
   void addFile(StringRef Path);
   void addLibrary(StringRef Name);
 
 private:
   void readConfigs(llvm::opt::InputArgList &Args);

ELF/Driver.cpp

 #include "Config.h"
 #include "Error.h"
 #include "ICF.h"
 #include "InputFiles.h"
 #include "LinkerScript.h"
 #include "SymbolTable.h"
 #include "Target.h"
 #include "Writer.h"
+#include "lld/Driver/Driver.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include <utility>
 
 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::object;

include/lld/Driver/Driver.h

 //===- lld/Driver/Driver.h - Linker Driver Emulator -----------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
-///
-/// \file
-///
-/// Interface for Drivers which convert command line arguments into
-/// LinkingContext objects, then perform the link.
-///
-//===----------------------------------------------------------------------===//
 
 #ifndef LLD_DRIVER_DRIVER_H
 #define LLD_DRIVER_DRIVER_H
 
-#include "lld/Core/LLVM.h"
-#include "lld/Core/Node.h"
-#include "llvm/ADT/Triple.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/raw_ostream.h"
-#include <memory>
-#include <set>
-#include <vector>
 
 namespace lld {
-class LinkingContext;
-class MachOLinkingContext;
-
-/// Base class for all Drivers.
-class Driver {
-protected:
-
-  /// Performs link using specified options
-  static bool link(LinkingContext &context,
-                   raw_ostream &diag = llvm::errs());
-
-  /// Parses the LLVM options from the context.
-  static void parseLLVMOptions(const LinkingContext &context);
-
-private:
-  Driver() = delete;
-};
-
-/// Driver for darwin/ld64 'ld' command line options.
-class DarwinLdDriver : public Driver {
-public:
-  /// Parses command line arguments same as darwin's ld and performs link.
-  /// Returns true iff there was an error.
-  static bool linkMachO(llvm::ArrayRef<const char *> args,
-                        raw_ostream &diag = llvm::errs());
-
-  /// Uses darwin style ld command line options to update LinkingContext object.
-  /// Returns true iff there was an error.
-  static bool parse(llvm::ArrayRef<const char *> args,
-                    MachOLinkingContext &info,
-                    raw_ostream &diag = llvm::errs());
-
-private:
-  DarwinLdDriver() = delete;
-};
-
-/// Driver for Windows 'link.exe' command line options
 namespace coff {
-bool link(llvm::ArrayRef<const char *> args);
+bool link(llvm::ArrayRef<const char *> Args);
 }
 
 namespace elf {
-bool link(llvm::ArrayRef<const char *> args, raw_ostream &diag = llvm::errs());
+bool link(llvm::ArrayRef<const char *> Args,
+          llvm::raw_ostream &Diag = llvm::errs());
 }
 
-} // end namespace lld
+namespace mach_o {
+bool link(llvm::ArrayRef<const char *> Args,
+          llvm::raw_ostream &Diag = llvm::errs());
+}
+}
 
 #endif

lib/Driver/CMakeLists.txt

 set(LLVM_TARGET_DEFINITIONS DarwinLdOptions.td)
 tablegen(LLVM DarwinLdOptions.inc -gen-opt-parser-defs)
 add_public_tablegen_target(DriverOptionsTableGen)
 
 add_lld_library(lldDriver
   DarwinLdDriver.cpp
-  Driver.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLD_INCLUDE_DIR}/lld/Driver
 
   LINK_LIBS
     lldConfig
     lldMachO
     lldCOFF

lib/Driver/DarwinLdDriver.cpp

 //===- lib/Driver/DarwinLdDriver.cpp --------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 ///
 /// Concrete instance of the Driver for darwin's ld.
 ///
 //===----------------------------------------------------------------------===//
 
-#include "lld/Core/File.h"
 #include "lld/Core/ArchiveLibraryFile.h"
+#include "lld/Core/File.h"
+#include "lld/Core/Instrumentation.h"
+#include "lld/Core/PassManager.h"
+#include "lld/Core/Resolver.h"
 #include "lld/Core/SharedLibraryFile.h"
 #include "lld/Driver/Driver.h"
 #include "lld/ReaderWriter/MachOLinkingContext.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Option/Arg.h"

 
 /// Parse number assuming it is base 16, but allow 0x prefix.
 static bool parseNumberBase16(StringRef numStr, uint64_t &baseAddress) {
   if (numStr.startswith_lower("0x"))
     numStr = numStr.drop_front(2);
   return numStr.getAsInteger(16, baseAddress);
 }
 
+static void parseLLVMOptions(const LinkingContext &ctx) {
+  // Honor -mllvm
+  if (!ctx.llvmOptions().empty()) {
+    unsigned numArgs = ctx.llvmOptions().size();
+    auto **args = new const char *[numArgs + 2];
+    args[0] = "lld (LLVM option parsing)";
+    for (unsigned i = 0; i != numArgs; ++i)
+      args[i + 1] = ctx.llvmOptions()[i];
+    args[numArgs + 1] = nullptr;
+    llvm::cl::ParseCommandLineOptions(numArgs + 1, args);
+  }
+}
+
 namespace lld {
+namespace mach_o {
 
-bool DarwinLdDriver::linkMachO(llvm::ArrayRef<const char *> args,
-                               raw_ostream &diagnostics) {
-  MachOLinkingContext ctx;
-  if (!parse(args, ctx, diagnostics))
-    return false;
-  if (ctx.doNothing())
-    return true;
-  return link(ctx, diagnostics);
-}
-
-bool DarwinLdDriver::parse(llvm::ArrayRef<const char *> args,
-                           MachOLinkingContext &ctx, raw_ostream &diagnostics) {
+bool parse(llvm::ArrayRef<const char *> args, MachOLinkingContext &ctx,
   // Parse command line options using DarwinLdOptions.td
   DarwinLdOptTable table;
   unsigned missingIndex;
   unsigned missingCount;
   llvm::opt::InputArgList parsedArgs =
       table.ParseArgs(args.slice(1), missingIndex, missingCount);
   if (missingCount) {
     diagnostics << "error: missing arg value for '"
if (ctx.getNodes().empty()) {
     diagnostics << "No input files\n";
     return false;
   }
 
   // Validate the combination of options used.
   return ctx.validate(diagnostics);
 }
 
+/// This is where the link is actually performed.
+bool link(llvm::ArrayRef<const char *> args, raw_ostream &diagnostics) {
+  MachOLinkingContext ctx;
+  if (!parse(args, ctx, diagnostics))
+    return false;
+  if (ctx.doNothing())
+    return true;
+  if (ctx.getNodes().empty())
+    return false;
 
+  for (std::unique_ptr<Node> &ie : ctx.getNodes())
+    if (FileNode *node = dyn_cast<FileNode>(ie.get()))
+      node->getFile()->parse();
+
+  std::vector<std::unique_ptr<File>> internalFiles;
+  ctx.createInternalFiles(internalFiles);
+  for (auto i = internalFiles.rbegin(), e = internalFiles.rend(); i != e; ++i) {
+    auto &members = ctx.getNodes();
+    members.insert(members.begin(), llvm::make_unique<FileNode>(std::move(*i)));
+  }
+
+  // Give target a chance to add files.
+  std::vector<std::unique_ptr<File>> implicitFiles;
+  ctx.createImplicitFiles(implicitFiles);
+  for (auto i = implicitFiles.rbegin(), e = implicitFiles.rend(); i != e; ++i) {
+    auto &members = ctx.getNodes();
+    members.insert(members.begin(), llvm::make_unique<FileNode>(std::move(*i)));
+  }
+
+  // Give target a chance to postprocess input files.
+  // Mach-O uses this chance to move all object files before library files.
+  ctx.finalizeInputFiles();
+
+  // Do core linking.
+  ScopedTask resolveTask(getDefaultDomain(), "Resolve");
+  Resolver resolver(ctx);
+  if (!resolver.resolve())
+    return false;
+  std::unique_ptr<SimpleFile> merged = resolver.resultFile();
+  resolveTask.end();
+
+  // Run passes on linked atoms.
+  ScopedTask passTask(getDefaultDomain(), "Passes");
+  PassManager pm;
+  ctx.addPasses(pm);
+  if (std::error_code ec = pm.runOnFile(*merged)) {
+    diagnostics << "Failed to write file '" << ctx.outputPath()
+                << "': " << ec.message() << "\n";
+    return false;
+  }
+
+  passTask.end();
+
+  // Give linked atoms to Writer to generate output file.
+  ScopedTask writeTask(getDefaultDomain(), "Write");
+  if (std::error_code ec = ctx.writeFile(*merged)) {
+    diagnostics << "Failed to write file '" << ctx.outputPath()
+                << "': " << ec.message() << "\n";
+    return false;
+  }
+
+  return true;
+}
+} // namespace mach_o
 } // namespace lld

lib/Driver/Driver.cpp

-//===- lib/Driver/Driver.cpp - Linker Driver Emulator -----------*- C++ -*-===//
-//
-//                             The LLVM Linker
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "lld/Core/ArchiveLibraryFile.h"
-#include "lld/Core/File.h"
-#include "lld/Core/Instrumentation.h"
-#include "lld/Core/LLVM.h"
-#include "lld/Core/PassManager.h"
-#include "lld/Core/Reader.h"
-#include "lld/Core/Resolver.h"
-#include "lld/Core/Writer.h"
-#include "lld/Driver/Driver.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringExtras.h"
-#include "llvm/ADT/StringSwitch.h"
-#include "llvm/Option/Arg.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Path.h"
-#include "llvm/Support/Process.h"
-#include "llvm/Support/raw_ostream.h"
-#include <mutex>
-
-namespace lld {
-
-void Driver::parseLLVMOptions(const LinkingContext &ctx) {
-  // Honor -mllvm
-  if (!ctx.llvmOptions().empty()) {
-    unsigned numArgs = ctx.llvmOptions().size();
-    auto **args = new const char *[numArgs + 2];
-    args[0] = "lld (LLVM option parsing)";
-    for (unsigned i = 0; i != numArgs; ++i)
-      args[i + 1] = ctx.llvmOptions()[i];
-    args[numArgs + 1] = nullptr;
-    llvm::cl::ParseCommandLineOptions(numArgs + 1, args);
-  }
-}
-
-/// This is where the link is actually performed.
-bool Driver::link(LinkingContext &ctx, raw_ostream &diagnostics) {
-  if (ctx.getNodes().empty())
-    return false;
-
-  for (std::unique_ptr<Node> &ie : ctx.getNodes())
-    if (FileNode *node = dyn_cast<FileNode>(ie.get()))
-      node->getFile()->parse();
-
-  std::vector<std::unique_ptr<File>> internalFiles;
-  ctx.createInternalFiles(internalFiles);
-  for (auto i = internalFiles.rbegin(), e = internalFiles.rend(); i != e; ++i) {
-    auto &members = ctx.getNodes();
-    members.insert(members.begin(), llvm::make_unique<FileNode>(std::move(*i)));
-  }
-
-  // Give target a chance to add files.
-  std::vector<std::unique_ptr<File>> implicitFiles;
-  ctx.createImplicitFiles(implicitFiles);
-  for (auto i = implicitFiles.rbegin(), e = implicitFiles.rend(); i != e; ++i) {
-    auto &members = ctx.getNodes();
-    members.insert(members.begin(), llvm::make_unique<FileNode>(std::move(*i)));
-  }
-
-  // Give target a chance to postprocess input files.
-  // Mach-O uses this chance to move all object files before library files.
-  ctx.finalizeInputFiles();
-
-  // Do core linking.
-  ScopedTask resolveTask(getDefaultDomain(), "Resolve");
-  Resolver resolver(ctx);
-  if (!resolver.resolve())
-    return false;
-  std::unique_ptr<SimpleFile> merged = resolver.resultFile();
-  resolveTask.end();
-
-  // Run passes on linked atoms.
-  ScopedTask passTask(getDefaultDomain(), "Passes");
-  PassManager pm;
-  ctx.addPasses(pm);
-  if (std::error_code ec = pm.runOnFile(*merged)) {
-    diagnostics << "Failed to write file '" << ctx.outputPath()
-                << "': " << ec.message() << "\n";
-    return false;
-  }
-
-  passTask.end();
-
-  // Give linked atoms to Writer to generate output file.
-  ScopedTask writeTask(getDefaultDomain(), "Write");
-  if (std::error_code ec = ctx.writeFile(*merged)) {
-    diagnostics << "Failed to write file '" << ctx.outputPath()
-                << "': " << ec.message() << "\n";
-    return false;
-  }
-
-  return true;
-}
-
-} // namespace lld

tools/lld/lld.cpp

int main(int Argc, const char **Argv) {
 
   std::vector<const char *> Args(Argv, Argv + Argc);
   switch (parseFlavor(Args)) {
   case Gnu:
     return !elf::link(Args);
   case WinLink:
     return !coff::link(Args);
   case Darwin:
-    return !DarwinLdDriver::linkMachO(Args);
+    return !mach_o::link(Args);
   default:
     die("-flavor option is missing. Available flavors are "
         "gnu, darwin or link.");
   }
 }

unittests/DriverTests/DarwinLdDriverTest.cpp

 #include "lld/ReaderWriter/MachOLinkingContext.h"
 #include "llvm/Support/MachO.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 using namespace lld;
 
+namespace lld {
+namespace mach_o {
+bool parse(llvm::ArrayRef<const char *> args, MachOLinkingContext &ctx,
+           raw_ostream &diagnostics);
+}
+}
+
 namespace {
 class DarwinLdParserTest : public testing::Test {
 protected:
   int inputFileCount() { return _ctx.getNodes().size(); }
 
   std::string inputFile(int index) {
     Node &node = *_ctx.getNodes()[index];
     if (node.kind() == Node::Kind::File)
       return cast<FileNode>(&node)->getFile()->path();
     llvm_unreachable("not handling other types of input files");
   }
 
   bool parse(std::vector<const char *> args) {
     args.insert(args.begin(), "ld");
     std::string errorMessage;
     raw_string_ostream os(errorMessage);
-    return DarwinLdDriver::parse(args, _ctx, os);
+    return mach_o::parse(args, _ctx, os);
   }
 
   MachOLinkingContext _ctx;
 };
 }
 
 TEST_F(DarwinLdParserTest, Basic) {
   EXPECT_TRUE(parse({"foo.o", "bar.o", "-arch", "i386"}));