[NFC][llvm-nm] refactor function dumpSymbolNamesFromFile

[DiggerLin]

Summary

split the function into several small functions.
change the function like

static void dumpSymbolNamesFromFile(std::string &Filename) {

ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
    MemoryBuffer::getFileOrSTDIN(Filename);
if (error(BufferOrErr.getError(), Filename))
  return;

LLVMContext Context;
LLVMContext *ContextPtr = NoLLVMBitcode ? nullptr : &Context;
Expected<std::unique_ptr<Binary>> BinaryOrErr =
    createBinary(BufferOrErr.get()->getMemBufferRef(), ContextPtr);
if (!BinaryOrErr) {
  error(BinaryOrErr.takeError(), Filename);
  return;
}
Binary &Bin = *BinaryOrErr.get();
if (Archive *A = dyn_cast<Archive>(&Bin))
  dumpArchive(A, Filename, ContextPtr);
else if (MachOUniversalBinary *UB = dyn_cast<MachOUniversalBinary>(&Bin))
  dumpMachOUniversalBinary(UB, Filename, ContextPtr);
else if (TapiUniversal *TU = dyn_cast<TapiUniversal>(&Bin))
  dumpTapiUniversal(TU, Filename);
else if (SymbolicFile *O = dyn_cast<SymbolicFile>(&Bin))
  dumpSymbolicFile(O, Filename);

}

[MaskRay]

LGTM.

[jhenderson]

Looks good, with one request though.

llvm/tools/llvm-nm/llvm-nm.cpp

2054

Might be worth a comment saying what the boolean return indicates.

[jhenderson]

Looks good, with the suggested comment rewrite.

llvm/tools/llvm-nm/llvm-nm.cpp

2054–2055

(and reflow as necesary).

I've taken the wording from a similar comment elsewhere.

llvm/tools/llvm-nm/llvm-nm.cpp

static bool checkMachOAndArchFlags(SymbolicFile *O, std::string &Filename) {
   const std::string ArchFlagName(ArchFlag);
   if (!llvm::is_contained(ArchFlags, ArchFlagName)) {
     error("No architecture specified", Filename);
     return false;
   }
   return true;
 }
 
-static void dumpSymbolNamesFromFile(std::string &Filename) {
-  ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
-      MemoryBuffer::getFileOrSTDIN(Filename);
-  if (error(BufferOrErr.getError(), Filename))
-    return;
-
-  LLVMContext Context;
-  LLVMContext *ContextPtr = NoLLVMBitcode ? nullptr : &Context;
-  Expected<std::unique_ptr<Binary>> BinaryOrErr =
-      createBinary(BufferOrErr.get()->getMemBufferRef(), ContextPtr);
-  if (!BinaryOrErr) {
-    error(BinaryOrErr.takeError(), Filename);
-    return;
-  }
-  Binary &Bin = *BinaryOrErr.get();
-
-  if (Archive *A = dyn_cast<Archive>(&Bin)) {
-    if (ArchiveMap) {
-      Archive::symbol_iterator I = A->symbol_begin();
-      Archive::symbol_iterator E = A->symbol_end();
-      if (I != E) {
-        outs() << "Archive map\n";
-        for (; I != E; ++I) {
-          Expected<Archive::Child> C = I->getMember();
-          if (!C) {
-            error(C.takeError(), Filename);
-            break;
-          }
-          Expected<StringRef> FileNameOrErr = C->getName();
-          if (!FileNameOrErr) {
-            error(FileNameOrErr.takeError(), Filename);
-            break;
-          }
-          StringRef SymName = I->getName();
-          outs() << SymName << " in " << FileNameOrErr.get() << "\n";
-        }
-        outs() << "\n";
-      }
-    }
+static void dumpArchiveMap(Archive *A, std::string &Filename) {
 
-    {
-      Error Err = Error::success();
-      for (auto &C : A->children(Err)) {
-        Expected<std::unique_ptr<Binary>> ChildOrErr =
-            C.getAsBinary(ContextPtr);
-        if (!ChildOrErr) {
-          if (auto E = isNotObjectErrorInvalidFileType(ChildOrErr.takeError()))
-            error(std::move(E), Filename, C);
-          continue;
-        }
-        if (SymbolicFile *O = dyn_cast<SymbolicFile>(&*ChildOrErr.get())) {
-          if (!MachOPrintSizeWarning && PrintSize &&  isa<MachOObjectFile>(O)) {
-            WithColor::warning(errs(), ToolName)
-                << "sizes with -print-size for Mach-O files are always zero.\n";
-            MachOPrintSizeWarning = true;
-          }
-          if (!checkMachOAndArchFlags(O, Filename))
-            return;
-          if (!PrintFileName && shouldDump(*O) && !ExportSymbols) {
-            outs() << "\n";
-            if (isa<MachOObjectFile>(O)) {
-              outs() << Filename << "(" << O->getFileName() << ")";
-            } else
-              outs() << O->getFileName();
-            outs() << ":\n";
-          }
-          dumpSymbolNamesFromObject(*O, false, Filename);
-        }
-      }
-      if (Err)
-        error(std::move(Err), A->getFileName());
-    }
-    return;
-  }
-  if (MachOUniversalBinary *UB = dyn_cast<MachOUniversalBinary>(&Bin)) {
-    // If we have a list of architecture flags specified dump only those.
-    if (!ArchAll && !ArchFlags.empty()) {
-      // Look for a slice in the universal binary that matches each ArchFlag.
-      bool ArchFound;
-      for (unsigned i = 0; i < ArchFlags.size(); ++i) {
-        ArchFound = false;
-        for (MachOUniversalBinary::object_iterator I = UB->begin_objects(),
-                                                   E = UB->end_objects();
-             I != E; ++I) {
-          if (ArchFlags[i] == I->getArchFlagName()) {
-            ArchFound = true;
-            Expected<std::unique_ptr<ObjectFile>> ObjOrErr =
-                I->getAsObjectFile();
-            std::string ArchiveName;
-            std::string ArchitectureName;
-            ArchiveName.clear();
-            ArchitectureName.clear();
-            if (ObjOrErr) {
-              ObjectFile &Obj = *ObjOrErr.get();
-              if (ArchFlags.size() > 1) {
-                if (PrintFileName)
-                  ArchitectureName = I->getArchFlagName();
-                else
-                  outs() << "\n" << Obj.getFileName() << " (for architecture "
-                         << I->getArchFlagName() << ")"
-                         << ":\n";
-              }
-              dumpSymbolNamesFromObject(Obj, false, ArchiveName,
-                                        ArchitectureName);
-            } else if (auto E = isNotObjectErrorInvalidFileType(
-                       ObjOrErr.takeError())) {
-              error(std::move(E), Filename, ArchFlags.size() > 1 ?
-                    StringRef(I->getArchFlagName()) : StringRef());
-              continue;
-            } else if (Expected<std::unique_ptr<Archive>> AOrErr =
-                           I->getAsArchive()) {
-              std::unique_ptr<Archive> &A = *AOrErr;
-              Error Err = Error::success();
-              for (auto &C : A->children(Err)) {
-                Expected<std::unique_ptr<Binary>> ChildOrErr =
-                    C.getAsBinary(ContextPtr);
-                if (!ChildOrErr) {
-                  if (auto E = isNotObjectErrorInvalidFileType(
-                                       ChildOrErr.takeError())) {
-                    error(std::move(E), Filename, C, ArchFlags.size() > 1 ?
-                          StringRef(I->getArchFlagName()) : StringRef());
-                  }
-                  continue;
-                }
-                if (SymbolicFile *O =
-                        dyn_cast<SymbolicFile>(&*ChildOrErr.get())) {
-                  if (PrintFileName) {
-                    ArchiveName = std::string(A->getFileName());
-                    if (ArchFlags.size() > 1)
-                      ArchitectureName = I->getArchFlagName();
-                  } else {
-                    outs() << "\n" << A->getFileName();
-                    outs() << "(" << O->getFileName() << ")";
-                    if (ArchFlags.size() > 1) {
-                      outs() << " (for architecture " << I->getArchFlagName()
-                             << ")";
-                    }
-                    outs() << ":\n";
-                  }
-                  dumpSymbolNamesFromObject(*O, false, ArchiveName,
-                                            ArchitectureName);
-                }
-              }
-              if (Err)
-                error(std::move(Err), A->getFileName());
-            } else {
-              consumeError(AOrErr.takeError());
-              error(Filename + " for architecture " +
+static void dumpArchive(Archive *A, std::string &Filename,
+                        LLVMContext *ContextPtr) {
+  if (ArchiveMap)
+    dumpArchiveMap(A, Filename);
+
+    Expected<std::unique_ptr<Binary>> ChildOrErr = C.getAsBinary(ContextPtr);
+      if (!MachOPrintSizeWarning && PrintSize && isa<MachOObjectFile>(O)) {
+
+static void dumpMachOUniversalBinaryMatchArchFlags(MachOUniversalBinary *UB,
+                                                   std::string &Filename,
+                                                   LLVMContext *ContextPtr) {
+        Expected<std::unique_ptr<ObjectFile>> ObjOrErr = I->getAsObjectFile();
+              outs() << "\n"
+                     << Obj.getFileName() << " (for architecture "
+          dumpSymbolNamesFromObject(Obj, false, ArchiveName, ArchitectureName);
+        } else if (auto E =
+                       isNotObjectErrorInvalidFileType(ObjOrErr.takeError())) {
+          error(std::move(E), Filename,
+                ArchFlags.size() > 1 ? StringRef(I->getArchFlagName())
+                                     : StringRef());
+              if (auto E =
+                      isNotObjectErrorInvalidFileType(ChildOrErr.takeError())) {
+                error(std::move(E), Filename, C,
+                      ArchFlags.size() > 1 ? StringRef(I->getArchFlagName())
+                                           : StringRef());
+            if (SymbolicFile *O = dyn_cast<SymbolicFile>(&*ChildOrErr.get())) {
                     StringRef(I->getArchFlagName()) +
                     " is not a Mach-O file or an archive file",
-                    "Mach-O universal file");
-            }
-          }
-        }
-        if (!ArchFound) {
-          error(ArchFlags[i],
-                "file: " + Filename + " does not contain architecture");
-          return;
-        }
-      }
-      return;
-    }
-    // No architecture flags were specified so if this contains a slice that
-    // matches the host architecture dump only that.
-    if (!ArchAll) {
-      Triple HostTriple = MachOObjectFile::getHostArch();
-      StringRef HostArchName = HostTriple.getArchName();
-      for (MachOUniversalBinary::object_iterator I = UB->begin_objects(),
-                                                 E = UB->end_objects();
-           I != E; ++I) {
-        if (HostArchName == I->getArchFlagName()) {
-          Expected<std::unique_ptr<ObjectFile>> ObjOrErr = I->getAsObjectFile();
-          std::string ArchiveName;
-          if (ObjOrErr) {
-            ObjectFile &Obj = *ObjOrErr.get();
-            dumpSymbolNamesFromObject(Obj, false);
-          } else if (auto E = isNotObjectErrorInvalidFileType(
-                     ObjOrErr.takeError())) {
-            error(std::move(E), Filename);
-            return;
-          } else if (Expected<std::unique_ptr<Archive>> AOrErr =
-                         I->getAsArchive()) {
-            std::unique_ptr<Archive> &A = *AOrErr;
-            Error Err = Error::success();
-            for (auto &C : A->children(Err)) {
-              Expected<std::unique_ptr<Binary>> ChildOrErr =
-                  C.getAsBinary(ContextPtr);
-              if (!ChildOrErr) {
-                if (auto E = isNotObjectErrorInvalidFileType(
-                                     ChildOrErr.takeError()))
-                  error(std::move(E), Filename, C);
-                continue;
-              }
-              if (SymbolicFile *O =
-                      dyn_cast<SymbolicFile>(&*ChildOrErr.get())) {
-                if (PrintFileName)
-                  ArchiveName = std::string(A->getFileName());
-                else
-                  outs() << "\n" << A->getFileName() << "(" << O->getFileName()
-                         << ")"
-                         << ":\n";
-                dumpSymbolNamesFromObject(*O, false, ArchiveName);
-              }
-            }
-            if (Err)
-              error(std::move(Err), A->getFileName());
-          } else {
-            consumeError(AOrErr.takeError());
-            error(Filename + " for architecture " +
+
+// Returns true If the binary contains a slice that matches the host
+// architecture, or false otherwise.
+static bool dumpMachOUniversalBinaryMatchHost(MachOUniversalBinary *UB,
+                                              std::string &Filename,
+                                              LLVMContext *ContextPtr) {
+      } else if (auto E = isNotObjectErrorInvalidFileType(ObjOrErr.takeError()))
+      else if (Expected<std::unique_ptr<Archive>> AOrErr = I->getAsArchive()) {
+            if (auto E =
+                    isNotObjectErrorInvalidFileType(ChildOrErr.takeError()))
+          if (SymbolicFile *O = dyn_cast<SymbolicFile>(&*ChildOrErr.get())) {
+              outs() << "\n"
+                     << A->getFileName() << "(" << O->getFileName() << ")"
                   StringRef(I->getArchFlagName()) +
                   " is not a Mach-O file or an archive file",
-                  "Mach-O universal file");
-          }
-          return;
-        }
-      }
-    }
-    // Either all architectures have been specified or none have been specified
-    // and this does not contain the host architecture so dump all the slices.
-    bool moreThanOneArch = UB->getNumberOfObjects() > 1;
-    for (const MachOUniversalBinary::ObjectForArch &O : UB->objects()) {
-      Expected<std::unique_ptr<ObjectFile>> ObjOrErr = O.getAsObjectFile();
-      std::string ArchiveName;
-      std::string ArchitectureName;
-      ArchiveName.clear();
-      ArchitectureName.clear();
-      if (ObjOrErr) {
-        ObjectFile &Obj = *ObjOrErr.get();
-        if (PrintFileName) {
-          if (isa<MachOObjectFile>(Obj) && moreThanOneArch)
-            ArchitectureName = O.getArchFlagName();
-        } else {
-          if (moreThanOneArch)
-            outs() << "\n";
-          outs() << Obj.getFileName();
-          if (isa<MachOObjectFile>(Obj) && moreThanOneArch)
-            outs() << " (for architecture " << O.getArchFlagName() << ")";
-          outs() << ":\n";
-        }
-        dumpSymbolNamesFromObject(Obj, false, ArchiveName, ArchitectureName);
-      } else if (auto E = isNotObjectErrorInvalidFileType(
-                 ObjOrErr.takeError())) {
-        error(std::move(E), Filename, moreThanOneArch ?
-              StringRef(O.getArchFlagName()) : StringRef());
-        continue;
-      } else if (Expected<std::unique_ptr<Archive>> AOrErr =
-                  O.getAsArchive()) {
-        std::unique_ptr<Archive> &A = *AOrErr;
-        Error Err = Error::success();
-        for (auto &C : A->children(Err)) {
-          Expected<std::unique_ptr<Binary>> ChildOrErr =
+      return true;
+  return false;
+
+static void dumpMachOUniversalBinaryArchAll(MachOUniversalBinary *UB,
+                                            std::string &Filename,
+                                            LLVMContext *ContextPtr) {
+    } else if (auto E = isNotObjectErrorInvalidFileType(ObjOrErr.takeError())) {
+      error(std::move(E), Filename,
+            moreThanOneArch ? StringRef(O.getArchFlagName()) : StringRef());
+    } else if (Expected<std::unique_ptr<Archive>> AOrErr = O.getAsArchive()) {
             C.getAsBinary(ContextPtr);
-          if (!ChildOrErr) {
-            if (auto E = isNotObjectErrorInvalidFileType(
-                                 ChildOrErr.takeError()))
-              error(std::move(E), Filename, C, moreThanOneArch ?
-                    StringRef(ArchitectureName) : StringRef());
-            continue;
-          }
-          if (SymbolicFile *F = dyn_cast<SymbolicFile>(&*ChildOrErr.get())) {
-            if (PrintFileName) {
-              ArchiveName = std::string(A->getFileName());
-              if (isa<MachOObjectFile>(F) && moreThanOneArch)
-                ArchitectureName = O.getArchFlagName();
-            } else {
-              outs() << "\n" << A->getFileName();
-              if (isa<MachOObjectFile>(F)) {
-                outs() << "(" << F->getFileName() << ")";
-                if (moreThanOneArch)
-                  outs() << " (for architecture " << O.getArchFlagName()
-                         << ")";
-              } else
-                outs() << ":" << F->getFileName();
-              outs() << ":\n";
-            }
-            dumpSymbolNamesFromObject(*F, false, ArchiveName, ArchitectureName);
-          }
-        }
-        if (Err)
-          error(std::move(Err), A->getFileName());
-      } else {
-        consumeError(AOrErr.takeError());
-        error(Filename + " for architecture " +
-              StringRef(O.getArchFlagName()) +
+          if (auto E = isNotObjectErrorInvalidFileType(ChildOrErr.takeError()))
+            error(std::move(E), Filename, C,
+                  moreThanOneArch ? StringRef(ArchitectureName) : StringRef());
+                outs() << " (for architecture " << O.getArchFlagName() << ")";
+      error(Filename + " for architecture " + StringRef(O.getArchFlagName()) +
                 " is not a Mach-O file or an archive file",
-              "Mach-O universal file");
-      }
-    }
+}
+
+static void dumpMachOUniversalBinary(MachOUniversalBinary *UB,
+                                     std::string &Filename,
+                                     LLVMContext *ContextPtr) {
+  // If we have a list of architecture flags specified dump only those.
+  if (!ArchAll && !ArchFlags.empty()) {
+    dumpMachOUniversalBinaryMatchArchFlags(UB, Filename, ContextPtr);
+    return;
+  }
+
+  // No architecture flags were specified so if this contains a slice that
+  // matches the host architecture dump only that.
+  if (!ArchAll && dumpMachOUniversalBinaryMatchHost(UB, Filename, ContextPtr))
     return;
-  }
+
+  // Either all architectures have been specified or none have been specified
+  // and this does not contain the host architecture so dump all the slices.
+  dumpMachOUniversalBinaryArchAll(UB, Filename, ContextPtr);
 
-  if (TapiUniversal *TU = dyn_cast<TapiUniversal>(&Bin)) {
-    for (const TapiUniversal::ObjectForArch &I : TU->objects()) {
-      StringRef ArchName = I.getArchFlagName();
-      const bool ShowArch =
-          ArchFlags.empty() || llvm::is_contained(ArchFlags, ArchName);
-      if (!ShowArch)
-        continue;
-      if (!AddInlinedInfo && !I.isTopLevelLib())
-        continue;
-      if (auto ObjOrErr = I.getAsObjectFile()) {
-        outs() << "\n"
-               << I.getInstallName() << " (for architecture " << ArchName << ")"
-               << ":\n";
-        dumpSymbolNamesFromObject(*ObjOrErr.get(), false, {}, ArchName);
-      } else if (Error E =
-                     isNotObjectErrorInvalidFileType(ObjOrErr.takeError())) {
-        error(std::move(E), Filename, ArchName);
-      }
-    }
-
-    return;
-  }
+static void dumpTapiUniversal(TapiUniversal *TU, std::string &Filename) {
 
-  if (SymbolicFile *O = dyn_cast<SymbolicFile>(&Bin)) {
-    if (!MachOPrintSizeWarning && PrintSize &&  isa<MachOObjectFile>(O)) {
-      WithColor::warning(errs(), ToolName)
-          << "sizes with --print-size for Mach-O files are always zero.\n";
-      MachOPrintSizeWarning = true;
-    }
-    if (!checkMachOAndArchFlags(O, Filename))
-      return;
-    dumpSymbolNamesFromObject(*O, true);
-  }
+static void dumpSymbolicFile(SymbolicFile *O, std::string &Filename) {
+  if (!MachOPrintSizeWarning && PrintSize && isa<MachOObjectFile>(O)) {
+
+static void dumpSymbolNamesFromFile(std::string &Filename) {
+  ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
+      MemoryBuffer::getFileOrSTDIN(Filename);
+  if (error(BufferOrErr.getError(), Filename))
+    return;
+
+  LLVMContext Context;
+  LLVMContext *ContextPtr = NoLLVMBitcode ? nullptr : &Context;
+  Expected<std::unique_ptr<Binary>> BinaryOrErr =
+      createBinary(BufferOrErr.get()->getMemBufferRef(), ContextPtr);
+  if (!BinaryOrErr) {
+    error(BinaryOrErr.takeError(), Filename);
+    return;
+  }
+  Binary &Bin = *BinaryOrErr.get();
+  if (Archive *A = dyn_cast<Archive>(&Bin))
+    dumpArchive(A, Filename, ContextPtr);
+  else if (MachOUniversalBinary *UB = dyn_cast<MachOUniversalBinary>(&Bin))
+    dumpMachOUniversalBinary(UB, Filename, ContextPtr);
+  else if (TapiUniversal *TU = dyn_cast<TapiUniversal>(&Bin))
+    dumpTapiUniversal(TU, Filename);
+  else if (SymbolicFile *O = dyn_cast<SymbolicFile>(&Bin))
+    dumpSymbolicFile(O, Filename);
 }
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
   BumpPtrAllocator A;
   StringSaver Saver(A);
   NmOptTable Tbl;
   ToolName = argv[0];