[PATCH] D64631: [obj2yaml] - Rework tool's error reporting logic for ELF target.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Happy to see more ErrorOr/std::error_code becoming Error/Expected 😊



================
Comment at: include/llvm/Object/ELF.h:364
+
+  auto Symbols = *SymsOrErr;
+  if (Index >= Symbols.size())
----------------
`Elf_Sym_Range Symbols = *SymsOrErr;`


================
Comment at: tools/obj2yaml/elf2yaml.cpp:164
+      if (Error E = dumpSymbols(&Sec, Y->Symbols))
+        return std::move(E);
     if (Sec.sh_type == ELF::SHT_DYNSYM)
----------------
`return E;` to avoid clang -Wpessimizing-move (automatic move from lvalues applies here)


================
Comment at: tools/obj2yaml/elf2yaml.cpp:176
+        return SecOrErr.takeError();
+      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(*SecOrErr));
       break;
----------------
`Y->Sections.emplace_back(*SecOrErr);`

We can leverage the ctor of unique_ptr here.


================
Comment at: tools/obj2yaml/elf2yaml.cpp:197
+        return SecOrErr.takeError();
+      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(*SecOrErr));
       break;
----------------
ditto


================
Comment at: tools/obj2yaml/elf2yaml.cpp:204
+        return GroupOrErr.takeError();
+      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(*GroupOrErr));
       break;
----------------
ditto


================
Comment at: tools/obj2yaml/elf2yaml.cpp:430
+  if (auto E = dumpCommonRelocationSection(Shdr, *S))
+    return std::move(E);
 
----------------
`return E;`


================
Comment at: tools/obj2yaml/elf2yaml.cpp:444
+      if (Error E = dumpRelocation(&Rel, SymTab, R))
+        return std::move(E);
       S->Relocations.push_back(R);
----------------
`return E;`


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

https://reviews.llvm.org/D64631