[PATCH] D88097: [llvm-readelf/obj] - Cleanup the code. NFCI.

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.

Generally looks fine, barring nits. I wonder whether it might be easier for downstream providers such as my team to merge this patch in if it were in separate commits for each of the major changes, but I don't really mind.



================
Comment at: llvm/tools/llvm-readobj/ARMEHABIPrinter.h:435
                                              uint64_t TableEntryOffset) const {
-  Expected<ArrayRef<uint8_t>> Contents = ELF->getSectionContents(*EHT);
+  // TODO: handle failture.
+  Expected<ArrayRef<uint8_t>> Contents = ELF.getSectionContents(*EHT);
----------------
Here and elsewhere.


================
Comment at: llvm/tools/llvm-readobj/ARMEHABIPrinter.h:582
+      // TODO: handle failture.
+      if (auto SectionName = ELF.getSectionName(Sec))
         SW.printString("SectionName", *SectionName);
----------------
Maybe another `auto` that could be replaced whilst you're at it?


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3941
+  Fields[2].Str = to_string(format_decimal(Symbol.st_size, 5));
+  Fields[2].Str = to_string(format_decimal(Symbol.st_size, 5));
 
----------------
Delete duplicated line.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:4967
     OS << "    Properties:";
-    for (const auto &Property : getGNUPropertyList<ELFT>(Desc))
+    for (const std::string &Property : getGNUPropertyList<ELFT>(Desc))
       OS << "    " << Property << "\n";
----------------



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

https://reviews.llvm.org/D88097