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

[int3]

Summary

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

[smeenai]

I suppose it's more efficient to have a single iteration of the argument list, but I also don't think argument parsing efficiency matters at all (or at least doesn't matter enough for this change to be a problem), so LGTM.

[thakis]

lld/MachO/Driver.cpp

594

nit: the handle prefix is somewhat common for option handling cf eg handleUndefined in the ELF linker

[int3]

lld/MachO/Driver.cpp

594

hm but the handle functions in ELF seem to take the arg / arg value. Whereas functions that take the ArgList and do both arg lookup + handling seem to be prefixed with get, e.g. getReproduceOption.

lld/MachO/Driver.cpp

static inline char toLowerDash(char x) {
   return x;
 }
 
 static std::string lowerDash(StringRef s) {
   return std::string(map_iterator(s.begin(), toLowerDash),
                      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)
           .Cases("tvos", "3", PlatformKind::tvOS)
           .Cases("watchos", "4", PlatformKind::watchOS)
           .Cases("bridgeos", "5", PlatformKind::bridgeOS)
           .Cases("mac-catalyst", "6", PlatformKind::macCatalyst)
           .Cases("ios-simulator", "7", PlatformKind::iOSSimulator)
           .Cases("tvos-simulator", "8", PlatformKind::tvOSSimulator)
           .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)
           .Default(UndefinedSymbolTreatment::unknown);
   if (treatment == UndefinedSymbolTreatment::unknown) {
     warn(Twine("unknown -undefined TREATMENT '") + treatmentStr +
          "', defaulting to 'error'");
     treatment = UndefinedSymbolTreatment::error;
   } else if (config->namespaceKind == NamespaceKind::twolevel &&
              (treatment == UndefinedSymbolTreatment::warning ||
               treatment == UndefinedSymbolTreatment::suppress)) {
     if (treatment == UndefinedSymbolTreatment::warning)
       error("'-undefined warning' only valid with '-flat_namespace'");
     else
       error("'-undefined suppress' only valid with '-flat_namespace'");
     treatment = UndefinedSymbolTreatment::error;
   }
-  config->undefinedSymbolTreatment = treatment;
+  return treatment;
 }
 
 static void warnIfDeprecatedOption(const opt::Option &opt) {
   if (!opt.getGroup().isValid())
     return;
   if (opt.getGroup().getID() == OPT_grp_deprecated) {
     warn("Option `" + opt.getPrefixedName() + "' is deprecated in ld64:");
     warn(opt.getHelpText());
if (const char *path = getReproduceOption(args)) {
     } else {
       error("--reproduce: " + toString(errOrWriter.takeError()));
     }
   }
 
   config = make<Configuration>();
   symtab = make<SymbolTable>();
   target = createTargetInfo(args);
+  config->platform = getPlatformVersion(args);
 
   config->entry = symtab->addUndefined(args.getLastArgValue(OPT_e, "_main"),
                                        /*file=*/nullptr,
                                        /*isWeakRef=*/false);
   for (auto *arg : args.filtered(OPT_u)) {
     config->explicitUndefineds.push_back(symtab->addUndefined(
         arg->getValue(), /*file=*/nullptr, /*isWeakRef=*/false));
   }
bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   config->forceLoadObjC = args.hasArg(OPT_ObjC);
   config->demangle = args.hasArg(OPT_demangle);
   config->implicitDylibs = !args.hasArg(OPT_no_implicit_dylibs);
 
   if (const opt::Arg *arg = args.getLastArg(OPT_static, OPT_dynamic))
     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 =
       getLibrarySearchPaths(args, config->systemLibraryRoots);
   config->frameworkSearchPaths =
       getFrameworkSearchPaths(args, config->systemLibraryRoots);
   if (const opt::Arg *arg =
           args.getLastArg(OPT_search_paths_first, OPT_search_dylibs_first))
if (args.hasArg(OPT_v)) {
     message(StringRef("Framework search paths:") +
             (config->frameworkSearchPaths.size()
                  ? "\n\t" + join(config->frameworkSearchPaths, "\n\t")
                  : ""));
   }
 
   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);
       break;
     case OPT_weak_library:
       if (auto *dylibFile =
               dyn_cast_or_null<DylibFile>(addFile(arg->getValue(), false)))
         dylibFile->forceWeakImport = true;
       break;
     case OPT_filelist:
       addFileList(arg->getValue());
       break;
     case OPT_force_load:
       addFile(arg->getValue(), true);
       break;
     case OPT_l:
     case OPT_weak_l:
       addLibrary(arg->getValue(), opt.getID() == OPT_weak_l);
       break;
     case OPT_framework:
     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;
     }
   }
 
   config->isPic = config->outputType == MH_DYLIB ||
                   config->outputType == MH_BUNDLE || isPie(args);