[PATCH] D52821: [Disassembler][llvm-readobj] ELF note dumper abstraction

Saleem Abdulrasool via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 4 10:25:02 PDT 2018


compnerd requested changes to this revision.
compnerd added inline comments.
This revision now requires changes to proceed.


================
Comment at: include/llvm/Object/ELFNoteDumper.h:28
+public:
+  ELFNoteDumper() {}
+
----------------
Why not use `= default`?


================
Comment at: include/llvm/Object/ELFNoteDumper.h:32
+  ELFNoteDumper &operator=(const ELFNoteDumper &) = delete;
+  virtual ~ELFNoteDumper() {}
+
----------------
We can use the destructor as the key function rather than emitting the data globally.


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:3811
+        }
+      }
+      if (!Dumped)
----------------
I'm not sure if I really like this approach.  We already have the name that identifies who should interpret the note.  There is nothing that prevents the use of the `aarch64-unknown-windows-msvc-elf` to use the "AMD" vendor namespace to add a note.  However, I don't think that any target registered will have that triple.  How would that work?


Repository:
  rL LLVM

https://reviews.llvm.org/D52821





More information about the llvm-commits mailing list