[lld-macho][nfc] Minor refactoring + clang-tidy fixes

[oontvoo]

Summary

• use "empty()" instead of "size()"
• refactor the re-export code so it doesn't create a new vector every time.

[int3]

I actually snuck in a clang-format cleanup yesterday :) https://github.com/llvm/llvm-project/commit/174deb0539ee4af7d20bfead9f73055782e878e3 (so just update the commit title I guess)

lld/MachO/Driver.cpp

1064–1067

1. no need for braces
2. I'm pretty sure the compiler can hoist out the construction of the vector

[int3]

lld/MachO/Driver.cpp

1064–1067

I'm pretty sure the compiler can hoist out the construction of the vector

well, I'm wrong: https://godbolt.org/z/szKbeojrd

[oontvoo]

removed unnecessary braces

[oontvoo]

In D100019#2674137, @int3 wrote:

I actually snuck in a clang-format cleanup yesterday :) https://github.com/llvm/llvm-project/commit/174deb0539ee4af7d20bfead9f73055782e878e3 (so just update the commit title I guess)

These are clang-tidy's suggested fixes (the size() vs empty()) - not clang-format :)

lld/MachO/Driver.cpp

1064–1067

I'm pretty sure the compiler can hoist out the construction of the vector

well, I'm wrong: https://godbolt.org/z/szKbeojrd

Probably because it can't know what foo does with the reference - hoisting it out could change the logic entirely:

void foo(const std::vector<int>& v) {
  static auto* last_vector = &v;
  static bool first = true;
  if (first) first = false;
  else  assert (&v != last_vector);  
}

[int3]

lld/MachO/Driver.cpp

359–361

ahh well this was one of the formatting changes I committed yesterday, hence the tidy/format confusion 😅

1064–1067

hmm true. Though I'm not sure if temporary rvalues are guaranteed to be given a fresh address every time...

[oontvoo]

rebase

[oontvoo]

lld/MachO/Driver.cpp

359–361

Done - sync'ing to head removed this change.

lld/MachO/Driver.cpp

if (Optional<std::string> path = findFramework(name)) {
     auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(*path, false));
     if (isWeak && dylibFile)
       dylibFile->forceWeakImport = true;
     return;
   }
   error("framework not found for -framework " + name);
 }
 
-// Parses LC_LINKER_OPTION contents, which can add additional command line flags.
-void macho::parseLCLinkerOption(InputFile* f, unsigned argc, StringRef data) {
+// Parses LC_LINKER_OPTION contents, which can add additional command line
+// flags.
+void macho::parseLCLinkerOption(InputFile *f, unsigned argc, StringRef data) {
   SmallVector<const char *, 4> argv;
   size_t offset = 0;
   for (unsigned i = 0; i < argc && offset < data.size(); ++i) {
     argv.push_back(data.data() + offset);
     offset += strlen(data.data() + offset) + 1;
   }
   if (argv.size() != argc || offset > data.size())
     fatal(toString(f) + ": invalid LC_LINKER_OPTION");
bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   lld::stderrOS = &stderrOS;
 
   errorHandler().cleanupCallback = []() { freeArena(); };
 
   errorHandler().logName = args::getFilenameWithoutExe(argsArr[0]);
   stderrOS.enable_colors(stderrOS.has_colors());
   // TODO: Set up error handler properly, e.g. the errorLimitExceededMsg
 
-
   MachOOptTable parser;
   InputArgList args = parser.parse(argsArr.slice(1));
 
   if (args.hasArg(OPT_help_hidden)) {
     parser.printHelp(argsArr[0], /*showHidden=*/true);
     return true;
   }
   if (args.hasArg(OPT_help)) {
bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   config->adhocCodesign = args.hasFlag(
       OPT_adhoc_codesign, OPT_no_adhoc_codesign,
       (config->target.Arch == AK_arm64 || config->target.Arch == AK_arm64e) &&
           config->target.Platform == PlatformKind::macOS);
 
   if (args.hasArg(OPT_v)) {
     message(getLLDVersion());
     message(StringRef("Library search paths:") +
-            (config->librarySearchPaths.size()
-                 ? "\n\t" + join(config->librarySearchPaths, "\n\t")
-                 : ""));
+            (config->librarySearchPaths.empty()
+                 ? ""
+                 : "\n\t" + join(config->librarySearchPaths, "\n\t")));
     message(StringRef("Framework search paths:") +
-            (config->frameworkSearchPaths.size()
-                 ? "\n\t" + join(config->frameworkSearchPaths, "\n\t")
-                 : ""));
+            (config->frameworkSearchPaths.empty()
+                 ? ""
+                 : "\n\t" + join(config->frameworkSearchPaths, "\n\t")));
   }
 
   config->progName = argsArr[0];
 
   config->timeTraceEnabled = args.hasArg(OPT_time_trace);
   config->timeTraceGranularity =
       args::getInteger(args, OPT_time_trace_granularity_eq, 500);
 
   // Initialize time trace profiler.
   if (config->timeTraceEnabled)
     timeTraceProfilerInitialize(config->timeTraceGranularity, config->progName);
 
   {
     TimeTraceScope timeScope("ExecuteLinker");
 
     initLLVM(); // must be run before any call to addFile()
     createFiles(args);
 
     config->isPic = config->outputType == MH_DYLIB ||
                     config->outputType == MH_BUNDLE || isPie(args);
 
     // Now that all dylibs have been loaded, search for those that should be
     // re-exported.
-    for (const Arg *arg : args.filtered(OPT_sub_library, OPT_sub_umbrella)) {
+    {
+      auto reexportHandler = [](const Arg *arg,
+                                const std::vector<StringRef> &extensions) {
         config->hasReexports = true;
         StringRef searchName = arg->getValue();
-      std::vector<StringRef> extensions;
-      if (arg->getOption().getID() == OPT_sub_library)
-        extensions = {".dylib", ".tbd"};
-      else
-        extensions = {".tbd"};
         if (!markReexport(searchName, extensions))
           error(arg->getSpelling() + " " + searchName +
                 " does not match a supplied dylib");
+      };
+      std::vector<StringRef> extensions = {".tbd"};
+      for (const Arg *arg : args.filtered(OPT_sub_umbrella))
+        reexportHandler(arg, extensions);
+
+      extensions.push_back(".dylib");
+      for (const Arg *arg : args.filtered(OPT_sub_library))
+        reexportHandler(arg, extensions);
     }
 
     // Parse LTO options.
     if (const Arg *arg = args.getLastArg(OPT_mcpu))
       parseClangOption(saver.save("-mcpu=" + StringRef(arg->getValue())),
                        arg->getSpelling());
 
     for (const Arg *arg : args.filtered(OPT_mllvm))