[PATCH] D54697: [llvm-objdump] Add `Version References` dumper (PR30241)

[Xing via Phabricator via llvm-commits]

Higuoxing marked 7 inline comments as done.
Higuoxing added inline comments.


================
Comment at: tools/llvm-objdump/ELFDump.cpp:164
+
+  typedef ELFFile<ELFT> ELFO;
+  using VerNeed = typename ELFO::Elf_Verneed;
----------------
jhenderson wrote:
> Why not use `using` here?
I use `typedef` here, just like what they do in other functions.


================
Comment at: tools/llvm-objdump/ELFDump.cpp:239-259
 void llvm::printELFDynamicSection(const object::ObjectFile *Obj) {
   if (const auto *ELFObj = dyn_cast<ELF32LEObjectFile>(Obj))
     printDynamicSection(ELFObj->getELFFile(), Obj->getFileName());
   else if (const auto *ELFObj = dyn_cast<ELF32BEObjectFile>(Obj))
     printDynamicSection(ELFObj->getELFFile(), Obj->getFileName());
   else if (const auto *ELFObj = dyn_cast<ELF64LEObjectFile>(Obj))
     printDynamicSection(ELFObj->getELFFile(), Obj->getFileName());
----------------
jhenderson wrote:
> Higuoxing wrote:
> > Seems that these codes are redundant, can we simplify this?
> I'm not sure I follow what you mean?
I mean, these functions are similar (they all cast `ELF<32|64><BE|LE>` to `ElfObj`), can we simplify these functions?

```
printELFFileHeader
printELFDynamicSection
printELFSymbolVersionRefs
```




Repository:
  rL LLVM

https://reviews.llvm.org/D54697