[Error] Add source location to cantFail

[hintonda]

Summary

Add llvm_cantFail macro, a drop in replacement for llvm::cantFail,
that adds source location, similar to assert, for !NDEBUG builds.

Refactor to extract error handling code and place it and new annotated
versions of cantFail that are called by the new macro in a detail
namespace, and change all instances in llvm to use the new macro.

[hintonda]

• Replace macro magic with matching 3-parameter template functions.

[lhames]

Hi Don,

This is a great idea!

I think it would actually be reasonable to have 'cantFail' be the macro though, and move the existing functions (which become an implementation detail) into a detail namespace: We want cantFail to be succinct as part of its value is clarifying flow when we know (from the surrounding context) that errors will not be generated.

[hintonda]

• Replace macro magic with matching 3-parameter template functions.
• Refactor cantFail move into detail namespace. Change macro name to

[hintonda]

• Add back original llvm::cantFail signatures so they'll still be

[hintonda]

Append "_annotated" suffix to detail functions.
Add "llvm_" prefix to macros.
Revert changes to all non-llvm project files.
Update calls in llvm to use new macro.

[hintonda]

@lhames, I think I've applied all your verbal changes, but please let me
know if I missed anything.

Essentially, previous behavior is maintained, but users can opt-in to the
new behavior, adding file and line numbers, by switching to the new macro.
I also updated all of llvm, but excluded other projects for the time being.
Most of them use llvm::cantFail, and could be updated as needed via a
script, e.g., something like this would do it:

find clang -type f \( -name "*.cpp" -o -name "*.h" \) -exec grep -l "cantFail" \{\} \; | xargs sed -i.bak 's/llvm::cantFail/llvm_cantFail/'

llvm/include/llvm/Support/Error.h

 ///   @code{.cpp}
 ///   // foo only attempts the fallible operation if DoFallibleOperation is
 ///   // true. If DoFallibleOperation is false then foo always returns
 ///   // Error::success().
 ///   Error foo(bool DoFallibleOperation);
 ///
 ///   cantFail(foo(false));
 ///   @endcode
-inline void cantFail(Error Err, const char *Msg = nullptr) {
+inline void cantFail(Error Err, const char *Msg = nullptr,
+                     const char *file = nullptr, unsigned line = 0) {
   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;
+    OS << Err << "\n" << Msg;
+    if (file)
+      OS << " at " << file << ":" << line;
     Msg = OS.str().c_str();
 #endif
     llvm_unreachable(Msg);
   }
 }
 
+inline void cantFail(Error Err, const char *file, unsigned line) {
+  cantFail(std::move(Err), nullptr, file, line);
+}
+
 /// 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) {
+T cantFail(Expected<T> ValOrErr, const char *Msg = nullptr,
+           const char *file = nullptr, unsigned line = 0) {
   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;
+    OS << E << "\n" << Msg;
+    if (file)
+      OS << " at " << file << ":" << line;
     Msg = OS.str().c_str();
 #endif
     llvm_unreachable(Msg);
   }
 }
 
+template <typename T>
+T cantFail(Expected<T> ValOrErr, const char *file, unsigned line) {
+  return cantFail<Expected<T>>(std::move(ValOrErr), nullptr, file, line);
+}
+
 /// 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) {
+T &cantFail(Expected<T &> ValOrErr, const char *Msg = nullptr,
+            const char *file = nullptr, unsigned line = 0) {
   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;
+    OS << E << "\n" << Msg;
+    if (file)
+      OS << " at " << file << ":" << line;;
     Msg = OS.str().c_str();
 #endif
     llvm_unreachable(Msg);
   }
 }
 
+template <typename T>
+T &cantFail(Expected<T &> ValOrErr, const char *file, unsigned line) {
+  return cantFail<Expected<T &>>(std::move(ValOrErr), nullptr, file, line);
+}
+
+#ifndef NDEBUG
+#define llvm_cantFail(...) ::llvm::cantFail(__VA_ARGS__, __FILE__, __LINE__)
+#else
+#define llvm_cantFail(...) ::llvm::cantFail(__VA_ARGS__)
+#endif
+
 /// Helper for testing applicability of, and applying, handlers for
 /// ErrorInfo types.
 template <typename HandlerT>
 class ErrorHandlerTraits
     : public ErrorHandlerTraits<decltype(
           &std::remove_reference<HandlerT>::type::operator())> {};
 
 // Specialization functions of the form 'Error (const ErrT&)'.
Error handleErrors(Error E, HandlerTs &&... Hs) {
   return handleErrorImpl(std::move(Payload), std::forward<HandlerTs>(Hs)...);
 }
 
 /// Behaves the same as handleErrors, except that by contract all errors
 /// *must* be handled by the given handlers (i.e. there must be no remaining
 /// errors after running the handlers, or llvm_unreachable is called).
 template <typename... HandlerTs>
 void handleAllErrors(Error E, HandlerTs &&... Handlers) {
-  cantFail(handleErrors(std::move(E), std::forward<HandlerTs>(Handlers)...));
+  llvm_cantFail(handleErrors(std::move(E), std::forward<HandlerTs>(Handlers)...));
 }
 
 /// Check that E is a non-error, then drop it.
 /// If E is an error, llvm_unreachable will be called.
 inline void handleAllErrors(Error E) {
-  cantFail(std::move(E));
+  llvm_cantFail(std::move(E));
 }
 
 /// Handle any errors (if present) in an Expected<T>, then try a recovery path.
 ///
 /// If the incoming value is a success value it is returned unmodified. If it
 /// is a failure value then it the contained error is passed to handleErrors.
 /// If handleErrors is able to handle the error then the RecoveryPath functor
 /// is called to supply the final result. If handleErrors is not able to

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\n"
-               "CustomError \\{7\\}")
+               "CustomError \\{7\\}\n"
+               "Failure value returned from cantFail wrapped call")
       << "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\n"
-               "CustomError \\{7\\}")
+               "CustomError \\{7\\}\n"
+               "Failure value returned from cantFail wrapped call")
       << " 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 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>("Original error message",
                                                 inconvertibleErrorCode()),
                         "Cantfail call failed"),
-               "Cantfail call failed\n"
-               "Original error message")
+               "Original error message\n"
+               "Cantfail call failed")
       << "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")
     << "cantFail(Expected<int>) did not cause an abort for failure value";
 }
 #endif
 
+TEST(Error, CantFailSourceLocation) {
+  std::string ErrStr;
+  raw_string_ostream OS(ErrStr);
+  OS << "\nFailure value returned from cantFail wrapped call";
+
+#if defined(NDEBUG)
+  // __FILE__ and __LINE_ not added
+  OS << "\n";
+  EXPECT_DEATH(llvm_cantFail(make_error<CustomError>(1)), OS.str())
+      << "cantFail(Error) did not cause an abort for failure value";
+#else
+  // __FILE__ and __LINE__ added
+  int Line = __LINE__;
+  OS << " at " << __FILE__ << ":" << (Line + 2) << "\n";
+  EXPECT_DEATH(llvm_cantFail(make_error<CustomError>(1)), OS.str())
+      << "cantFail(Error) did not cause an abort for failure value";
+#endif
+}
 
 // Test Checked Expected<T> in success mode.
 TEST(Error, CheckedExpectedInSuccessMode) {
   Expected<int> A = 7;
   EXPECT_TRUE(!!A) << "Expected with non-error value doesn't convert to 'true'";
   // Access is safe in second test, since we checked the error in the first.
   EXPECT_EQ(*A, 7) << "Incorrect Expected non-error value";
 }