[PATCH] D76684: [obj2yaml] - Refactor how we dump sections. NFCI.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 25 02:08:20 PDT 2020


jhenderson added inline comments.


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:119-120
   StringRef Name = *NameOrErr;
+  if (Name.empty())
+    return "";
+
----------------
Could you explain this case, please, perhaps with a comment in the code?


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:284
+Expected<ELFYAML::RawContentSection *>
+ELFDumper<ELFT>::dumpSectionAsPlaceholder(const Elf_Shdr *Shdr) {
+  auto S = std::make_unique<ELFYAML::RawContentSection>();
----------------
`dumpPlaceholderSection` maybe? Slightly shorter name.


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:348
+      // file. But we still want to dump them, because their properties can be
+      // important. See comments for shouldPrintSection()` for more details.
+      return [this](const Elf_Shdr *S) { return dumpSectionAsPlaceholder(S); };
----------------
Looks like you have a copy-paste/formatting error around `shouldPrintSection()`.


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

https://reviews.llvm.org/D76684





More information about the llvm-commits mailing list