[torque] Refactor compiler interface to not 'leak' contextuals

The Torque compiler makes heavy use of scoped globals (contextuals).
This created a problem for the design of the compiler interface:

    - Either the compiler provides all the necessary scopes itself,
      disallowing callers any access to the contextuals, which might
      contain data the caller is interested in (such as the
      compilation result).
    - Or the caller provides all the necessary scopes.

This design was fine when the compiler executable was the only user.
With the recent addition of unit tests and the language server, this
interface became brittle, as missing scopes are only detected at
runtime.

This CL refactors the compiler interface to not leak contextual
scopes past the interface boundary. Content of contextuals is
collected and returned, providing access for the caller and freedom
to either use the data directly or move it into the callers own scopes.

R=sigurds@chromium.org

Bug: v8:7793
Change-Id: Ieb988522d08fc6026b3fb74d976008e566146770
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1529000
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60867}

src/torque/ls/message-handler.cc

@@ -66,15 +66,16 @@ namespace {
 void RecompileTorque() {
   Logger::Log("[info] Start compilation run ...\n");
 
-  LanguageServerData::Get() = LanguageServerData();
-  SourceFileMap::Get() = SourceFileMap();
-
   TorqueCompilerOptions options;
   options.output_directory = "";
   options.verbose = false;
   options.collect_language_server_data = true;
   options.abort_on_lint_errors = false;
-  CompileTorque(TorqueFileList::Get(), options);
+
+  TorqueCompilerResult result = CompileTorque(TorqueFileList::Get(), options);
+
+  LanguageServerData::Get() = result.language_server_data;
+  SourceFileMap::Get() = result.source_file_map;
 
   Logger::Log("[info] Finished compilation run ...\n");
 }

src/torque/torque-compiler.cc

@@ -87,38 +87,55 @@ void CompileCurrentAst(TorqueCompilerOptions options) {
   if (LintErrorStatus::HasLintErrors()) std::abort();
 }
 
+TorqueCompilerResult CollectResultFromContextuals() {
+  TorqueCompilerResult result;
+  result.source_file_map = SourceFileMap::Get();
+  result.language_server_data = LanguageServerData::Get();
+  return result;
+}
+
+TorqueCompilerResult ResultFromError(TorqueError& error) {
+  TorqueCompilerResult result;
+  result.source_file_map = SourceFileMap::Get();
+  result.error = error;
+  return result;
+}
+
 }  // namespace
 
 TorqueCompilerResult CompileTorque(const std::string& source,
                                    TorqueCompilerOptions options) {
+  SourceFileMap::Scope source_map_scope;
   CurrentSourceFile::Scope no_file_scope(SourceFileMap::AddSource("<torque>"));
-  CurrentAst::Scope ast_scope_;
-  LintErrorStatus::Scope lint_error_status_scope_;
+  CurrentAst::Scope ast_scope;
+  LintErrorStatus::Scope lint_error_status_scope;
+  LanguageServerData::Scope server_data_scope;
 
-  TorqueCompilerResult result;
   try {
     ParseTorque(source);
     CompileCurrentAst(options);
   } catch (TorqueError& error) {
-    result.error = error;
+    return ResultFromError(error);
   }
-  return result;
+
+  return CollectResultFromContextuals();
 }
 
 TorqueCompilerResult CompileTorque(std::vector<std::string> files,
                                    TorqueCompilerOptions options) {
+  SourceFileMap::Scope source_map_scope;
   CurrentSourceFile::Scope unknown_source_file_scope(SourceId::Invalid());
-  CurrentAst::Scope ast_scope_;
-  LintErrorStatus::Scope lint_error_status_scope_;
+  CurrentAst::Scope ast_scope;
+  LintErrorStatus::Scope lint_error_status_scope;
+  LanguageServerData::Scope server_data_scope;
 
-  TorqueCompilerResult result;
   try {
     for (const auto& path : files) ReadAndParseTorqueFile(path);
     CompileCurrentAst(options);
   } catch (TorqueError& error) {
-    result.error = error;
+    return ResultFromError(error);
   }
-  return result;
+  return CollectResultFromContextuals();
 }
 
 }  // namespace torque

src/torque/torque-compiler.h

@@ -7,6 +7,7 @@
 
 #include "src/torque/ast.h"
 #include "src/torque/contextual.h"
+#include "src/torque/server-data.h"
 #include "src/torque/source-positions.h"
 #include "src/torque/utils.h"
 
@@ -22,6 +23,17 @@ struct TorqueCompilerOptions {
 };
 
 struct TorqueCompilerResult {
+  // Map translating SourceIds to filenames. This field is
+  // set on errors, so the SourcePosition of the error can be
+  // resolved.
+  SourceFileMap source_file_map;
+
+  // Eagerly collected data needed for the LanguageServer.
+  // Set the corresponding options flag to enable.
+  LanguageServerData language_server_data;
+
+  // If any error occurred during either parsing or compilation,
+  // this field will be set.
   base::Optional<TorqueError> error;
 };
 

src/torque/torque.cc

@@ -29,8 +29,6 @@ int WrappedMain(int argc, const char** argv) {
     files.emplace_back(argv[i]);
   }
 
-  SourceFileMap::Scope source_file_map_scope;
-
   TorqueCompilerOptions options;
   options.output_directory = output_directory;
   options.verbose = verbose;
@@ -39,6 +37,10 @@ int WrappedMain(int argc, const char** argv) {
 
   TorqueCompilerResult result = CompileTorque(files, options);
   if (result.error) {
+    // PositionAsString requires the SourceFileMap to be set to
+    // resolve the file name.
+    SourceFileMap::Scope source_file_map_scope(result.source_file_map);
+
     TorqueError& error = *result.error;
     if (error.position) std::cerr << PositionAsString(*error.position) << ": ";
     std::cerr << "Torque error: " << error.message << "\n";

test/unittests/torque/ls-server-data-unittest.cc

@@ -23,7 +23,9 @@ struct TestCompiler {
     options.verbose = false;
     options.collect_language_server_data = true;
 
-    CompileTorque(source, options);
+    const TorqueCompilerResult result = CompileTorque(source, options);
+    SourceFileMap::Get() = result.source_file_map;
+    LanguageServerData::Get() = result.language_server_data;
   }
 };