[lld] 1168736 - [lld-macho][nfc] Parse more options using getLastArg{Value}

[Jez Ng via llvm-commits]

Author: Jez Ng
Date: 2021-03-03T15:52:06-05:00
New Revision: 1168736c668872d7e0000a64daf63dff1c242f80

URL: https://github.com/llvm/llvm-project/commit/1168736c668872d7e0000a64daf63dff1c242f80
DIFF: https://github.com/llvm/llvm-project/commit/1168736c668872d7e0000a64daf63dff1c242f80.diff

LOG: [lld-macho][nfc] Parse more options using getLastArg{Value}

The option-iterating loop should be reserved for options whose command-line
order is important. I think LLD-ELF follows a similar design.

Reviewed By: #lld-macho, smeenai

Differential Revision: https://reviews.llvm.org/D97797

Added: 
    

Modified: 
    lld/MachO/Driver.cpp

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 7426fcb0fae8..6179adb1bb3b 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -560,13 +560,18 @@ static std::string lowerDash(StringRef s) {
                      map_iterator(s.end(), toLowerDash));
 }
 
-static void handlePlatformVersion(const opt::Arg *arg) {
+static PlatformInfo getPlatformVersion(const opt::ArgList &args) {
+  const opt::Arg *arg = args.getLastArg(OPT_platform_version);
+  PlatformInfo platform;
+  if (!arg)
+    return platform;
+
   StringRef platformStr = arg->getValue(0);
   StringRef minVersionStr = arg->getValue(1);
   StringRef sdkVersionStr = arg->getValue(2);
 
   // TODO(compnerd) see if we can generate this case list via XMACROS
-  config->platform.kind =
+  platform.kind =
       StringSwitch<PlatformKind>(lowerDash(platformStr))
           .Cases("macos", "1", PlatformKind::macOS)
           .Cases("ios", "2", PlatformKind::iOS)
@@ -579,23 +584,25 @@ static void handlePlatformVersion(const opt::Arg *arg) {
           .Cases("watchos-simulator", "9", PlatformKind::watchOSSimulator)
           .Cases("driverkit", "10", PlatformKind::driverKit)
           .Default(PlatformKind::unknown);
-  if (config->platform.kind == PlatformKind::unknown)
+  if (platform.kind == PlatformKind::unknown)
     error(Twine("malformed platform: ") + platformStr);
   // TODO: check validity of version strings, which varies by platform
   // NOTE: ld64 accepts version strings with 5 components
   // llvm::VersionTuple accepts no more than 4 components
   // Has Apple ever published version strings with 5 components?
-  if (config->platform.minimum.tryParse(minVersionStr))
+  if (platform.minimum.tryParse(minVersionStr))
     error(Twine("malformed minimum version: ") + minVersionStr);
-  if (config->platform.sdk.tryParse(sdkVersionStr))
+  if (platform.sdk.tryParse(sdkVersionStr))
     error(Twine("malformed sdk version: ") + sdkVersionStr);
+  return platform;
 }
 
-static void handleUndefined(const opt::Arg *arg) {
-  StringRef treatmentStr = arg->getValue(0);
+static UndefinedSymbolTreatment
+getUndefinedSymbolTreatment(const opt::ArgList &args) {
+  StringRef treatmentStr = args.getLastArgValue(OPT_undefined);
   auto treatment =
       StringSwitch<UndefinedSymbolTreatment>(treatmentStr)
-          .Case("error", UndefinedSymbolTreatment::error)
+          .Cases("error", "", UndefinedSymbolTreatment::error)
           .Case("warning", UndefinedSymbolTreatment::warning)
           .Case("suppress", UndefinedSymbolTreatment::suppress)
           .Case("dynamic_lookup", UndefinedSymbolTreatment::dynamic_lookup)
@@ -613,7 +620,7 @@ static void handleUndefined(const opt::Arg *arg) {
       error("'-undefined suppress' only valid with '-flat_namespace'");
     treatment = UndefinedSymbolTreatment::error;
   }
-  config->undefinedSymbolTreatment = treatment;
+  return treatment;
 }
 
 static void warnIfDeprecatedOption(const opt::Option &opt) {
@@ -752,6 +759,7 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   config = make<Configuration>();
   symtab = make<SymbolTable>();
   target = createTargetInfo(args);
+  config->platform = getPlatformVersion(args);
 
   config->entry = symtab->addUndefined(args.getLastArgValue(OPT_e, "_main"),
                                        /*file=*/nullptr,
@@ -792,11 +800,12 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
     config->staticLink = (arg->getOption().getID() == OPT_static);
 
   if (const opt::Arg *arg =
-          args.getLastArg(OPT_flat_namespace, OPT_twolevel_namespace)) {
+          args.getLastArg(OPT_flat_namespace, OPT_twolevel_namespace))
     config->namespaceKind = arg->getOption().getID() == OPT_twolevel_namespace
                                 ? NamespaceKind::twolevel
                                 : NamespaceKind::flat;
-  }
+
+  config->undefinedSymbolTreatment = getUndefinedSymbolTreatment(args);
 
   config->systemLibraryRoots = getSystemLibraryRoots(args);
   config->librarySearchPaths =
@@ -846,12 +855,13 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
 
   initLLVM(); // must be run before any call to addFile()
 
+  // This loop should be reserved for options whose exact ordering matters.
+  // Other options should be handled via filtered() and/or getLastArg().
   for (const auto &arg : args) {
     const auto &opt = arg->getOption();
     warnIfDeprecatedOption(opt);
     warnIfUnimplementedOption(opt);
 
-    // TODO: are any of these better handled via filtered() or getLastArg()?
     switch (opt.getID()) {
     case OPT_INPUT:
       addFile(arg->getValue(), false);
@@ -875,12 +885,6 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
     case OPT_weak_framework:
       addFramework(arg->getValue(), opt.getID() == OPT_weak_framework);
       break;
-    case OPT_platform_version:
-      handlePlatformVersion(arg);
-      break;
-    case OPT_undefined:
-      handleUndefined(arg);
-      break;
     default:
       break;
     }