Migrate remaining exits to FATAL_*_ERROR calls (#704)

Adds FATAL for things that are most likely programming errors in executable_semantics.

common/check.h

@@ -9,16 +9,12 @@
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 
-namespace CheckInternal {
+namespace Carbon {
 
-// Wraps a stream and exiting for CHECK.
-class ExitWrapper {
+// Wraps a stream and exiting for fatal errors.
+class ExitingStream {
  public:
-  ExitWrapper() {
-    // Start by printing a stack trace.
-    llvm::sys::PrintStackTrace(llvm::errs());
-  }
-  ~ExitWrapper() {
+  LLVM_ATTRIBUTE_NORETURN ~ExitingStream() {
     // Finish with a newline.
     llvm::errs() << "\n";
     exit(-1);
@@ -26,18 +22,24 @@ class ExitWrapper {
 
   // Indicates that initial input is in, so this is where a ": " should be added
   // before user input.
-  ExitWrapper& add_separator() {
+  ExitingStream& add_separator() {
     separator = true;
     return *this;
   }
 
+  // Prints a stack traces.
+  ExitingStream& print_stack() {
+    llvm::sys::PrintStackTrace(llvm::errs());
+    return *this;
+  }
+
   // If the bool cast occurs, it's because the condition is false. This supports
-  // && short-circuiting the creation of ExitWrapper.
+  // && short-circuiting the creation of ExitingStream.
   explicit operator bool() const { return true; }
 
   // Forward output to llvm::errs.
   template <typename T>
-  ExitWrapper& operator<<(const T& message) {
+  ExitingStream& operator<<(const T& message) {
     if (separator) {
       llvm::errs() << ": ";
       separator = false;
@@ -51,17 +53,23 @@ class ExitWrapper {
   bool separator = false;
 };
 
-}  // namespace CheckInternal
-
 // 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)) &&                                                  \
-      (CheckInternal::ExitWrapper() << "CHECK failure: " #condition) \
+#define CHECK(condition)                                                      \
+  (!(condition)) &&                                                           \
+      (Carbon::ExitingStream().print_stack() << "CHECK failure: " #condition) \
           .add_separator()
 
+// This is similar to CHECK, but is unconditional.
+//
+// For example:
+//   FATAL() << "Unreachable!";
+#define FATAL() Carbon::ExitingStream().print_stack() << "FATAL: "
+
+}  // namespace Carbon
+
 #endif  // COMMON_CHECK_H_

common/check_test.cpp

@@ -11,7 +11,7 @@ namespace Carbon {
 TEST(CheckTest, CheckTrue) { CHECK(true); }
 
 TEST(CheckTest, CheckFalse) {
-  ASSERT_DEATH({ CHECK(false); }, "CHECK failure: false");
+  ASSERT_DEATH({ CHECK(false); }, "\nCHECK failure: false\n");
 }
 
 TEST(CheckTest, CheckTrueCallbackNotUsed) {
@@ -25,7 +25,7 @@ TEST(CheckTest, CheckTrueCallbackNotUsed) {
 }
 
 TEST(CheckTest, CheckFalseMessage) {
-  ASSERT_DEATH({ CHECK(false) << "msg"; }, "CHECK failure: false: msg");
+  ASSERT_DEATH({ CHECK(false) << "msg"; }, "\nCHECK failure: false: msg\n");
 }
 
 TEST(CheckTest, CheckOutputForms) {
@@ -35,4 +35,14 @@ TEST(CheckTest, CheckOutputForms) {
   CHECK(true) << msg << str << i << 0;
 }
 
+TEST(CheckTest, Fatal) {
+  ASSERT_DEATH({ FATAL() << "msg"; }, "\nFATAL: msg\n");
+}
+
+auto FatalNoReturnRequired() -> int { FATAL() << "msg"; }
+
+TEST(ErrorTest, FatalNoReturnRequired) {
+  ASSERT_DEATH({ FatalNoReturnRequired(); }, "\nFATAL: msg\n");
+}
+
 }  // namespace Carbon

executable_semantics/ast/pattern.cpp

@@ -86,7 +86,7 @@ AlternativePattern::AlternativePattern(int line_num,
                                        const TuplePattern* arguments)
     : Pattern(Kind::AlternativePattern, line_num), arguments(arguments) {
   if (alternative->tag() != ExpressionKind::FieldAccessExpression) {
-    FATAL_USER_ERROR(alternative->line_num)
+    FATAL_PROGRAM_ERROR(alternative->line_num)
         << "Alternative pattern must have the form of a field access.";
   }
   const auto& field_access = alternative->GetFieldAccessExpression();

executable_semantics/common/BUILD

@@ -17,7 +17,7 @@ cc_library(
     name = "error",
     hdrs = ["error.h"],
     deps = [
-        "@llvm-project//llvm:Support",
+        "//common:check",
     ],
 )
 

executable_semantics/common/error.h

@@ -5,56 +5,35 @@
 #ifndef EXECUTABLE_SEMANTICS_COMMON_ERROR_H_
 #define EXECUTABLE_SEMANTICS_COMMON_ERROR_H_
 
-#include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/Signals.h"
-#include "llvm/Support/raw_ostream.h"
+#include "common/check.h"
 
 namespace Carbon {
 
-namespace ErrorInternal {
-
-// An error-printing stream that exits on destruction.
-class ExitingStream {
- public:
-  // Ends the error with a newline and exits.
-  LLVM_ATTRIBUTE_NORETURN virtual ~ExitingStream() {
-    llvm::errs() << "\n";
-    exit(-1);
-  }
-
-  // Forward output to llvm::errs.
-  template <typename T>
-  ExitingStream& operator<<(const T& message) {
-    llvm::errs() << message;
-    return *this;
-  }
-};
-
-}  // namespace ErrorInternal
-
 // Prints an error and exits. This should be used for non-recoverable errors
 // with user input.
 //
 // For example:
-//   FATAL_USER_ERROR(line_num) << "Line is bad!";
-//   FATAL_USER_ERROR_NO_LINE() << "Application is bad!";
+//   FATAL_PROGRAM_ERROR(line_num) << "Line is bad!";
+//   FATAL_PROGRAM_ERROR_NO_LINE() << "Application is bad!";
 //
-// Where possible, try to identify the error as a compilation error or runtime
-// error. The generic user error option is provided as a fallback for cases that
-// don't fit either of those classifications.
+// Where possible, try to identify the error as a compilation or
+// runtime error. Use CHECK/FATAL for internal errors. The generic program error
+// option is provided as a fallback for cases that don't fit those
+// classifications.
 
-#define FATAL_USER_ERROR_NO_LINE() ErrorInternal::ExitingStream() << "ERROR: "
+#define FATAL_PROGRAM_ERROR_NO_LINE() \
+  Carbon::ExitingStream() << "PROGRAM ERROR: "
 
-#define FATAL_USER_ERROR(line) FATAL_USER_ERROR_NO_LINE() << line << ": "
+#define FATAL_PROGRAM_ERROR(line) FATAL_PROGRAM_ERROR_NO_LINE() << line << ": "
 
 #define FATAL_COMPILATION_ERROR_NO_LINE() \
-  ErrorInternal::ExitingStream() << "COMPILATION ERROR: "
+  Carbon::ExitingStream() << "COMPILATION ERROR: "
 
 #define FATAL_COMPILATION_ERROR(line) \
   FATAL_COMPILATION_ERROR_NO_LINE() << line << ": "
 
 #define FATAL_RUNTIME_ERROR_NO_LINE() \
-  ErrorInternal::ExitingStream() << "RUNTIME ERROR: "
+  Carbon::ExitingStream() << "RUNTIME ERROR: "
 
 #define FATAL_RUNTIME_ERROR(line) FATAL_RUNTIME_ERROR_NO_LINE() << line << ": "
 

executable_semantics/common/error_test.cpp

@@ -9,28 +9,24 @@
 namespace Carbon {
 namespace {
 
-TEST(ErrorTest, FatalUserError) {
-  ASSERT_DEATH({ FATAL_RUNTIME_ERROR_NO_LINE() << "test"; }, "ERROR: test\n");
+TEST(ErrorTest, FatalProgramError) {
+  ASSERT_DEATH({ FATAL_PROGRAM_ERROR_NO_LINE() << "test"; },
+               "^PROGRAM ERROR: test\n");
 }
 
 TEST(ErrorTest, FatalRuntimeError) {
   ASSERT_DEATH({ FATAL_RUNTIME_ERROR_NO_LINE() << "test"; },
-               "RUNTIME ERROR: test\n");
+               "^RUNTIME ERROR: test\n");
 }
 
 TEST(ErrorTest, FatalCompilationError) {
   ASSERT_DEATH({ FATAL_COMPILATION_ERROR_NO_LINE() << "test"; },
-               "COMPILATION ERROR: test\n");
+               "^COMPILATION ERROR: test\n");
 }
 
-TEST(ErrorTest, FatalUserErrorLine) {
-  ASSERT_DEATH({ FATAL_USER_ERROR(1) << "test"; }, "ERROR: 1: test\n");
-}
-
-auto NoReturnRequired() -> int { FATAL_USER_ERROR_NO_LINE() << "test"; }
-
-TEST(ErrorTest, NoReturnRequired) {
-  ASSERT_DEATH({ NoReturnRequired(); }, "ERROR: test\n");
+TEST(ErrorTest, FatalProgramErrorLine) {
+  ASSERT_DEATH({ FATAL_PROGRAM_ERROR(1) << "test"; },
+               "^PROGRAM ERROR: 1: test\n");
 }
 
 }  // namespace

executable_semantics/interpreter/dictionary.h

@@ -5,7 +5,6 @@
 #ifndef EXECUTABLE_SEMANTICS_INTERPRETER_DICTIONARY_H_
 #define EXECUTABLE_SEMANTICS_INTERPRETER_DICTIONARY_H_
 
-#include <iostream>
 #include <list>
 #include <optional>
 #include <string>

executable_semantics/interpreter/heap.cpp

@@ -34,9 +34,8 @@ void Heap::Write(const Address& a, const Value* v, int line_num) {
 
 void Heap::CheckAlive(const Address& address, int line_num) {
   if (!alive_[address.index]) {
-    FATAL_RUNTIME_ERROR(line_num)
-        << ": undefined behavior: access to dead value "
-        << *values_[address.index];
+    FATAL_RUNTIME_ERROR(line_num) << "undefined behavior: access to dead value "
+                                  << *values_[address.index];
   }
 }
 

executable_semantics/interpreter/interpreter.cpp

@@ -99,8 +99,7 @@ auto EvalPrim(Operator op, const std::vector<const Value*>& args, int line_num)
     case Operator::Ptr:
       return global_arena->New<PointerType>(args[0]);
     case Operator::Deref:
-      llvm::errs() << line_num << ": dereference not implemented yet\n";
-      exit(-1);
+      FATAL() << "dereference not implemented yet";
   }
 }
 
@@ -298,10 +297,7 @@ auto PatternMatch(const Value* p, const Value* v, Env values,
           return values;
         }
         default:
-          llvm::errs()
-              << "internal error, expected a tuple value in pattern, not " << *v
-              << "\n";
-          exit(-1);
+          FATAL() << "expected a tuple value in pattern, not " << *v;
       }
     case Value::Kind::AlternativeValue:
       switch (v->Tag()) {
@@ -320,11 +316,7 @@ auto PatternMatch(const Value* p, const Value* v, Env values,
           return *matches;
         }
         default:
-          llvm::errs()
-              << "internal error, expected a choice alternative in pattern, "
-                 "not "
-              << *v << "\n";
-          exit(-1);
+          FATAL() << "expected a choice alternative in pattern, not " << *v;
       }
     case Value::Kind::FunctionType:
       switch (v->Tag()) {
@@ -377,11 +369,7 @@ void PatternAssignment(const Value* pat, const Value* val, int line_num) {
           break;
         }
         default:
-          llvm::errs()
-              << "internal error, expected a tuple value on right-hand-side, "
-                 "not "
-              << *val << "\n";
-          exit(-1);
+          FATAL() << "expected a tuple value on right-hand-side, not " << *val;
       }
       break;
     }
@@ -397,11 +385,7 @@ void PatternAssignment(const Value* pat, const Value* val, int line_num) {
           break;
         }
         default:
-          llvm::errs()
-              << "internal error, expected an alternative in left-hand-side, "
-                 "not "
-              << *val << "\n";
-          exit(-1);
+          FATAL() << "expected an alternative in left-hand-side, not " << *val;
       }
       break;
     }
@@ -428,8 +412,7 @@ void StepLvalue() {
           CurrentEnv(state).Get(exp->GetIdentifierExpression().name);
       if (!pointer) {
         FATAL_RUNTIME_ERROR(exp->line_num)
-            << ": could not find `" << exp->GetIdentifierExpression().name
-            << "`";
+            << "could not find `" << exp->GetIdentifierExpression().name << "`";
       }
       const Value* v = global_arena->New<PointerValue>(*pointer);
       frame->todo.Pop();
@@ -610,8 +593,7 @@ void StepExp() {
           CurrentEnv(state).Get(exp->GetIdentifierExpression().name);
       if (!pointer) {
         FATAL_RUNTIME_ERROR(exp->line_num)
-            << ": could not find `" << exp->GetIdentifierExpression().name
-            << "`";
+            << "could not find `" << exp->GetIdentifierExpression().name << "`";
       }
       const Value* pointee = state->heap.Read(*pointer, exp->line_num);
       frame->todo.Pop(1);
@@ -670,8 +652,7 @@ void StepExp() {
         frame->todo.Pop(1);
         CallFunction(exp->line_num, act->results, state);
       } else {
-        llvm::errs() << "internal error in handle_value with Call\n";
-        exit(-1);
+        FATAL() << "in handle_value with Call pos " << act->pos;
       }
       break;
     case ExpressionKind::IntTypeLiteral: {

executable_semantics/interpreter/typecheck.cpp

@@ -82,8 +82,7 @@ auto ReifyType(const Value* t, int line_num) -> const Expression* {
       return Expression::MakeIdentifierExpression(
           0, cast<VariableType>(*t).Name());
     default:
-      llvm::errs() << line_num << ": expected a type, not " << *t << "\n";
-      exit(-1);
+      FATAL() << "expected a type, not " << *t;
   }
 }
 
@@ -117,10 +116,9 @@ auto ArgumentDeduction(int line_num, TypeEnv deduced, const Value* param,
       }
       for (size_t i = 0; i < param_tup.Elements().size(); ++i) {
         if (param_tup.Elements()[i].name != arg_tup.Elements()[i].name) {
-          std::cerr << line_num << ": mismatch in tuple names, "
-                    << param_tup.Elements()[i].name
-                    << " != " << arg_tup.Elements()[i].name << std::endl;
-          exit(-1);
+          FATAL_COMPILATION_ERROR(line_num)
+              << "mismatch in tuple names, " << param_tup.Elements()[i].name
+              << " != " << arg_tup.Elements()[i].name;
         }
         deduced =
             ArgumentDeduction(line_num, deduced, param_tup.Elements()[i].value,
@@ -173,10 +171,7 @@ auto ArgumentDeduction(int line_num, TypeEnv deduced, const Value* param,
     case Value::Kind::BindingPlaceholderValue:
     case Value::Kind::AlternativeConstructorValue:
     case Value::Kind::ContinuationValue:
-      llvm::errs() << line_num
-                   << ": internal error in ArgumentDeduction: expected type, "
-                   << "not value " << *param << "\n";
-      exit(-1);
+      FATAL() << "In ArgumentDeduction: expected type, not value " << *param;
   }
 }
 
@@ -228,9 +223,7 @@ auto Substitute(TypeEnv dict, const Value* type) -> const Value* {
     case Value::Kind::BindingPlaceholderValue:
     case Value::Kind::AlternativeConstructorValue:
     case Value::Kind::ContinuationValue:
-      llvm::errs() << "internal error in Substitute: expected type, "
-                   << "not value " << *type << "\n";
-      exit(-1);
+      FATAL() << "In Substitute: expected type, not value " << *type;
   }
 }
 
@@ -447,11 +440,9 @@ auto TypeCheckExp(const Expression* e, TypeEnv types, Env values)
               // TODO: change the following to a CHECK once the real checking
               // has been added to the type checking of function signatures.
               if (!deduced_args.Get(deduced_param.name)) {
-                std::cerr << e->line_num
-                          << ": error, could not deduce type argument for type "
-                             "parameter "
-                          << deduced_param.name << std::endl;
-                exit(-1);
+                FATAL_COMPILATION_ERROR(e->line_num)
+                    << "could not deduce type argument for type parameter "
+                    << deduced_param.name;
               }
             }
             parameter_type = Substitute(deduced_args, parameter_type);

executable_semantics/interpreter/value.cpp

@@ -81,8 +81,7 @@ auto GetMember(const Value* v, const std::string& f, int line_num)
       return global_arena->New<AlternativeConstructorValue>(f, choice.Name());
     }
     default:
-      llvm::errs() << "field access not allowed for value " << *v << "\n";
-      exit(-1);
+      FATAL() << "field access not allowed for value " << *v;
   }
 }
 
@@ -126,8 +125,7 @@ auto SetFieldImpl(const Value* value,
       return global_arena->New<TupleValue>(elements);
     }
     default:
-      llvm::errs() << "field access not allowed for value " << *value << "\n";
-      exit(-1);
+      FATAL() << "field access not allowed for value " << *value;
   }
 }
 
@@ -345,10 +343,9 @@ auto TypeEqual(const Value* t1, const Value* t2) -> bool {
     case Value::Kind::VariableType:
       return cast<VariableType>(*t1).Name() == cast<VariableType>(*t2).Name();
     default:
-      llvm::errs() << "TypeEqual used to compare non-type values\n"
-                   << *t1 << "\n"
-                   << *t2 << "\n";
-      exit(-1);
+      FATAL() << "TypeEqual used to compare non-type values\n"
+              << *t1 << "\n"
+              << *t2;
   }
 }
 
@@ -393,7 +390,6 @@ auto ValueEqual(const Value* v1, const Value* v2, int line_num) -> bool {
     case Value::Kind::TupleValue:
       return FieldsValueEqual(cast<TupleValue>(*v1).Elements(),
                               cast<TupleValue>(*v2).Elements(), line_num);
-    default:
     case Value::Kind::IntType:
     case Value::Kind::BoolType:
     case Value::Kind::TypeType:
@@ -403,14 +399,14 @@ auto ValueEqual(const Value* v1, const Value* v2, int line_num) -> bool {
     case Value::Kind::StructType:
     case Value::Kind::ChoiceType:
     case Value::Kind::ContinuationType:
+    case Value::Kind::VariableType:
       return TypeEqual(v1, v2);
     case Value::Kind::StructValue:
     case Value::Kind::AlternativeValue:
     case Value::Kind::BindingPlaceholderValue:
     case Value::Kind::AlternativeConstructorValue:
     case Value::Kind::ContinuationValue:
-      llvm::errs() << "ValueEqual does not support this kind of value.\n";
-      exit(-1);
+      FATAL() << "ValueEqual does not support this kind of value: " << *v1;
   }
 }
 

executable_semantics/syntax/lexer.lpp

@@ -6,7 +6,6 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 %{
 #include <cstdlib>
-#include <iostream>
 
 #include "common/check.h"
 #include "executable_semantics/common/tracing_flag.h"
@@ -215,11 +214,10 @@ operator and its operand, leading to three more cases:
   if (Carbon::tracing_output) {
     // Print a newline because tracing prints an incomplete line
     // "Reading a token: ".
-    std::cerr << std::endl;
+    llvm::errs() << "\n";
   }
-  std::cerr << context.current_token_position << ": invalid character '\\x"
-            << llvm::toHex(llvm::StringRef(yytext, 1)) << "' in source file." << std::endl;
-  std::exit(1);
+  FATAL_COMPILATION_ERROR(yylineno) << "invalid character '\\x"
+            << llvm::toHex(llvm::StringRef(yytext, 1)) << "' in source file.";
 }
 
 <<EOF>>    {

executable_semantics/syntax/paren_contents.h

@@ -71,7 +71,7 @@ auto ParenContents<Term>::TupleElements(int line_num) const
       result.push_back(TupleElement(*element.name, element.term));
     } else {
       if (seen_named_member) {
-        FATAL_USER_ERROR(line_num)
+        FATAL_PROGRAM_ERROR(line_num)
             << "positional members must come before named members";
       }
       result.push_back(TupleElement(std::to_string(i), element.term));

executable_semantics/syntax/parse.cpp

@@ -4,8 +4,6 @@
 
 #include "executable_semantics/syntax/parse.h"
 
-#include <iostream>
-
 #include "common/check.h"
 #include "executable_semantics/common/error.h"
 #include "executable_semantics/common/tracing_flag.h"
@@ -23,8 +21,8 @@ auto parse(const std::string& input_file_name)
     -> std::variant<AST, SyntaxErrorCode> {
   yyin = fopen(input_file_name.c_str(), "r");
   if (yyin == nullptr) {
-    FATAL_USER_ERROR_NO_LINE() << "Error opening '" << input_file_name
-                               << "': " << std::strerror(errno);
+    FATAL_PROGRAM_ERROR_NO_LINE() << "Error opening '" << input_file_name
+                                  << "': " << std::strerror(errno);
   }
 
   std::optional<AST> parsed_input = std::nullopt;

executable_semantics/syntax/parse_and_lex_context.cpp

@@ -4,9 +4,6 @@
 
 #include "executable_semantics/syntax/parse_and_lex_context.h"
 
-#include <cstring>
-#include <iostream>
-
 // Writes a syntax error diagnostic, containing message, for the input file at
 // the given line, to standard error.
 auto Carbon::ParseAndLexContext::PrintDiagnostic(const std::string& message,
@@ -14,7 +11,5 @@ auto Carbon::ParseAndLexContext::PrintDiagnostic(const std::string& message,
   // TODO: Do we really want this to be fatal?  It makes the comment and the
   // name a lie, and renders some of the other yyparse() result propagation code
   // moot.
-  std::cerr << input_file_name << ":" << line_num << ": " << message
-            << std::endl;
-  exit(-1);
+  FATAL_COMPILATION_ERROR(line_num) << message;
 }

executable_semantics/syntax/parser.ypp

@@ -51,7 +51,6 @@
 #include <cstdarg>
 #include <cstdio>
 #include <cstdlib>
-#include <iostream>
 #include <list>
 #include <vector>
 

executable_semantics/testdata/experimental_continuation9.golden

@@ -1,2 +1,2 @@
-RUNTIME ERROR: 12: : undefined behavior: access to dead value 1
+RUNTIME ERROR: 12: undefined behavior: access to dead value 1
 EXIT CODE: 255

executable_semantics/testdata/fun_named_params2.golden

@@ -1,2 +1,2 @@
-ERROR: 5: positional members must come before named members
+PROGRAM ERROR: 5: positional members must come before named members
 EXIT CODE: 255

executable_semantics/testdata/generic_function_fail2.golden

@@ -1,2 +1,2 @@
-10: error, could not deduce type argument for type parameter T
+COMPILATION ERROR: 10: could not deduce type argument for type parameter T
 EXIT CODE: 255

executable_semantics/testdata/global_variable8.golden

@@ -1,2 +1,2 @@
-RUNTIME ERROR: 8: : could not find `y`
+RUNTIME ERROR: 8: could not find `y`
 EXIT CODE: 255

executable_semantics/testdata/invalid_char.golden

@@ -1,2 +1,2 @@
-1.1: invalid character '\xFE' in source file.
-EXIT CODE: 1
+COMPILATION ERROR: 1: invalid character '\xFE' in source file.
+EXIT CODE: 255

executable_semantics/testdata/pattern_variable_fail.golden

@@ -1,2 +1,2 @@
-executable_semantics/testdata/pattern_variable_fail.carbon:7: syntax error, unexpected :
+COMPILATION ERROR: 7: syntax error, unexpected :
 EXIT CODE: 255

executable_semantics/testdata/tuple4.golden

@@ -1,2 +1,2 @@
-ERROR: 6: positional members must come before named members
+PROGRAM ERROR: 6: positional members must come before named members
 EXIT CODE: 255