[PATCH] D92641: [llvm-readelf/obj] - Handle out-of-order PT_LOADs better.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 10 22:58:38 PST 2020


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


================
Comment at: llvm/unittests/Object/ELFObjectFileTest.cpp:377
+
+    EXPECT_TRUE((bool)DataOrErr);
+    EXPECT_TRUE(WarnString ==
----------------
grimar wrote:
> jhenderson wrote:
> > I think if `DataOrErr` is false, we need to somehow handle the error, right? Actually maybe this should just be `ASSERT_TRUE`?
> I can't use `ASSERT_TRUE` here (or `ASSERT_THAT_EXPECTED`). Its intention is to return instantly,
> and so it calls `return` internally I think. And so I can't use it in lambda:
> ```
> Error	C2440	'return': cannot convert from 'void' to 'const uint8_t *'	ObjectTests	D:\Work3\LLVM\llvm-project\llvm\unittests\Object\ELFObjectFileTest.cpp	377	
> ```
> 
> > I think if DataOrErr is false, we need to somehow handle the error, right?
> 
> I don't think it is very important here? We expect `toMappedAddr` call to succeed here,
> this is what test is wrapped around. When something in a unit test goes wrong,
> we can probably just report and fail, no matter how.
> 
I can consume an error and `return nullptr` right after `EXPECT_TRUE((bool)DataOrErr)` probably.
It should be a bit cleaner. Should I?


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

https://reviews.llvm.org/D92641



More information about the llvm-commits mailing list