[PATCH] D120913: [NFC][llvm-nm] create a new helper function exportSymbolNamesFromFiles for --export-symbols

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM.

> do we still another NFC patch

To summarise, yes, we need another patch (at least one, possibly more), but I don't really mind what that patch looks like (i.e. it doesn't have to match what I suggested before).

My key end goal of the suggestions I made in D112735 <https://reviews.llvm.org/D112735> was the removal of references to `ExportSymbols` causing early outs in otherwise common functions. I've highlighted the one section of code that has such references. I don't really mind how you achieve that end goal, as long as it leaves the code in a clean structural state (or at least as clean as it was before you started the export symbols work). My suggestion was merely one part of the wider plan, to ensure we had the information needed, and isn't necessarily the only approach, so if you have another approach then that's fine too.

Away from that, there's at least one other minor improvement I'd like to see: rename `exportSymbolNamesFromFiles` to `dumpExportSymbolList`, as I think "dump" indicates "print something" more clearly than "export". The "export symbol list" part of the name then is what is being dumped, and nicely lines up with the option name.



================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:1893-1903
   if (ExportSymbols && Obj.isXCOFF()) {
     XCOFFObjectFile *XCOFFObj = cast<XCOFFObjectFile>(&Obj);
     getXCOFFExports(XCOFFObj, SymbolList, ArchiveName);
     return;
   }
 
   if (PrintObjectLabel && !ExportSymbols)
----------------
References to `ExportSymbols`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120913/new/

https://reviews.llvm.org/D120913