[Error] Make llvm::cantFail include the original error messages

[hintonda]

Summary

The current implementation eats the current errors and just outputs
the message parameter passed to llvm::cantFail. This change appends
the original error message(s), so the user can see exactly why
cantFail failed.

[lhames]

I think this should be made conditional on NDEBUG so that it doesn’t bloat callsites in release mode, but otherwise this looks good to me!

[hintonda]

• Wrap new login in !NDEBUG.

llvm/include/llvm/Support/Error.h

 ///   Error foo(bool DoFallibleOperation);
 ///
 ///   cantFail(foo(false));
 ///   @endcode
 inline void cantFail(Error Err, const char *Msg = nullptr) {
   if (Err) {
     if (!Msg)
       Msg = "Failure value returned from cantFail wrapped call";
+#ifndef NDEBUG
+    std::string Str;
+    raw_string_ostream OS(Str);
+    OS << Msg << "\n" << Err;
+    Msg = OS.str().c_str();
+#endif
     llvm_unreachable(Msg);
   }
 }
 
 /// Report a fatal error if ValOrErr is a failure value, otherwise unwraps and
 /// returns the contained value.
 ///
 /// This function can be used to wrap calls to fallible functions ONLY when it
 /// is known that the Error will always be a success value. E.g.
 ///
 ///   @code{.cpp}
 ///   // foo only attempts the fallible operation if DoFallibleOperation is
 ///   // true. If DoFallibleOperation is false then foo always returns an int.
 ///   Expected<int> foo(bool DoFallibleOperation);
 ///
 ///   int X = cantFail(foo(false));
 ///   @endcode
 template <typename T>
 T cantFail(Expected<T> ValOrErr, const char *Msg = nullptr) {
   if (ValOrErr)
     return std::move(*ValOrErr);
   else {
     if (!Msg)
       Msg = "Failure value returned from cantFail wrapped call";
+#ifndef NDEBUG
+    std::string Str;
+    raw_string_ostream OS(Str);
+    auto E = ValOrErr.takeError();
+    OS << Msg << "\n" << E;
+    Msg = OS.str().c_str();
+#endif
     llvm_unreachable(Msg);
   }
 }
 
 /// Report a fatal error if ValOrErr is a failure value, otherwise unwraps and
 /// returns the contained reference.
 ///
 /// This function can be used to wrap calls to fallible functions ONLY when it
 /// is known that the Error will always be a success value. E.g.
 ///
 ///   @code{.cpp}
 ///   // foo only attempts the fallible operation if DoFallibleOperation is
 ///   // true. If DoFallibleOperation is false then foo always returns a Bar&.
 ///   Expected<Bar&> foo(bool DoFallibleOperation);
 ///
 ///   Bar &X = cantFail(foo(false));
 ///   @endcode
 template <typename T>
 T& cantFail(Expected<T&> ValOrErr, const char *Msg = nullptr) {
   if (ValOrErr)
     return *ValOrErr;
   else {
     if (!Msg)
       Msg = "Failure value returned from cantFail wrapped call";
+#ifndef NDEBUG
+    std::string Str;
+    raw_string_ostream OS(Str);
+    auto E = ValOrErr.takeError();
+    OS << Msg << "\n" << E;
+    Msg = OS.str().c_str();
+#endif
     llvm_unreachable(Msg);
   }
 }
 
 /// Helper for testing applicability of, and applying, handlers for
 /// ErrorInfo types.
 template <typename HandlerT>
 class ErrorHandlerTraits

llvm/unittests/Support/ErrorTest.cpp

TEST(Error, FailureToHandle) {
   auto FailToHandle = []() {
     handleAllErrors(make_error<CustomError>(7), [&](const CustomSubError &SE) {
       errs() << "This should never be called";
       exit(1);
     });
   };
 
   EXPECT_DEATH(FailToHandle(),
-               "Failure value returned from cantFail wrapped call")
+               "Failure value returned from cantFail wrapped call\n"
+               "CustomError \\{7\\}")
       << "Unhandled Error in handleAllErrors call did not cause an "
          "abort()";
 }
 #endif
 
 // Test that handleAllUnhandledErrors crashes if an error is returned from a
 // handler.
 // Test runs in debug mode only.
 #if LLVM_ENABLE_ABI_BREAKING_CHECKS
 TEST(Error, FailureFromHandler) {
   auto ReturnErrorFromHandler = []() {
     handleAllErrors(make_error<CustomError>(7),
                     [&](std::unique_ptr<CustomSubError> SE) {
                       return Error(std::move(SE));
                     });
   };
 
   EXPECT_DEATH(ReturnErrorFromHandler(),
-               "Failure value returned from cantFail wrapped call")
+               "Failure value returned from cantFail wrapped call\n"
+               "CustomError \\{7\\}")
       << " Error returned from handler in handleAllErrors call did not "
          "cause abort()";
 }
 #endif
 
 // Test that we can return values from handleErrors.
 TEST(Error, CatchErrorFromHandler) {
   int ErrorInfo = 0;
TEST(Error, CantFailSuccess) {
   int Dummy = 42;
   int &Y = cantFail(Expected<int&>(Dummy));
   EXPECT_EQ(&Dummy, &Y) << "Reference mangled by cantFail";
 }
 
 // Test that cantFail results in a crash if you pass it a failure value.
 #if LLVM_ENABLE_ABI_BREAKING_CHECKS && !defined(NDEBUG)
 TEST(Error, CantFailDeath) {
-  EXPECT_DEATH(
-      cantFail(make_error<StringError>("foo", inconvertibleErrorCode()),
+  EXPECT_DEATH(cantFail(make_error<StringError>("Original error message",
+                                                inconvertibleErrorCode()),
                         "Cantfail call failed"),
-      "Cantfail call failed")
+               "Cantfail call failed\n"
+               "Original error message")
       << "cantFail(Error) did not cause an abort for failure value";
 
   EXPECT_DEATH(
       {
         auto IEC = inconvertibleErrorCode();
         int X = cantFail(Expected<int>(make_error<StringError>("foo", IEC)));
         (void)X;
       },
       "Failure value returned from cantFail wrapped call")