[PATCH] D81928: [llvm-readobj] - Split the printGnuHashTable(). NFCI.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 17 01:34:55 PDT 2020


grimar marked 2 inline comments as done.
grimar added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2742
+
+  return GnuHashTable->values(NumSyms);
+}
----------------
jhenderson wrote:
> Does the change to return an empty array in D81937 actually belong here? I'm a little confused why we seem to be introducing a function that is known to be broken from the outset, only to fix it immediately, rather than just doing it right straight up.
I just want to keep this patch NFC, It opens road for other patches (D81937 is only one of them). And to fix this place, we need a test case. This is what D81937 does, it adds a test case and fixes another one. I think it deserves to be an independent change.



================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2782
+  if (!Chains) {
+    reportWarning(Chains.takeError(), ObjF->getFileName());
     return;
----------------
jhenderson wrote:
> Does it make sense to change this to `reportUniqueWarning` whilst you're here?
I can do this. It seems to be a no-op change,


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

https://reviews.llvm.org/D81928





More information about the llvm-commits mailing list