[PATCH] D155045: [llvm-objdump] Create ObjectFile specific dumpers

Fri Jul 14 08:30:12 PDT 2023

mtrofin accepted this revision.
mtrofin added a comment.
This revision is now accepted and ready to land.

lgtm, some nits.

================
Comment at: llvm/tools/llvm-objdump/COFFDump.cpp:40

-class COFFDumper {
+class COFFDumper : public Dumper {
 public:
----------------
(Comment for discussion, not blocking this patch) should `ObjDumper` be reused by both readobj and objdump - this was prompted by my going to initially suggest - also for a potential future patch - renaming `Dumper` to something more specific (like `ObjDumper`) - then realized the name was taken. Or maybe one could be `ObjDumper` and the other `ReadObjDumper`?

Anyway, this can go offline on discourse.

================
Comment at: llvm/tools/llvm-objdump/ELFDump.cpp:44
+template <class ELFT>
+static std::unique_ptr<Dumper> createDumper(const ELFObjectFile<ELFT> &Obj) {
+  return std::make_unique<ELFDumper<ELFT>>(Obj);
----------------
does this buy much - looks like `objdump::createELFDumper` below would just look about the same, and (subjectively) even maybe a bit more clear?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155045