[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