[PATCH] D112735: export unique symbol list for xcoff with llvm-nm new option "--export-symbols"

[Digger Lin via Phabricator via llvm-commits]

DiggerLin marked 27 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:701
+
+static void printMergedSymbolList() {
+  for (const auto &Sym : SymbolList) {
----------------
jhenderson wrote:
> Rather than having separate `printMergedSymbolList` and `printSymbolList` which both print the symbols, I think you just need one `printSymbolList` function. The `SymbolList` is cleared in the calling function, where it really belongs, rather than where it is currently cleared at the end of `printSymbolList`. There are then two calling functions, `printAllSymbols`, which builds up a single list of symbols, and is used for this new option you're implementing, and the existing `dumpSymbolNamesFromObject`.
> 
> Further, I realise that the format printed for the --export-symbols is actually just another `OutputFormat`, like `posix` or `sysv`. By making the small structural change above, you can then just implement it much like the differences in these formats is implemented, within `printSymbolList`.
I do not think I can  as your suggestion, in the function
static void printSymbolList(SymbolicFile &Obj, bool printName,
                            StringRef ArchiveName, StringRef ArchitectureName)

there is a parameter SymbolicFile &Obj , It print out all the symbols belong to the Obj,

but if we print out the export symbol list at the end, it do now have any SymbolicFile &Obj , So I can not use the same interface.


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:2388
 
+  if (ExportSymbols) {
+    sortSymbolList();
----------------
jhenderson wrote:
> This looks like you're going to print both the regular list of symbol names, and the exported list, which isn't what I was expecting. Are you sure it's what you meant to do?
It do not  print both the regular list of symbol names, and the exported list at same with  when ExportSymbols is true.

in the function


```
static void dumpSymbolNamesFromObject(SymbolicFile &Obj, bool printName,
                                      StringRef ArchiveName = {},
                                      StringRef ArchitectureName = {}) {

....
    if (ExportSymbols && Obj.isXCOFF()) {
      XCOFFObjectFile *XCOFFObj = dyn_cast<XCOFFObjectFile>(&Obj);
      assert(XCOFFObj && "Not a XCOFF Object file.");
      exportSymbolsForXCOFF(XCOFFObj, ArchiveName);
      return;
    }
...
  
  sortSymbolList();
  printSymbolList(Obj, printName, ArchiveName, ArchitectureName);
....
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112735