[PATCH] D120357: [llvm-nm]add helper function to print out the object file name, archive name, architecture name

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:1889
+  CurrentFilename = Obj.getFileName();
+  if (IsSymbolsEmpty && SymbolList.empty() && !Quiet) {
     writeFileName(errs(), ArchiveName, ArchitectureName);
----------------
DiggerLin wrote:
> jhenderson wrote:
> > Why have you just introduced this change? How is it better than the previous code?
> yes, in the original code of function getSymbolNamesFromObject( )
> 
> ```
>   auto Symbols = Obj.symbols();
>   .....
>  
>   if (DynamicSyms) {
>     const auto *E = dyn_cast<ELFObjectFileBase>(&Obj);
>     if (!E) {
>       error("File format has no dynamic symbol table", Obj.getFileName());
>       return;
>     }
>   Symbols = E->getDynamicSymbolIterators();
> ```
>  The value of Symbols be changed by the line 
> Symbols = E->getDynamicSymbolIterators();
>  Symbols  maybe not the equal to  Obj.symbols()
> 
> so we can not use the 
> if ( Obj.symbols().empty() && SymbolList.empty() && !Quiet) 
Thanks, I see. I think I'd prefer it if you pulled the logic on whether or not an object has symbols into a separate function (maybe `hasSymbols`) which takes `Obj` and returns the value based on the appropriate set of symbols for whether `DynamicSyms` is specified or not. You probably would then benefit from a separate helper function used by this new function and `getSymbolNamesFromObject` that returns the dynamic symbols list (or an empty list if there was an error). Something like the following:

```
static bool hasSymbols(SymbolicFile &Obj) {
  if (DynamicSyms)
    return !getDynamicSyms(Obj).empty();
  return !Obj.symbols().empty();
}

static <insert return type here> getDynamicSyms(SymbolFile &Obj) {
  const auto *E = dyn_cast<ELFObjectFileBase>(&Obj);
  if (!E) {
    error("File format has no dynamic symbol table", Obj.getFileName());
    return {};
  }
  return E->getDynamicSymbolIterators();
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120357