[PATCH] D92923: [llvm-readelf/obj] - Add support of multiple SHT_SYMTAB_SHNDX sections.

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: llvm/unittests/Object/ELFTest.cpp:66-69
+      if (Expected<uint8_t> ValOrErr = R[I])
+        EXPECT_EQ(*ValOrErr, I);
+      else
+        ADD_FAILURE() << toString(ValOrErr.takeError());
----------------
grimar wrote:
> jhenderson wrote:
> > Could this be:
> > 
> > ```
> > ASSERT_THAT_EXPECTED(ValOrErr, Succeeded());
> > EXPECT_EQ(*ValOrErr, I);
> > ```
> > 
> > Or something to that effect? It seems simpler, and if the good index-value cases don't work, there doesn't seem to be much point in worrying about the rest of the test case running, so asserting is fine.
> Yes, it is possible to write in this way here. Done.
Actually, does the ASSERT_THAT_EXPECTED work (i.e. stop the test) if the Expected is in a failure state here? Just slightly concerned given that it's in a lambda. If it doesn't, feel free to go back to the previous version.


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

https://reviews.llvm.org/D92923