[PATCH] D126792: [lld-macho] Addressed additional post-commit comments from D126046

[Vy Nguyen via Phabricator via llvm-commits]

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

- fixed newlines
- renamed helper function for clarity
- added additional comment


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126792

Files:
  lld/MachO/Driver.cpp
  lld/MachO/SyntheticSections.cpp


Index: lld/MachO/SyntheticSections.cpp
===================================================================
--- lld/MachO/SyntheticSections.cpp
+++ lld/MachO/SyntheticSections.cpp
@@ -987,7 +987,9 @@
 
   // Local symbols aren't in the SymbolTable, so we walk the list of object
   // files to gather them.
-  // But if `-x` is set, then we don't need to.
+  // But if `-x` is set, then we don't need to. localSymbolsHandler() will do
+  // the right thing regardless, but this check is a perf optimization because
+  // iterating through all the input files and their symbols is expensive.
   if (config->localSymbolsPresence != SymtabPresence::None) {
     for (const InputFile *file : inputFiles) {
       if (auto *objFile = dyn_cast<ObjFile>(file)) {
Index: lld/MachO/Driver.cpp
===================================================================
--- lld/MachO/Driver.cpp
+++ lld/MachO/Driver.cpp
@@ -938,8 +938,8 @@
   return matchLiteral(symbolName) || matchGlob(symbolName);
 }
 
-static void handleSymbolPatternsListHelper(const Arg *arg,
-                                           SymbolPatterns &symbolPatterns) {
+static void parseSymbolPatternsFile(const Arg *arg,
+                                    SymbolPatterns &symbolPatterns) {
   StringRef path = arg->getValue();
   Optional<MemoryBufferRef> buffer = readFile(path);
   if (!buffer) {
@@ -953,6 +953,7 @@
       symbolPatterns.insert(line);
   }
 }
+
 static void handleSymbolPatterns(InputArgList &args,
                                  SymbolPatterns &symbolPatterns,
                                  unsigned singleOptionCode,
@@ -960,7 +961,7 @@
   for (const Arg *arg : args.filtered(singleOptionCode))
     symbolPatterns.insert(arg->getValue());
   for (const Arg *arg : args.filtered(listFileOptionCode))
-    handleSymbolPatternsListHelper(arg, symbolPatterns);
+    parseSymbolPatternsFile(arg, symbolPatterns);
 }
 
 static void createFiles(const InputArgList &args) {
@@ -1426,7 +1427,6 @@
       switch (arg->getOption().getID()) {
       case OPT_x:
         config->localSymbolsPresence = SymtabPresence::None;
-
         break;
       case OPT_non_global_symbols_no_strip_list:
         if (excludeLocal) {
@@ -1435,7 +1435,7 @@
         } else {
           includeLocal = true;
           config->localSymbolsPresence = SymtabPresence::SelectivelyIncluded;
-          handleSymbolPatternsListHelper(arg, config->localSymbolPatterns);
+          parseSymbolPatternsFile(arg, config->localSymbolPatterns);
         }
         break;
       case OPT_non_global_symbols_strip_list:
@@ -1445,7 +1445,7 @@
         } else {
           excludeLocal = true;
           config->localSymbolsPresence = SymtabPresence::SelectivelyExcluded;
-          handleSymbolPatternsListHelper(arg, config->localSymbolPatterns);
+          parseSymbolPatternsFile(arg, config->localSymbolPatterns);
         }
         break;
       default:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126792.433440.patch
Type: text/x-patch
Size: 2911 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220601/e36589c3/attachment.bin>