[PATCH] D87141: [llvm-readobj/elf] - Introduce Relocation<ELFT> helper.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:217
+  Optional<int64_t> getAddend() const { return Addend; }
+  const Elf_Rel &getRel() const { return Reloc; }
+
----------------
jhenderson wrote:
> I'd rather avoid this, for reasons which I think were previously discussed when you first tried something like this - it essentially lies about the underlying relocation type. Furthermore, I don't think it is necessary - you can store the relocation's properties in the class itself, derived at construction time when you already have a "real" relocation. In other words, your members might become something like this:
> 
> ```
> uint64_t Offset;
> Optional<int64_t> Addend;
> uint32_t SymIndex;
> uint32_t Type;
> ```
> 
> You might need the odd new function elsewhere, to replace where you do things like `getRelocationSymbol`, to take an index, but I don't think any of them will be that complex.
Done.


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

https://reviews.llvm.org/D87141