[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