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

This reverts commit ffe6940f.

Reason for revert: Breaks UBSan bot

Original change's description:
> Reland^2 "[torque] Throw exception instead of aborting if something goes wrong"
> 
> This is a reland of 251d1623
> 
> The reland fixes ASAN component builds by adding RTTI build config to both
> torque executables. Big thanks to sigurds for finding the fix.
> 
> Original change's description:
> > Reland "[torque] Throw exception instead of aborting if something goes wrong"
> >
> > This is a reland of 3bd49f9b
> >
> > The issue on the windows bot is apparently a compiler bug in MSVC related to
> > move construction. The fix seems to be to change the order of the fields in
> > "JsonParseResult" (go figure).
> >
> > Drive-by-change: Fix LS on windows by emitting correct line endings and
> > enabling exceptions for the LS executable as well.
> >
> > 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}
> >
> > Bug: v8:8880
> > Change-Id: I00e6591bbb4c516dd7540a7e27196853bc637f11
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1545995
> > Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
> > Commit-Queue: Simon Zünd <szuend@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#60736}
> 
> Bug: v8:8880
> Change-Id: Iba198d771169283e83e74324f27aa9e90b8d8975
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1563770
> Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> Commit-Queue: Simon Zünd <szuend@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#60804}

TBR=sigurds@chromium.org,tebbi@chromium.org,szuend@chromium.org

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

BUILD.gn

@@ -3314,26 +3314,13 @@ v8_source_set("torque_base") {
   ]
 
   deps = [
-    ":v8_shared_internal_headers",
-  ]
-
-  public_deps = [
     ":v8_libbase",
+    ":v8_shared_internal_headers",
   ]
 
-  configs = [
-    ":internal_config",
-    "//build/config/compiler:exceptions",
-    "//build/config/compiler:rtti",
-  ]
-
-  remove_configs = [
-    "//build/config/compiler:no_exceptions",
-    "//build/config/compiler:no_rtti",
-  ]
-
+  configs = [ ":internal_config" ]
   if (is_win && is_asan) {
-    remove_configs += [ "//build/config/sanitizers:default_sanitizer_flags" ]
+    remove_configs = [ "//build/config/sanitizers:default_sanitizer_flags" ]
   }
 }
 
@@ -3351,23 +3338,13 @@ v8_source_set("torque_ls_base") {
     "src/torque/ls/message.h",
   ]
 
-  public_deps = [
+  deps = [
     ":torque_base",
   ]
 
-  configs = [
-    ":internal_config",
-    "//build/config/compiler:exceptions",
-    "//build/config/compiler:rtti",
-  ]
-
-  remove_configs = [
-    "//build/config/compiler:no_exceptions",
-    "//build/config/compiler:no_rtti",
-  ]
-
+  configs = [ ":internal_config" ]
   if (is_win && is_asan) {
-    remove_configs += [ "//build/config/sanitizers:default_sanitizer_flags" ]
+    remove_configs = [ "//build/config/sanitizers:default_sanitizer_flags" ]
   }
 }
 
@@ -3738,19 +3715,9 @@ if (current_toolchain == v8_snapshot_toolchain) {
       "//build/win:default_exe_manifest",
     ]
 
-    configs = [
-      ":internal_config",
-      "//build/config/compiler:exceptions",
-      "//build/config/compiler:rtti",
-    ]
-
-    remove_configs = [
-      "//build/config/compiler:no_exceptions",
-      "//build/config/compiler:no_rtti",
-    ]
-
+    configs = [ ":internal_config" ]
     if (is_win && is_asan) {
-      remove_configs += [ "//build/config/sanitizers:default_sanitizer_flags" ]
+      remove_configs = [ "//build/config/sanitizers:default_sanitizer_flags" ]
     }
   }
 }
@@ -3768,19 +3735,9 @@ v8_executable("torque-language-server") {
     "//build/win:default_exe_manifest",
   ]
 
-  configs = [
-    ":internal_config",
-    "//build/config/compiler:exceptions",
-    "//build/config/compiler:rtti",
-  ]
-
-  remove_configs = [
-    "//build/config/compiler:no_exceptions",
-    "//build/config/compiler:no_rtti",
-  ]
-
+  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 {
-  JsonValue value;
-  base::Optional<TorqueError> error;
-};
-
-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,14 +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& operator=(const JsonValue& other) = delete;
-
-  JsonValue(JsonValue&& other) V8_NOEXCEPT = default;
-  JsonValue& operator=(JsonValue&& other) V8_NOEXCEPT = default;
-
   static JsonValue From(double number) {
     JsonValue result;
     result.tag = JsonValue::NUMBER;

src/torque/ls/message-handler.cc

@@ -25,13 +25,6 @@ namespace ls {
 static const char kContentLength[] = "Content-Length: ";
 static const size_t kContentLengthSize = sizeof(kContentLength) - 1;
 
-#ifdef V8_OS_WIN
-// On Windows, in text mode, \n is translated to \r\n.
-constexpr const char* kProtocolLineEnding = "\n\n";
-#else
-constexpr const char* kProtocolLineEnding = "\r\n\r\n";
-#endif
-
 JsonValue ReadMessage() {
   std::string line;
   std::getline(std::cin, line);
@@ -49,7 +42,7 @@ JsonValue ReadMessage() {
 
   Logger::Log("[incoming] ", content, "\n\n");
 
-  return ParseJson(content).value;
+  return ParseJson(content);
 }
 
 void WriteMessage(JsonValue& message) {
@@ -57,7 +50,7 @@ void WriteMessage(JsonValue& message) {
 
   Logger::Log("[outgoing] ", content, "\n\n");
 
-  std::cout << kContentLength << content.size() << kProtocolLineEnding;
+  std::cout << kContentLength << content.size() << "\r\n\r\n";
   std::cout << content << std::flush;
 }
 

src/torque/torque-compiler.cc

@@ -89,36 +89,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