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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I would say that the exact position of errors and warnings in the output is of lesser concern, and it's okay for them to change as the implementation changes, as long as the error messages have sufficient context in them - if they don't, this is something that could be improved though.

However, it occurs to me that a proposal of "fetch all symbols" and then "dump all symbols" may be costly in terms of memory usage. I'm usually not so fussed about memory usage, but I'd want to see what the impact of this approach would be. I guess the way to do better, without adversely impact the memory footprint of regular usage, is to have a lambda passed down the call stack, which operates on an object when it is encountered. Under regular usage, this lambda would be a function to print all symbols for that object (sorted as needed). Under export symbols usage, it would store the symbols in a global vector, which would then be printed etc at a later point. Outline code:

  void forEachFile(function_ref<void(const std::vector<NMSymbol> &, /*other args as needed*/)> DoThis, ArrayRef<StringRef> Filenames) {
    // for each input file, determine file kind, call processArchive/processMachOUniversalBinary/...
  }
  
  void process*(... DoThis, ...) {
    // if multiple objects in file, loop over all doing the following, otherwise, do the following on just the one file:
    auto Syms = getSymbolsFromObject(Obj);
    DoThis(Syms, ...);
  }
  
  int main() {
  ...
  
    if (ExportSymbols) {
      std::vector<NMSymbol> Syms;
      forEachFile([&Syms](const std::vector<NMSymbol> &SymbolList, ...) {
                     Syms.insert(Syms.end(), SymbolList.begin(), SymbolList.end());
                  }, InputFilenames);
      // filter, sort, print exports
    } else {
      forEachFile([](const std::vector<NMSymbol> &SymbolList){
                     // sort then print symbols
                  }, InputFilenames);
    }
  }

Hopefully you see what I'm getting at. This isn't really a high priority for me though, so don't feel obliged to look at this further, but if you do get a chance to look, I think it is worth considering. Hopefully the recent refactoring should put the code in a decent place for adopting something like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120913