[PATCH] D104646: [AIX][XCOFF] 64-bit relocation reading support

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:334
+    if (Error E = RelocationsOrErr.takeError()) {
+      consumeError(std::move(E));
+      return relocation_iterator(RelocationRef());
----------------
I'd add a TODO to say to report the error up the stack. Same below.

In general, `consumeError` calls should be accompanied with a comment saying why it's okay to throw away the error. In this case, it's just because the interface doesn't currently support reporting the error, hence the TODO to improve the situation.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:148
                   << " {\n";
-    for (auto Reloc : Relocations) {
-      StringRef SymbolName = unwrapOrError(
-          Obj.getFileName(), Obj.getSymbolNameByIndex(Reloc.SymbolIndex));
+    for (const Rel &Reloc : Relocations) {
+      Expected<StringRef> ErrOrSymbolName =
----------------
Or something similar - "Rel" could easily be confused with the variable name. I'd probably change the other instanecs in a similar manner too.


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

https://reviews.llvm.org/D104646