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

[James Henderson via Phabricator via llvm-commits]

jhenderson 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; }
+
----------------
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.


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

https://reviews.llvm.org/D87141