Replaced std::exit() with return Carbon::ErrorOr for expected errors like invalid syntax (#1120)

* Replaced std::exit() with return llvm::Expected/llvm::Error<T> for expected errors like invalid syntax.

* Use llvm::formatv() for formatting lexer error messages.
x

* Addresed merge errors.

* Fixed impl scope.

* Made ErrorBuilder::operator<< nodiscard, to catch code forgetting 'return' in 'return FATAL_COMPILATION_ERROR()'.

* FatalComplationError() -> ParseAndLexContext::RecordLexerError().
Other usages of ERROR_TOKEN in lexer.lpp were actually supposed to be END_OF_FILE.

* Update executable_semantics/syntax/parse_and_lex_context.h

Co-authored-by: Jon Meow <jperkins@google.com>

* Code review fixes.

* Update executable_semantics/syntax/parser.ypp

Co-authored-by: Jon Meow <jperkins@google.com>

* More code review fixes.

* Update executable_semantics/interpreter/type_checker.h

Co-authored-by: Jon Meow <jperkins@google.com>

* Yet more code review fixes...

* Update executable_semantics/syntax/lexer.lpp

Co-authored-by: Jon Meow <jperkins@google.com>

* code review comments

* Update executable_semantics/interpreter/interpreter.cpp

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

* Apply suggestions from code review

Co-authored-by: Jon Meow <jperkins@google.com>

* Update executable_semantics/syntax/lexer.lpp

Co-authored-by: Jon Meow <jperkins@google.com>

* code review

* code review

* Apply suggestions from code review

Co-authored-by: Jon Meow <jperkins@google.com>

* formatted code

* review comments

* Switched to the new ErrorOr<V> error implementation

* code review comments

* fixed comment

* restored ostream.h as #976 makes the change unnecesary

* review comments

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

executable_semantics/syntax/BUILD

@@ -30,6 +30,7 @@ cc_library(
     deps = [
         ":syntax",
         "@com_google_googletest//:gtest",
+        "@llvm-project//llvm:Support",
     ],
 )
 
@@ -57,6 +58,7 @@ cc_library(
     deps = [
         ":bison_wrap",
         "//common:check",
+        "//common:error",
         "//common:ostream",
         "//common:string_helpers",
         "//executable_semantics/ast",

executable_semantics/syntax/lexer.lpp

@@ -8,20 +8,15 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
   #include <cstdlib>
 
   #include "common/check.h"
+  #include "common/error.h"
   #include "common/string_helpers.h"
   #include "executable_semantics/syntax/parse_and_lex_context.h"
   #include "executable_semantics/syntax/parser.h"
   #include "llvm/ADT/StringExtras.h"
+  #include "llvm/Support/FormatVariadic.h"
 
-  // Prints a newline in trace mode because trace prints an incomplete line
-  // "Reading a token: " which can prevent LIT from finding expected patterns.
-  #define FATAL_SYNTAX_ERROR(context)                                       \
-    RAW_EXITING_STREAM() << (context.trace() ? "\n" : "")                   \
-                         << "COMPILATION ERROR: " << (context.source_loc()) \
-                         << ": "
-
-  // Reads and returns a single character. Fails on EOF.
-  char ReadChar(yyscan_t yyscanner, const Carbon::ParseAndLexContext& context);
+  // Reads and returns a single character. Reports an error on EOF.
+  auto ReadChar(yyscan_t yyscanner, Carbon::ParseAndLexContext& context) -> int;
 %}
 
 /* Turn off legacy bits we don't need. */
@@ -258,7 +253,13 @@ string_literal        \"([^\\\"\n\v\f\r]|\\.)*\"
 
 {intrinsic_identifier} {
   BEGIN(AFTER_OPERAND);
-  return ARG_TOKEN(intrinsic_identifier, yytext);
+  Carbon::ErrorOr<Carbon::IntrinsicExpression::Intrinsic> intrinsic =
+      Carbon::IntrinsicExpression::FindIntrinsic(yytext, context.source_loc());
+  if (intrinsic.ok()) {
+    return ARG_TOKEN(intrinsic_identifier, *intrinsic);
+  } else {
+    return context.RecordSyntaxError(intrinsic.error().message());
+  }
 }
 
 {identifier} {
@@ -270,7 +271,8 @@ string_literal        \"([^\\\"\n\v\f\r]|\\.)*\"
   BEGIN(AFTER_OPERAND);
   int val = 0;
   if (!llvm::to_integer(yytext, val)) {
-    FATAL_SYNTAX_ERROR(context) << "Invalid integer literal: " << yytext;
+    return context.RecordSyntaxError(
+        llvm::formatv("Invalid integer literal: {0}", yytext));
   }
   return ARG_TOKEN(integer_literal, val);
 }
@@ -280,7 +282,8 @@ string_literal        \"([^\\\"\n\v\f\r]|\\.)*\"
   CHECK(str.consume_front("\"") && str.consume_back("\""));
   std::optional<std::string> unescaped = Carbon::UnescapeStringLiteral(str);
   if (unescaped == std::nullopt) {
-    FATAL_SYNTAX_ERROR(context) << "Invalid escaping in string: " << yytext;
+    return context.RecordSyntaxError(
+        llvm::formatv("Invalid escaping in string: {0}", yytext));
   }
   return ARG_TOKEN(string_literal, *unescaped);
 }
@@ -291,24 +294,37 @@ string_literal        \"([^\\\"\n\v\f\r]|\\.)*\"
   // Scans for the closing """, checking for possible escape sequences
   // like \""".
   for (;;) {
-    char c = ReadChar(yyscanner, context);
+    int c = ReadChar(yyscanner, context);
+    if (c <= 0) {
+      return SIMPLE_TOKEN(END_OF_FILE);
+    }
     s.push_back(c);
     if (c != '"' && c != '\\') {
       continue;
     }
     if (c == '\\') {
       // \" in \""" is not a terminator.
-      s.push_back(ReadChar(yyscanner, context));
+      c = ReadChar(yyscanner, context);
+      if (c <= 0) {
+        return SIMPLE_TOKEN(END_OF_FILE);
+      }
+      s.push_back(c);
       continue;
     }
 
     c = ReadChar(yyscanner, context);
+    if (c <= 0) {
+      return SIMPLE_TOKEN(END_OF_FILE);
+    }
     s.push_back(c);
     if (c != '"') {
       continue;
     }
 
     c = ReadChar(yyscanner, context);
+    if (c <= 0) {
+      return SIMPLE_TOKEN(END_OF_FILE);
+    }
     s.push_back(c);
     if (c == '"') {
       break;
@@ -317,8 +333,8 @@ string_literal        \"([^\\\"\n\v\f\r]|\\.)*\"
   Carbon::ErrorOr<std::string> block_string =
       Carbon::ParseBlockStringLiteral(s);
   if (!block_string.ok()) {
-    FATAL_SYNTAX_ERROR(context)
-        << "Invalid block string: " << block_string.error();
+    return context.RecordSyntaxError(llvm::formatv(
+        "Invalid block string: {0}", block_string.error().message()));
   }
   return ARG_TOKEN(string_literal, *block_string);
 }
@@ -345,17 +361,17 @@ string_literal        \"([^\\\"\n\v\f\r]|\\.)*\"
 }
 
 . {
-  FATAL_SYNTAX_ERROR(context)
-      << "invalid character '\\x" << llvm::toHex(llvm::StringRef(yytext, 1))
-      << "' in source file.";
+  return context.RecordSyntaxError(
+      llvm::formatv("invalid character '\\x{0}' in source file.",
+                    llvm::toHex(llvm::StringRef(yytext, 1))));
 }
 
 %%
 
-char ReadChar(yyscan_t yyscanner, const Carbon::ParseAndLexContext& context) {
+auto ReadChar(yyscan_t yyscanner, Carbon::ParseAndLexContext& context) -> int {
   const int c = yyinput(yyscanner);
-  if (c == EOF) {
-    FATAL_SYNTAX_ERROR(context) << "Unexpected end of file";
+  if (c <= 0) {
+    context.RecordSyntaxError("Unexpected end of file");
   }
   return c;
 }

executable_semantics/syntax/parse.cpp

@@ -5,16 +5,17 @@
 #include "executable_semantics/syntax/parse.h"
 
 #include "common/check.h"
+#include "common/error.h"
 #include "executable_semantics/common/error.h"
 #include "executable_semantics/syntax/lexer.h"
 #include "executable_semantics/syntax/parse_and_lex_context.h"
 #include "executable_semantics/syntax/parser.h"
+#include "llvm/Support/Error.h"
 
 namespace Carbon {
 
 auto ParseImpl(yyscan_t scanner, Nonnull<Arena*> arena,
-               std::string_view input_file_name, bool trace)
-    -> std::variant<AST, SyntaxErrorCode> {
+               std::string_view input_file_name, bool trace) -> ErrorOr<AST> {
   // Prepare other parser arguments.
   std::optional<AST> ast = std::nullopt;
   ParseAndLexContext context(arena->New<std::string>(input_file_name), trace);
@@ -24,11 +25,12 @@ auto ParseImpl(yyscan_t scanner, Nonnull<Arena*> arena,
   if (trace) {
     parser.set_debug_level(1);
   }
-  auto syntax_error_code = parser();
 
-  // Return an error if appropriate.
-  if (syntax_error_code != 0) {
-    return syntax_error_code;
+  if (auto syntax_error_code = parser(); syntax_error_code != 0) {
+    const std::string error_message = context.error_messages().empty()
+                                          ? "Unknown parser error"
+                                          : context.error_messages()[0];
+    return Error(error_message);
   }
 
   // Return parse results.
@@ -38,11 +40,11 @@ auto ParseImpl(yyscan_t scanner, Nonnull<Arena*> arena,
 }
 
 auto Parse(Nonnull<Arena*> arena, std::string_view input_file_name, bool trace)
-    -> std::variant<AST, SyntaxErrorCode> {
+    -> ErrorOr<AST> {
   FILE* input_file = fopen(std::string(input_file_name).c_str(), "r");
   if (input_file == nullptr) {
-    FATAL_PROGRAM_ERROR_NO_LINE() << "Error opening '" << input_file_name
-                                  << "': " << std::strerror(errno);
+    return FATAL_PROGRAM_ERROR_NO_LINE() << "Error opening '" << input_file_name
+                                         << "': " << std::strerror(errno);
   }
 
   // Prepare the lexer.
@@ -51,8 +53,7 @@ auto Parse(Nonnull<Arena*> arena, std::string_view input_file_name, bool trace)
   auto buffer = yy_create_buffer(input_file, YY_BUF_SIZE, scanner);
   yy_switch_to_buffer(buffer, scanner);
 
-  std::variant<AST, SyntaxErrorCode> result =
-      ParseImpl(scanner, arena, input_file_name, trace);
+  ErrorOr<AST> result = ParseImpl(scanner, arena, input_file_name, trace);
 
   // Clean up the lexer.
   yy_delete_buffer(buffer, scanner);
@@ -64,7 +65,7 @@ auto Parse(Nonnull<Arena*> arena, std::string_view input_file_name, bool trace)
 
 auto ParseFromString(Nonnull<Arena*> arena, std::string_view input_file_name,
                      std::string_view file_contents, bool trace)
-    -> std::variant<Carbon::AST, SyntaxErrorCode> {
+    -> ErrorOr<AST> {
   // Prepare the lexer.
   yyscan_t scanner;
   yylex_init(&scanner);
@@ -72,8 +73,7 @@ auto ParseFromString(Nonnull<Arena*> arena, std::string_view input_file_name,
       yy_scan_bytes(file_contents.data(), file_contents.size(), scanner);
   yy_switch_to_buffer(buffer, scanner);
 
-  std::variant<AST, SyntaxErrorCode> result =
-      ParseImpl(scanner, arena, input_file_name, trace);
+  ErrorOr<AST> result = ParseImpl(scanner, arena, input_file_name, trace);
 
   // Clean up the lexer.
   yy_delete_buffer(buffer, scanner);

executable_semantics/syntax/parse.h

@@ -13,20 +13,17 @@
 
 namespace Carbon {
 
-// This is the code given us by Bison, for now.
-using SyntaxErrorCode = int;
-
 // Returns the AST representing the contents of the named file, or an error code
 // if parsing fails. Allocations go into the provided arena.
 auto Parse(Nonnull<Arena*> arena, std::string_view input_file_name, bool trace)
-    -> std::variant<Carbon::AST, SyntaxErrorCode>;
+    -> ErrorOr<Carbon::AST>;
 
 // Equivalent to `Parse`, but parses the contents of `file_contents`.
 // `input_file_name` is used only for reporting source locations, and does
 // not need to name a real file.
 auto ParseFromString(Nonnull<Arena*> arena, std::string_view input_file_name,
                      std::string_view file_contents, bool trace)
-    -> std::variant<Carbon::AST, SyntaxErrorCode>;
+    -> ErrorOr<Carbon::AST>;
 
 }  // namespace Carbon
 

executable_semantics/syntax/parse_and_lex_context.cpp

@@ -6,11 +6,21 @@
 
 namespace Carbon {
 
-auto ParseAndLexContext::PrintDiagnostic(const std::string& message) -> void {
-  // 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.
-  FATAL_COMPILATION_ERROR(source_loc()) << message;
+auto ParseAndLexContext::RecordSyntaxError(const std::string& message,
+                                           bool prefix_with_newline)
+    -> Parser::symbol_type {
+  // Optionally adds a newline in trace mode because trace prints an incomplete
+  // line "Reading a token: " which can prevent LIT from finding expected
+  // patterns.
+  // TODO: support formatting of `SourceLocation` instances with formatv().
+  std::string full_message;
+  llvm::raw_string_ostream(full_message)
+      << (prefix_with_newline && trace() ? "\n" : "")
+      << "COMPILATION ERROR: " << source_loc() << ": " << message;
+  error_messages_.push_back(full_message);
+
+  // TODO: use `YYerror` token once bison is upgraded to at least 3.5.
+  return Parser::make_END_OF_FILE(current_token_position);
 }
 
 }  // namespace Carbon

executable_semantics/syntax/parse_and_lex_context.h

@@ -20,8 +20,11 @@ class ParseAndLexContext {
   ParseAndLexContext(Nonnull<const std::string*> input_file_name, bool trace)
       : input_file_name_(input_file_name), trace_(trace) {}
 
-  // Writes a syntax error diagnostic containing message to standard error.
-  auto PrintDiagnostic(const std::string& message) -> void;
+  // Formats ands records a lexer error. Returns an error token as a
+  // convenience.
+  auto RecordSyntaxError(const std::string& message,
+                         bool prefix_with_newline = false)
+      -> Parser::symbol_type;
 
   auto source_loc() const -> SourceLocation {
     return SourceLocation(input_file_name_,
@@ -33,12 +36,18 @@ class ParseAndLexContext {
   // The source range of the token being (or just) lex'd.
   location current_token_position;
 
+  auto error_messages() const -> const std::vector<std::string> {
+    return error_messages_;
+  }
+
  private:
   // A path to the file processed, relative to the current working directory
   // when *this is called.
   Nonnull<const std::string*> input_file_name_;
 
   bool trace_;
+
+  std::vector<std::string> error_messages_;
 };
 
 }  // namespace Carbon

executable_semantics/syntax/parse_test.cpp

@@ -23,10 +23,10 @@ fn Foo() {}
 
 TEST(ParseTest, ParseFromString) {
   Arena arena;
-  std::variant<AST, SyntaxErrorCode> parse_result =
+  ErrorOr<AST> parse_result =
       ParseFromString(&arena, "file.carbon", FileContents, /*trace=*/false);
-  ASSERT_TRUE(std::holds_alternative<AST>(parse_result));
-  EXPECT_EQ(std::get<AST>(parse_result).declarations.size(), 1);
+  ASSERT_TRUE(parse_result.ok());
+  EXPECT_EQ(parse_result->declarations.size(), 1);
 }
 
 }  // namespace

executable_semantics/syntax/parse_test_matchers_internal.h

@@ -12,6 +12,8 @@
 #include <variant>
 
 #include "executable_semantics/syntax/parse.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace Carbon::TestingInternal {
 
@@ -32,14 +34,14 @@ class ParsedAsMatcher {
     DescribeToImpl(out, /*negated=*/true);
   }
 
-  auto MatchAndExplain(const std::variant<AST, SyntaxErrorCode>& result,
+  auto MatchAndExplain(const ErrorOr<AST>& result,
                        ::testing::MatchResultListener* listener) const -> bool {
-    if (std::holds_alternative<SyntaxErrorCode>(result)) {
-      *listener << "holds error code " << std::get<SyntaxErrorCode>(result);
+    if (!result.ok()) {
+      *listener << "is a failed parse with error: " << result.error().message();
       return false;
     } else {
       *listener << "is a successful parse whose ";
-      return ast_matcher_.MatchAndExplain(std::get<AST>(result), listener);
+      return ast_matcher_.MatchAndExplain(*result, listener);
     }
   }
 

executable_semantics/syntax/parser.ypp

@@ -62,6 +62,7 @@
   #include "common/check.h"
   #include "executable_semantics/syntax/parse_and_lex_context.h"
   #include "llvm/ADT/StringExtras.h"
+  #include "llvm/Support/raw_ostream.h"
 }  // %code top
 
 %code requires {
@@ -86,13 +87,13 @@
 
 %code {
   void Carbon::Parser::error(const location_type&, const std::string& message) {
-    context.PrintDiagnostic(message);
+    context.RecordSyntaxError(message);
   }
 }  // %code
 
 %token <int> integer_literal
 %token <std::string> identifier
-%token <std::string> intrinsic_identifier
+%token <IntrinsicExpression::Intrinsic> intrinsic_identifier
 %token <std::string> sized_type_literal
 %token <std::string> string_literal
 %type <std::string> designator
@@ -582,7 +583,16 @@ non_expression_pattern:
 | paren_pattern
     { $$ = $1; }
 | postfix_expression tuple_pattern
-    { $$ = arena->New<AlternativePattern>(context.source_loc(), $1, $2); }
+    {
+      ErrorOr<Nonnull<AlternativePattern*>> alternative_pattern =
+          AlternativePattern::Create(arena, context.source_loc(), $1, $2);
+      if (alternative_pattern.ok()) {
+        $$ = *alternative_pattern;
+      } else {
+        context.RecordSyntaxError(alternative_pattern.error().message());
+        YYERROR;
+      }
+    }
 | VAR non_expression_pattern
     { $$ = arena->New<VarPattern>(context.source_loc(), $2); }
 ;
@@ -804,13 +814,25 @@ receiver:
 function_declaration:
   FN identifier deduced_params receiver maybe_empty_tuple_pattern return_term block
     {
-      $$ = arena->New<FunctionDeclaration>(context.source_loc(), $2, $3, $4, $5,
-                                           $6, $7);
+      ErrorOr<FunctionDeclaration*> fn = FunctionDeclaration::Create(
+          arena, context.source_loc(), $2, $3, $4, $5, $6, $7);
+      if (fn.ok()) {
+        $$ = *fn;
+      } else {
+        context.RecordSyntaxError(fn.error().message());
+        YYERROR;
+      }
     }
 | FN identifier deduced_params receiver maybe_empty_tuple_pattern return_term SEMICOLON
     {
-      $$ = arena->New<FunctionDeclaration>(context.source_loc(), $2, $3, $4, $5,
-                                           $6, std::nullopt);
+      ErrorOr<FunctionDeclaration*> fn = FunctionDeclaration::Create(
+          arena, context.source_loc(), $2, $3, $4, $5, $6, std::nullopt);
+      if (fn.ok()) {
+        $$ = *fn;
+      } else {
+        context.RecordSyntaxError(fn.error().message());
+        YYERROR;
+      }
     }
 ;
 variable_declaration: identifier COLON pattern