[PATCH] D87959: [lld-macho][NFC] Refactor syslibroot / library path lookup

[Jez Ng via Phabricator via llvm-commits]

int3 created this revision.
int3 added a reviewer: lld-macho.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
int3 requested review of this revision.

- Move computation of systemLibraryRoots into a separate function, so we can add more functionality to it without things becoming unwieldy
- Have `getSearchPaths` and related functions return by value instead of by output parameter. NRVO should ensure that performance is unaffected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87959

Files:
  lld/MachO/Driver.cpp


Index: lld/MachO/Driver.cpp
===================================================================
--- lld/MachO/Driver.cpp
+++ lld/MachO/Driver.cpp
@@ -176,10 +176,11 @@
   return true;
 }
 
-static void getSearchPaths(std::vector<StringRef> &paths, unsigned optionCode,
-                           opt::InputArgList &args,
-                           const std::vector<StringRef> &roots,
-                           const SmallVector<StringRef, 2> &systemPaths) {
+static std::vector<StringRef>
+getSearchPaths(unsigned optionCode, opt::InputArgList &args,
+               const std::vector<StringRef> &roots,
+               const SmallVector<StringRef, 2> &systemPaths) {
+  std::vector<StringRef> paths;
   StringRef optionLetter{optionCode == OPT_F ? "F" : "L"};
   for (StringRef path : args::getStrings(args, optionCode)) {
     // NOTE: only absolute paths are re-rooted to syslibroot(s)
@@ -201,7 +202,7 @@
 
   // `-Z` suppresses the standard "system" search paths.
   if (args.hasArg(OPT_Z))
-    return;
+    return paths;
 
   for (auto const &path : systemPaths) {
     for (auto root : roots) {
@@ -211,19 +212,34 @@
         paths.push_back(saver.save(buffer.str()));
     }
   }
+  return paths;
+}
+
+static std::vector<StringRef> getSystemLibraryRoots(opt::InputArgList &args) {
+  std::vector<StringRef> roots;
+  for (const Arg *arg : args.filtered(OPT_syslibroot))
+    roots.push_back(arg->getValue());
+  // NOTE: the final `-syslibroot` being `/` will ignore all roots
+  if (roots.size() && roots.back() == "/")
+    roots.clear();
+  // NOTE: roots can never be empty - add an empty root to simplify the library
+  // and framework search path computation.
+  if (roots.empty())
+    roots.emplace_back("");
+  return roots;
 }
 
-static void getLibrarySearchPaths(opt::InputArgList &args,
-                                  const std::vector<StringRef> &roots,
-                                  std::vector<StringRef> &paths) {
-  getSearchPaths(paths, OPT_L, args, roots, {"/usr/lib", "/usr/local/lib"});
+static std::vector<StringRef>
+getLibrarySearchPaths(opt::InputArgList &args,
+                      const std::vector<StringRef> &roots) {
+  return getSearchPaths(OPT_L, args, roots, {"/usr/lib", "/usr/local/lib"});
 }
 
-static void getFrameworkSearchPaths(opt::InputArgList &args,
-                                    const std::vector<StringRef> &roots,
-                                    std::vector<StringRef> &paths) {
-  getSearchPaths(paths, OPT_F, args, roots,
-                 {"/Library/Frameworks", "/System/Library/Frameworks"});
+static std::vector<StringRef>
+getFrameworkSearchPaths(opt::InputArgList &args,
+                        const std::vector<StringRef> &roots) {
+  return getSearchPaths(OPT_F, args, roots,
+                        {"/Library/Frameworks", "/System/Library/Frameworks"});
 }
 
 // Returns slices of MB by parsing MB as an archive file.
@@ -565,19 +581,11 @@
   config->runtimePaths = args::getStrings(args, OPT_rpath);
   config->allLoad = args.hasArg(OPT_all_load);
 
-  std::vector<StringRef> &roots = config->systemLibraryRoots;
-  for (const Arg *arg : args.filtered(OPT_syslibroot))
-    roots.push_back(arg->getValue());
-  // NOTE: the final `-syslibroot` being `/` will ignore all roots
-  if (roots.size() && roots.back() == "/")
-    roots.clear();
-  // NOTE: roots can never be empty - add an empty root to simplify the library
-  // and framework search path computation.
-  if (roots.empty())
-    roots.emplace_back("");
-
-  getLibrarySearchPaths(args, roots, config->librarySearchPaths);
-  getFrameworkSearchPaths(args, roots, config->frameworkSearchPaths);
+  config->systemLibraryRoots = getSystemLibraryRoots(args);
+  config->librarySearchPaths =
+      getLibrarySearchPaths(args, config->systemLibraryRoots);
+  config->frameworkSearchPaths =
+      getFrameworkSearchPaths(args, config->systemLibraryRoots);
   config->forceLoadObjC = args.hasArg(OPT_ObjC);
 
   if (args.hasArg(OPT_v)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87959.292939.patch
Type: text/x-patch
Size: 3995 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200919/058a0467/attachment.bin>