[clang] [llvm] [OpenMP][OMPIRBuilder] Handle non-failing calls properly (PR #115863)
Michael Kruse via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 13 16:26:10 PST 2024
================
@@ -345,18 +344,15 @@ TEST_F(OpenMPIRBuilderTest, CreateBarrier) {
IRBuilder<> Builder(BB);
- OpenMPIRBuilder::InsertPointOrErrorTy BarrierIP1 =
- OMPBuilder.createBarrier({IRBuilder<>::InsertPoint()}, OMPD_for);
- assert(BarrierIP1 && "unexpected error");
+ ASSERT_TRUE(expectedToOptional(
+ OMPBuilder.createBarrier({IRBuilder<>::InsertPoint()}, OMPD_for)));
----------------
Meinersbur wrote:
`llvm::Expected` as well as `std::optional` are convertable to `bool`, but because `ASSERT_TRUE` actually converts to `AssertionResult`, only the latter works. So this is the shorter version of the same (which I have been using so far):
```suggestion
ASSERT_TRUE((bool)OMPBuilder.createBarrier({IRBuilder<>::InsertPoint()}, OMPD_for));
```
I hoped you could find something nicer. Some tests define their own macro `ASSERT_NO_ERROR`. LLVM itself defines `ASSERT_THAT_EXPECTED` to be used in GTest. None of these return the value in-case of no-error, so ugly to use, but seem to be the canonical way to do the check. How about a helper function similar to `cantFail`, but guaranteed to crash instead of undefined behaviour?
```
template<typename T>
T successOrAbort(Expected<T> &&ValOrErr) {
if (ValOrErr)
return std::move(*ValOrErr);
report_fatal_error(ValOrErr.takeError());
}
```
Something really cool would properly integreate into the GTest envorinment.
```
template<typename T>
T successOrAbort(Expected<T> &&ValOrErr) {
if (ValOrErr)
return std::move(*ValOrErr);
GTEST_NONFATAL_FAILURE_("Expected<T> contains error value"); // Might be considered internal to GTest.
abort(); // Cannot continue without a value to unwrap
}
```
or make it a macro such we can customize the message and use `GTEST_MESSAGE_AT_` and report the affected line number.
https://github.com/llvm/llvm-project/pull/115863
More information about the cfe-commits
mailing list