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

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added inline comments.


================
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());
----------------
jhenderson wrote:
> 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.
Yes, it works fine here, I've checked it. It doesn't stop the test, but it reports a failure and returns from the labda,
what is OK here.

If we wanted to return some value from the labda or if we needed to terminate the test then it wouldn't work.


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

https://reviews.llvm.org/D92923