Revert "[torque] Throw exception instead of aborting if something goes wrong"

This reverts commit 3bd49f9b.

Reason for revert: Build failure on Win Bot

Original change's description:
> [torque] Throw exception instead of aborting if something goes wrong
> 
> This CL enables exceptions for the Torque compiler and Torque language
> server. Instead of aborting when something goes wrong during
> compilation, a TorqueError is thrown, containing the error message
> and a source position. The compiler executable still prints the error
> and aborts, while the language server will pass this information
> along to the client (not included in this CL).
> 
> R=​danno@chromium.org
> 
> Bug: v8:8880
> Change-Id: Iad83c46fb6a91c1babbc0ae7dbd94fbe4e7f1663
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1526003
> Reviewed-by: Daniel Clifford <danno@chromium.org>
> Commit-Queue: Simon Zünd <szuend@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#60512}

TBR=danno@chromium.org,mvstanton@chromium.org,szuend@chromium.org

Change-Id: Iac253da404eaf6e534267f59a42ca93667c205e4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8880
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1543849Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60513}

BUILD.gn

@@ -3225,13 +3225,9 @@ v8_source_set("torque_base") {
     ":v8_shared_internal_headers",
   ]
 
-  configs = [
-    ":internal_config",
-    "//build/config/compiler:exceptions",
-  ]
-  remove_configs = [ "//build/config/compiler:no_exceptions" ]
+  configs = [ ":internal_config" ]
   if (is_win && is_asan) {
-    remove_configs += [ "//build/config/sanitizers:default_sanitizer_flags" ]
+    remove_configs = [ "//build/config/sanitizers:default_sanitizer_flags" ]
   }
 }
 
@@ -3253,13 +3249,9 @@ v8_source_set("torque_ls_base") {
     ":torque_base",
   ]
 
-  configs = [
-    ":internal_config",
-    "//build/config/compiler:exceptions",
-  ]
-  remove_configs = [ "//build/config/compiler:no_exceptions" ]
+  configs = [ ":internal_config" ]
   if (is_win && is_asan) {
-    remove_configs += [ "//build/config/sanitizers:default_sanitizer_flags" ]
+    remove_configs = [ "//build/config/sanitizers:default_sanitizer_flags" ]
   }
 }
 

src/torque/ls/json-parser.cc

@@ -180,19 +180,13 @@ class JsonGrammar : public Grammar {
   Symbol file = {Rule({&value})};
 };
 
-JsonParserResult ParseJson(const std::string& input) {
+JsonValue ParseJson(const std::string& input) {
   // Torque needs a CurrentSourceFile scope during parsing.
   // As JSON lives in memory only, a unknown file scope is created.
   SourceFileMap::Scope source_map_scope;
   CurrentSourceFile::Scope unkown_file(SourceFileMap::AddSource("<json>"));
 
-  JsonParserResult result;
-  try {
-    result.value = (*JsonGrammar().Parse(input)).Cast<JsonValue>();
-  } catch (TorqueError& error) {
-    result.error = error;
-  }
-  return result;
+  return (*JsonGrammar().Parse(input)).Cast<JsonValue>();
 }
 
 }  // namespace ls

src/torque/ls/json-parser.h

@@ -6,21 +6,14 @@
 #define V8_TORQUE_LS_JSON_PARSER_H_
 
 #include "src/base/macros.h"
-#include "src/base/optional.h"
 #include "src/torque/ls/json.h"
-#include "src/torque/utils.h"
 
 namespace v8 {
 namespace internal {
 namespace torque {
 namespace ls {
 
-struct JsonParserResult {
-  base::Optional<TorqueError> error;
-  JsonValue value;
-};
-
-V8_EXPORT_PRIVATE JsonParserResult ParseJson(const std::string& input);
+V8_EXPORT_PRIVATE JsonValue ParseJson(const std::string& input);
 
 }  // namespace ls
 }  // namespace torque

src/torque/ls/json.h

@@ -26,13 +26,6 @@ struct JsonValue {
  public:
   enum { OBJECT, ARRAY, STRING, NUMBER, BOOL, IS_NULL } tag;
 
-  // JsonValues can only be moved, not copied.
-  JsonValue() V8_NOEXCEPT = default;
-  constexpr JsonValue(const JsonValue& other) = delete;
-  JsonValue(JsonValue&& other) V8_NOEXCEPT = default;
-  JsonValue& operator=(const JsonValue& other) = delete;
-  JsonValue& operator=(JsonValue&& other) V8_NOEXCEPT = default;
-
   static JsonValue From(double number) {
     JsonValue result;
     result.tag = JsonValue::NUMBER;

src/torque/ls/message-handler.cc

@@ -42,7 +42,7 @@ JsonValue ReadMessage() {
 
   Logger::Log("[incoming] ", content, "\n\n");
 
-  return ParseJson(content).value;
+  return ParseJson(content);
 }
 
 void WriteMessage(JsonValue& message) {

src/torque/torque-compiler.cc

@@ -85,36 +85,24 @@ void CompileCurrentAst(TorqueCompilerOptions options) {
 
 }  // namespace
 
-TorqueCompilerResult CompileTorque(const std::string& source,
-                                   TorqueCompilerOptions options) {
+void CompileTorque(const std::string& source, TorqueCompilerOptions options) {
   CurrentSourceFile::Scope no_file_scope(SourceFileMap::AddSource("<torque>"));
   CurrentAst::Scope ast_scope_;
   LintErrorStatus::Scope lint_error_status_scope_;
 
-  TorqueCompilerResult result;
-  try {
-    ParseTorque(source);
-    CompileCurrentAst(options);
-  } catch (TorqueError& error) {
-    result.error = error;
-  }
-  return result;
+  ParseTorque(source);
+  CompileCurrentAst(options);
 }
 
-TorqueCompilerResult CompileTorque(std::vector<std::string> files,
-                                   TorqueCompilerOptions options) {
+void CompileTorque(std::vector<std::string> files,
+                   TorqueCompilerOptions options) {
   CurrentSourceFile::Scope unknown_source_file_scope(SourceId::Invalid());
   CurrentAst::Scope ast_scope_;
   LintErrorStatus::Scope lint_error_status_scope_;
 
-  TorqueCompilerResult result;
-  try {
-    for (const auto& path : files) ReadAndParseTorqueFile(path);
-    CompileCurrentAst(options);
-  } catch (TorqueError& error) {
-    result.error = error;
-  }
-  return result;
+  for (const auto& path : files) ReadAndParseTorqueFile(path);
+
+  CompileCurrentAst(options);
 }
 
 }  // namespace torque

src/torque/torque-compiler.h

@@ -21,14 +21,10 @@ struct TorqueCompilerOptions {
   bool abort_on_lint_errors;
 };
 
-struct TorqueCompilerResult {
-  base::Optional<TorqueError> error;
-};
-
-V8_EXPORT_PRIVATE TorqueCompilerResult
-CompileTorque(const std::string& source, TorqueCompilerOptions options);
-TorqueCompilerResult CompileTorque(std::vector<std::string> files,
-                                   TorqueCompilerOptions options);
+V8_EXPORT_PRIVATE void CompileTorque(const std::string& source,
+                                     TorqueCompilerOptions options);
+void CompileTorque(std::vector<std::string> files,
+                   TorqueCompilerOptions options);
 
 }  // namespace torque
 }  // namespace internal

src/torque/torque.cc

@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "src/torque/source-positions.h"
 #include "src/torque/torque-compiler.h"
 
 namespace v8 {
@@ -37,13 +36,7 @@ int WrappedMain(int argc, const char** argv) {
   options.collect_language_server_data = false;
   options.abort_on_lint_errors = true;
 
-  TorqueCompilerResult result = CompileTorque(files, options);
-  if (result.error) {
-    TorqueError& error = *result.error;
-    if (error.position) std::cerr << PositionAsString(*error.position) << ": ";
-    std::cerr << "Torque error: " << error.message << "\n";
-    v8::base::OS::Abort();
-  }
+  CompileTorque(files, options);
 
   return 0;
 }

src/torque/utils.cc

@@ -120,11 +120,11 @@ std::string CurrentPositionAsString() {
 
 DEFINE_CONTEXTUAL_VARIABLE(LintErrorStatus)
 
-[[noreturn]] void ThrowTorqueError(const std::string& message,
-                                   bool include_position) {
-  TorqueError error(message);
-  if (include_position) error.position = CurrentSourcePosition::Get();
-  throw error;
+[[noreturn]] void ReportErrorString(const std::string& error,
+                                    bool print_position) {
+  if (print_position) std::cerr << CurrentPositionAsString() << ": ";
+  std::cerr << ": Torque error: " << error << "\n";
+  v8::base::OS::Abort();
 }
 
 void LintError(const std::string& error) {

src/torque/utils.h

@@ -14,7 +14,6 @@
 #include "src/base/functional.h"
 #include "src/base/optional.h"
 #include "src/torque/contextual.h"
-#include "src/torque/source-positions.h"
 
 namespace v8 {
 namespace internal {
@@ -52,26 +51,19 @@ bool IsSnakeCase(const std::string& s);
 bool IsValidNamespaceConstName(const std::string& s);
 bool IsValidTypeName(const std::string& s);
 
-struct TorqueError : public std::exception {
-  explicit TorqueError(const std::string& message) : message(message) {}
-
-  std::string message;
-  base::Optional<SourcePosition> position;
-};
-
-[[noreturn]] void ThrowTorqueError(const std::string& error,
-                                   bool include_position);
+[[noreturn]] void ReportErrorString(const std::string& error,
+                                    bool print_position);
 template <class... Args>
 [[noreturn]] void ReportError(Args&&... args) {
   std::stringstream s;
   USE((s << std::forward<Args>(args))...);
-  ThrowTorqueError(s.str(), true);
+  ReportErrorString(s.str(), true);
 }
 template <class... Args>
 [[noreturn]] void ReportErrorWithoutPosition(Args&&... args) {
   std::stringstream s;
   USE((s << std::forward<Args>(args))...);
-  ThrowTorqueError(s.str(), false);
+  ReportErrorString(s.str(), false);
 }
 
 std::string CapifyStringWithUnderscores(const std::string& camellified_string);

test/unittests/torque/ls-json-unittest.cc

@@ -5,9 +5,7 @@
 #include "src/torque/ls/json-parser.h"
 #include "src/torque/ls/json.h"
 #include "src/torque/source-positions.h"
-#include "src/torque/utils.h"
 #include "test/unittests/test-utils.h"
-#include "testing/gmock-support.h"
 
 namespace v8 {
 namespace internal {
@@ -15,38 +13,38 @@ namespace torque {
 namespace ls {
 
 TEST(LanguageServerJson, TestJsonPrimitives) {
-  const JsonValue true_result = ParseJson("true").value;
+  const JsonValue true_result = ParseJson("true");
   ASSERT_EQ(true_result.tag, JsonValue::BOOL);
   EXPECT_EQ(true_result.ToBool(), true);
 
-  const JsonValue false_result = ParseJson("false").value;
+  const JsonValue false_result = ParseJson("false");
   ASSERT_EQ(false_result.tag, JsonValue::BOOL);
   EXPECT_EQ(false_result.ToBool(), false);
 
-  const JsonValue null_result = ParseJson("null").value;
+  const JsonValue null_result = ParseJson("null");
   ASSERT_EQ(null_result.tag, JsonValue::IS_NULL);
 
-  const JsonValue number = ParseJson("42").value;
+  const JsonValue number = ParseJson("42");
   ASSERT_EQ(number.tag, JsonValue::NUMBER);
   EXPECT_EQ(number.ToNumber(), 42);
 }
 
 TEST(LanguageServerJson, TestJsonStrings) {
-  const JsonValue basic = ParseJson("\"basic\"").value;
+  const JsonValue basic = ParseJson("\"basic\"");
   ASSERT_EQ(basic.tag, JsonValue::STRING);
   EXPECT_EQ(basic.ToString(), "basic");
 
-  const JsonValue singleQuote = ParseJson("\"'\"").value;
+  const JsonValue singleQuote = ParseJson("\"'\"");
   ASSERT_EQ(singleQuote.tag, JsonValue::STRING);
   EXPECT_EQ(singleQuote.ToString(), "'");
 }
 
 TEST(LanguageServerJson, TestJsonArrays) {
-  const JsonValue empty_array = ParseJson("[]").value;
+  const JsonValue empty_array = ParseJson("[]");
   ASSERT_EQ(empty_array.tag, JsonValue::ARRAY);
   EXPECT_EQ(empty_array.ToArray().size(), (size_t)0);
 
-  const JsonValue number_array = ParseJson("[1, 2, 3, 4]").value;
+  const JsonValue number_array = ParseJson("[1, 2, 3, 4]");
   ASSERT_EQ(number_array.tag, JsonValue::ARRAY);
 
   const JsonArray& array = number_array.ToArray();
@@ -54,7 +52,7 @@ TEST(LanguageServerJson, TestJsonArrays) {
   ASSERT_EQ(array[1].tag, JsonValue::NUMBER);
   EXPECT_EQ(array[1].ToNumber(), 2);
 
-  const JsonValue string_array_object = ParseJson("[\"a\", \"b\"]").value;
+  const JsonValue string_array_object = ParseJson("[\"a\", \"b\"]");
   ASSERT_EQ(string_array_object.tag, JsonValue::ARRAY);
 
   const JsonArray& string_array = string_array_object.ToArray();
@@ -64,12 +62,11 @@ TEST(LanguageServerJson, TestJsonArrays) {
 }
 
 TEST(LanguageServerJson, TestJsonObjects) {
-  const JsonValue empty_object = ParseJson("{}").value;
+  const JsonValue empty_object = ParseJson("{}");
   ASSERT_EQ(empty_object.tag, JsonValue::OBJECT);
   EXPECT_EQ(empty_object.ToObject().size(), (size_t)0);
 
-  const JsonValue primitive_fields =
-      ParseJson("{ \"flag\": true, \"id\": 5}").value;
+  const JsonValue primitive_fields = ParseJson("{ \"flag\": true, \"id\": 5}");
   EXPECT_EQ(primitive_fields.tag, JsonValue::OBJECT);
 
   const JsonValue& flag = primitive_fields.ToObject().at("flag");
@@ -81,8 +78,7 @@ TEST(LanguageServerJson, TestJsonObjects) {
   EXPECT_EQ(id.ToNumber(), 5);
 
   const JsonValue& complex_fields =
-      ParseJson("{ \"array\": [], \"object\": { \"name\": \"torque\" } }")
-          .value;
+      ParseJson("{ \"array\": [], \"object\": { \"name\": \"torque\" } }");
   ASSERT_EQ(complex_fields.tag, JsonValue::OBJECT);
 
   const JsonValue& array = complex_fields.ToObject().at("array");
@@ -95,26 +91,12 @@ TEST(LanguageServerJson, TestJsonObjects) {
   EXPECT_EQ(object.ToObject().at("name").ToString(), "torque");
 }
 
-// These tests currently fail on Windows as there seems to be a linking
-// issue with exceptions enabled for Torque.
-// TODO(szuend): Remove the OS check when errors are reported differently,
-//               or the issue is resolved.
-#if !defined(V8_OS_WIN)
-using ::testing::HasSubstr;
-TEST(LanguageServerJson, ParserError) {
-  JsonParserResult result = ParseJson("{]");
-  ASSERT_TRUE(result.error.has_value());
-  EXPECT_THAT(result.error->message,
-              HasSubstr("Parser Error: unexpected token"));
+TEST(LanguageServerJsonDeathTest, SyntaxError) {
+  ASSERT_DEATH(ParseJson("{]"), "Parser Error: unexpected token");
+  ASSERT_DEATH(ParseJson("{ noquoteskey: null }"),
+               "Lexer Error: unknown token");
 }
 
-TEST(LanguageServerJson, LexerError) {
-  JsonParserResult result = ParseJson("{ noquoteskey: null }");
-  ASSERT_TRUE(result.error.has_value());
-  EXPECT_THAT(result.error->message, HasSubstr("Lexer Error: unknown token"));
-}
-#endif
-
 }  // namespace ls
 }  // namespace torque
 }  // namespace internal