Improve readability of MinGW driver. NFC.

[ruiu]

Summary

In addition to removing a few global variables and functions, I believe
this patch improves code readability a bit in general.

[mstorsjo]

Looks good to me on a read-through, although I didn't test run it yet.

[mstorsjo]

Tested in a proper mingw/clang/lld setup, LGTM.

[pcc]

lld/MinGW/Driver.cpp

176 ↗

(On Diff #114631)

It looks like you are changing the link order; now all non--l inputs will appear before -l inputs. I don't think this should matter except in some edge cases, but it would at least mean that this change is not NFC.

[ruiu]

• Address review comment.

lld/MinGW/Driver.cpp

176 ↗

(On Diff #114631)

That's true. For some reason I believed that this wouldn't change the behavior, but it indeed does. Fixed.

[mstorsjo]

LGTM

[pcc]

LGTM

lld/trunk/MinGW/Driver.cpp

 class COFFLdOptTable : public opt::OptTable {
 public:
   COFFLdOptTable() : OptTable(InfoTable, false) {}
   opt::InputArgList parse(ArrayRef<const char *> Argv);
 };
 
 } // namespace
 
-static std::vector<std::string> LinkArgs;
-static std::vector<StringRef> SearchPaths;
-
-static void error(const Twine &Msg) {
+LLVM_ATTRIBUTE_NORETURN static void error(const Twine &Msg) {
   errs() << Msg << "\n";
-  llvm_shutdown();
   exit(1);
 }
 
 // Find a file by concatenating given paths.
 static Optional<std::string> findFile(StringRef Path1, const Twine &Path2) {
   SmallString<128> S;
   sys::path::append(S, Path1, Path2);
   if (sys::fs::exists(S))
     return S.str().str();
   return None;
 }
 
-static Optional<std::string> findFromSearchPaths(StringRef Path) {
+// This is for -lfoo. We'll look for libfoo.dll.a or libfoo.a from search paths.
+static std::string
+searchLibrary(StringRef Name, ArrayRef<StringRef> SearchPaths, bool BStatic) {
+  if (Name.startswith(":")) {
     for (StringRef Dir : SearchPaths)
-    if (Optional<std::string> S = findFile(Dir, Path))
-      return S;
-  return None;
+      if (Optional<std::string> S = findFile(Dir, Name.substr(1)))
+        return *S;
+    error("unable to find library -l" + Name);
   }
 
-// This is for -lfoo. We'll look for libfoo.dll.a or libfoo.a from search paths.
-static Optional<std::string> searchLibrary(StringRef Name, bool StaticOnly) {
-  if (Name.startswith(":"))
-    return findFromSearchPaths(Name.substr(1));
   for (StringRef Dir : SearchPaths) {
-    if (!StaticOnly)
+    if (!BStatic)
       if (Optional<std::string> S = findFile(Dir, "lib" + Name + ".dll.a"))
-        return S;
+        return *S;
     if (Optional<std::string> S = findFile(Dir, "lib" + Name + ".a"))
-      return S;
-  }
-  return None;
+      return *S;
   }
-
-// Add a given library by searching it from input search paths.
-static void addLibrary(StringRef Name, bool StaticOnly) {
-  if (Optional<std::string> Path = searchLibrary(Name, StaticOnly))
-    LinkArgs.push_back(*Path);
-  else
-    error("unable to find library -l" + Name);
 }
 
-static void createFiles(opt::InputArgList &Args) {
-  for (auto *Arg : Args) {
-    switch (Arg->getOption().getUnaliasedOption().getID()) {
-    case OPT_l:
-      addLibrary(Arg->getValue(), Args.hasArg(OPT_Bstatic));
-      break;
-    case OPT_INPUT:
-      LinkArgs.push_back(Arg->getValue());
-      break;
-    }
-  }
-}
-
-static void forward(opt::InputArgList &Args, unsigned Key,
-                    const std::string &OutArg, std::string Default = "") {
-  StringRef S = Args.getLastArgValue(Key);
-  if (!S.empty())
-    LinkArgs.push_back(std::string("-").append(OutArg).append(":").append(S));
-  else if (!Default.empty())
-    LinkArgs.push_back(
-        std::string("-").append(OutArg).append(":").append(Default));
-}
-
-static void forwardValue(opt::InputArgList &Args, unsigned Key,
-                         const std::string &CmpArg, const std::string &OutArg) {
-  StringRef S = Args.getLastArgValue(Key);
-  if (S == CmpArg)
-    LinkArgs.push_back(std::string("-").append(OutArg));
-}
-
-static bool convertValue(opt::InputArgList &Args, unsigned Key,
-                         StringRef OutArg) {
-  if (Args.hasArg(Key)) {
-    LinkArgs.push_back(std::string("-").append(OutArg));
-    return true;
-  }
-  return false;
-}
-
 opt::InputArgList COFFLdOptTable::parse(ArrayRef<const char *> Argv) {
   unsigned MissingIndex;
   unsigned MissingCount;
+
   SmallVector<const char *, 256> Vec(Argv.data(), Argv.data() + Argv.size());
   opt::InputArgList Args = this->ParseArgs(Vec, MissingIndex, MissingCount);
+
   if (MissingCount)
-    error(Twine(Args.getArgString(MissingIndex)) + ": missing argument");
-  if (!Args.hasArgNoClaim(OPT_INPUT) && !Args.hasArgNoClaim(OPT_l))
-    error("no input files");
+    error(StringRef(Args.getArgString(MissingIndex)) + ": missing argument");
   for (auto *Arg : Args.filtered(OPT_UNKNOWN))
     error("unknown argument: " + Arg->getSpelling());
+  if (!Args.hasArgNoClaim(OPT_INPUT) && !Args.hasArgNoClaim(OPT_l))
+    error("no input files");
   return Args;
 }
 
+// Convert Unix-ish command line arguments to Windows-ish ones and
+// then call coff::link.
 bool link(ArrayRef<const char *> ArgsArr, raw_ostream &Diag) {
   COFFLdOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
-  LinkArgs.push_back(ArgsArr[0]);
 
-  forwardValue(Args, OPT_m, "i386pe", "machine:x86");
-  forwardValue(Args, OPT_m, "i386pep", "machine:x64");
-  forwardValue(Args, OPT_m, "thumb2pe", "machine:arm");
-  forwardValue(Args, OPT_m, "arm64pe", "machine:arm64");
-
-  forward(Args, OPT_o, "out",
-          convertValue(Args, OPT_shared, "dll") ? "a.dll" : "a.exe");
-  forward(Args, OPT_entry, "entry");
-  forward(Args, OPT_subs, "subsystem");
-  forward(Args, OPT_outlib, "implib");
-  forward(Args, OPT_stack, "stack");
+  std::vector<std::string> LinkArgs;
+  auto Add = [&](const Twine &S) { LinkArgs.push_back(S.str()); };
 
-  for (auto *Arg : Args.filtered(OPT_L))
-    SearchPaths.push_back(Arg->getValue());
+  Add(ArgsArr[0]);
 
-  createFiles(Args);
+  if (auto *A = Args.getLastArg(OPT_entry))
+    Add("-entry:" + StringRef(A->getValue()));
+  if (auto *A = Args.getLastArg(OPT_subs))
+    Add("-subsystem:" + StringRef(A->getValue()));
+  if (auto *A = Args.getLastArg(OPT_outlib))
+    Add("-implib:" + StringRef(A->getValue()));
+  if (auto *A = Args.getLastArg(OPT_stack))
+    Add("-stack:" + StringRef(A->getValue()));
+
+  if (auto *A = Args.getLastArg(OPT_o))
+    Add("-out:" + StringRef(A->getValue()));
+  else if (Args.hasArg(OPT_shared))
+    Add("-out:a.dll");
+  else
+    Add("-out:a.exe");
+
+  if (Args.hasArg(OPT_shared))
+    Add("-dll");
+
+  if (auto *A = Args.getLastArg(OPT_m)) {
+    StringRef S = A->getValue();
+    if (S == "i386pe")
+      Add("-machine:x86");
+    else if (S == "i386pep")
+      Add("-machine:x64");
+    else if (S == "thumb2pe")
+      Add("-machine:arm");
+    else if (S == "arm64pe")
+      Add("-machine:arm64");
+    else
+      error("unknown parameter: -m" + S);
+  }
 
-  // handle __image_base__
   if (Args.getLastArgValue(OPT_m) == "i386pe")
-    LinkArgs.push_back("/alternatename:__image_base__=___ImageBase");
+    Add("-alternatename:__image_base__=___ImageBase");
   else
-    LinkArgs.push_back("/alternatename:__image_base__=__ImageBase");
+    Add("-alternatename:__image_base__=__ImageBase");
+
+  std::vector<StringRef> SearchPaths;
+  for (auto *A : Args.filtered(OPT_L))
+    SearchPaths.push_back(A->getValue());
+
+  for (auto *A : Args.filtered(OPT_INPUT, OPT_l)) {
+    if (A->getOption().getUnaliasedOption().getID() == OPT_INPUT)
+      Add(A->getValue());
+    else
+      Add(searchLibrary(A->getValue(), SearchPaths, Args.hasArg(OPT_Bstatic)));
+  }
 
-  // repack vector of strings to vector of const char pointers for coff::link
+  // Repack vector of strings to vector of const char pointers for coff::link.
   std::vector<const char *> Vec;
   for (const std::string &S : LinkArgs)
     Vec.push_back(S.c_str());
   return coff::link(Vec);
 }
 
 } // namespace mingw
 } // namespace lld