Rework exiting system to have better backtraces. (#863)

Previously, the program exit was triggered by the destructor.
Unfortunately, C++ doesn't make it precisely clear where the destructor
is run, and Clang doesn't generate reliable debug information for that
to give good backtraces. Among other things, when combining separate
cleanup regions in Clang there may be no single canonical location.

Instead, move the ExitingStream system to use an explicit
low-precedence operator overload to flush the output and exit. This
ensures the stream and other actions are completed first but then
immediately exits the program in a way that has a definitive source
location and produces reliable backtraces.

I've tried to add comments and helpers to make this as clear as possible
given that it is a subtle and surprising issue.

Co-authored-by: Geoff Romer <gromer@google.com>

common/check.h

@@ -10,25 +10,25 @@
 #include "llvm/Support/raw_ostream.h"
 
 namespace Carbon {
+namespace Internal {
 
-// Wraps a stream and exiting for fatal errors.
+// Wraps a stream and exiting for fatal errors. Should only be used by the
+// macros below.
 class ExitingStream {
  public:
-  [[noreturn]] ~ExitingStream() {
-    // Finish with a newline.
-    llvm::errs() << "\n";
-    if (treat_as_bug) {
-      std::abort();
-    } else {
-      std::exit(-1);
-    }
-  }
+  // A tag type that renders as ": " in an ExitingStream, but only if it is
+  // followed by additional output. Otherwise, it renders as "". Primarily used
+  // when building macros around these streams.
+  struct AddSeparator {};
 
-  // Indicates that initial input is in, so this is where a ": " should be added
-  // before user input.
-  ExitingStream& AddSeparator() {
-    separator = true;
-    return *this;
+  // Internal type used in macros to dispatch to the `operator|` overload below.
+  struct Helper {};
+
+  [[noreturn]] ~ExitingStream() {
+    llvm_unreachable(
+        "Exiting streams should only be constructed with the below macros that "
+        "ensure the special operator| exits the program prior to their "
+        "destruction!");
   }
 
   // Indicates that the program is exiting due to a bug in the program, rather
@@ -53,6 +53,24 @@ class ExitingStream {
     return *this;
   }
 
+  ExitingStream& operator<<(AddSeparator /*unused*/) {
+    separator = true;
+    return *this;
+  }
+
+  // Low-precedence binary operator overload used in macros below to flush the
+  // output and exit the program. We do this in a binary operator rather than
+  // the destructor to ensure good debug info and backtraces for errors.
+  [[noreturn]] friend auto operator|(Helper /*unused*/, ExitingStream& rhs) {
+    // Finish with a newline.
+    llvm::errs() << "\n";
+    if (rhs.treat_as_bug) {
+      std::abort();
+    } else {
+      std::exit(-1);
+    }
+  }
+
  private:
   // Whether a separator should be printed if << is used again.
   bool separator = false;
@@ -61,26 +79,34 @@ class ExitingStream {
   bool treat_as_bug = false;
 };
 
+}  // namespace Internal
+
+// Raw exiting stream. This should be used when building other forms of exiting
+// macros like those below. It evaluates to a temporary `ExitingStream` object
+// that can be manipulated, streamed into, and then will exit the program.
+#define RAW_EXITING_STREAM() \
+  Carbon::Internal::ExitingStream::Helper() | Carbon::Internal::ExitingStream()
+
 // Checks the given condition, and if it's false, prints a stack, streams the
 // error message, then exits. This should be used for unexpected errors, such as
 // a bug in the application.
 //
 // For example:
 //   CHECK(is_valid) << "Data is not valid!";
-#define CHECK(condition)                                                 \
-  (!(condition)) &&                                                      \
-      (Carbon::ExitingStream() << "CHECK failure at " << __FILE__ << ":" \
-                               << __LINE__ << ": " #condition)           \
-          .AddSeparator()                                                \
-          .TreatAsBug()
+#define CHECK(condition)                                                  \
+  (condition) ? (void)0                                                   \
+              : RAW_EXITING_STREAM().TreatAsBug()                         \
+                    << "CHECK failure at " << __FILE__ << ":" << __LINE__ \
+                    << ": " #condition                                    \
+                    << Carbon::Internal::ExitingStream::AddSeparator()
 
 // This is similar to CHECK, but is unconditional. Writing FATAL() is clearer
 // than CHECK(false) because it avoids confusion about control flow.
 //
 // For example:
 //   FATAL() << "Unreachable!";
-#define FATAL()                        \
-  Carbon::ExitingStream().TreatAsBug() \
+#define FATAL()                     \
+  RAW_EXITING_STREAM().TreatAsBug() \
       << "FATAL failure at " << __FILE__ << ":" << __LINE__ << ": "
 
 }  // namespace Carbon

executable_semantics/common/error.h

@@ -21,19 +21,17 @@ namespace Carbon {
 // option is provided as a fallback for cases that don't fit those
 // classifications.
 
-#define FATAL_PROGRAM_ERROR_NO_LINE() \
-  Carbon::ExitingStream() << "PROGRAM ERROR: "
+#define FATAL_PROGRAM_ERROR_NO_LINE() RAW_EXITING_STREAM() << "PROGRAM ERROR: "
 
 #define FATAL_PROGRAM_ERROR(line) FATAL_PROGRAM_ERROR_NO_LINE() << line << ": "
 
 #define FATAL_COMPILATION_ERROR_NO_LINE() \
-  Carbon::ExitingStream() << "COMPILATION ERROR: "
+  RAW_EXITING_STREAM() << "COMPILATION ERROR: "
 
 #define FATAL_COMPILATION_ERROR(line) \
   FATAL_COMPILATION_ERROR_NO_LINE() << line << ": "
 
-#define FATAL_RUNTIME_ERROR_NO_LINE() \
-  Carbon::ExitingStream() << "RUNTIME ERROR: "
+#define FATAL_RUNTIME_ERROR_NO_LINE() RAW_EXITING_STREAM() << "RUNTIME ERROR: "
 
 #define FATAL_RUNTIME_ERROR(line) FATAL_RUNTIME_ERROR_NO_LINE() << line << ": "