[cleanup] Avoid non-const reference arguments in src/torque

This CL changes non-const reference arguments to either a const
reference, or pass-by-value combined with std::move.

Bug: v8:9429
Change-Id: Iabace132f855462612ac31922fbd8b456d8ae20d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1690827Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Auto-Submit: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62583}

src/torque/ls/message-handler.cc

@@ -53,7 +53,7 @@ JsonValue ReadMessage() {
   return ParseJson(content).value;
 }
 
-void WriteMessage(JsonValue& message) {  // NOLINT(runtime/references)
+void WriteMessage(JsonValue message) {
   std::string content = SerializeToString(message);
 
   Logger::Log("[outgoing] ", content, "\n\n");
@@ -74,7 +74,7 @@ void ResetCompilationErrorDiagnostics(MessageWriter writer) {
     // Trigger empty array creation.
     USE(notification.params().diagnostics_size());
 
-    writer(notification.GetJsonValue());
+    writer(std::move(notification.GetJsonValue()));
   }
   DiagnosticsFiles::Get() = {};
 }
@@ -151,7 +151,7 @@ void SendCompilationDiagnostics(const TorqueCompilerResult& result,
 
   for (auto& pair : collector.notifications()) {
     PublishDiagnosticsNotification& notification = pair.second;
-    writer(notification.GetJsonValue());
+    writer(std::move(notification.GetJsonValue()));
 
     // Record all source files for which notifications are sent, so they
     // can be reset before the next compiler run.
@@ -205,7 +205,7 @@ void HandleInitializeRequest(InitializeRequest request, MessageWriter writer) {
   //               "workspace/didChangeWatchedFiles" capability.
   // TODO(szuend): Check if client supports "LocationLink". This will
   //               influence the result of "goto definition".
-  writer(response.GetJsonValue());
+  writer(std::move(response.GetJsonValue()));
 }
 
 void HandleInitializedNotification(MessageWriter writer) {
@@ -224,7 +224,7 @@ void HandleInitializedNotification(MessageWriter writer) {
   reg.set_id("did-change-id");
   reg.set_method("workspace/didChangeWatchedFiles");
 
-  writer(request.GetJsonValue());
+  writer(std::move(request.GetJsonValue()));
 }
 
 void HandleTorqueFileListNotification(TorqueFileListNotification notification,
@@ -258,7 +258,7 @@ void HandleGotoDefinitionRequest(GotoDefinitionRequest request,
   // the definition not beeing found.
   if (!id.IsValid()) {
     response.SetNull("result");
-    writer(response.GetJsonValue());
+    writer(std::move(response.GetJsonValue()));
     return;
   }
 
@@ -272,7 +272,7 @@ void HandleGotoDefinitionRequest(GotoDefinitionRequest request,
     response.SetNull("result");
   }
 
-  writer(response.GetJsonValue());
+  writer(std::move(response.GetJsonValue()));
 }
 
 void HandleChangeWatchedFilesNotification(
@@ -325,14 +325,13 @@ void HandleDocumentSymbolRequest(DocumentSymbolRequest request,
   // Trigger empty array creation in case no symbols were found.
   USE(response.result_size());
 
-  writer(response.GetJsonValue());
+  writer(std::move(response.GetJsonValue()));
 }
 
 }  // namespace
 
-void HandleMessage(JsonValue& raw_message,  // NOLINT(runtime/references)
-                   MessageWriter writer) {
-  Request<bool> request(raw_message);
+void HandleMessage(JsonValue raw_message, MessageWriter writer) {
+  Request<bool> request(std::move(raw_message));
 
   // We ignore responses for now. They are matched to requests
   // by id and don't have a method set.
@@ -345,21 +344,23 @@ void HandleMessage(JsonValue& raw_message,  // NOLINT(runtime/references)
 
   const std::string method = request.method();
   if (method == "initialize") {
-    HandleInitializeRequest(InitializeRequest(request.GetJsonValue()), writer);
+    HandleInitializeRequest(
+        InitializeRequest(std::move(request.GetJsonValue())), writer);
   } else if (method == "initialized") {
     HandleInitializedNotification(writer);
   } else if (method == "torque/fileList") {
     HandleTorqueFileListNotification(
-        TorqueFileListNotification(request.GetJsonValue()), writer);
+        TorqueFileListNotification(std::move(request.GetJsonValue())), writer);
   } else if (method == "textDocument/definition") {
-    HandleGotoDefinitionRequest(GotoDefinitionRequest(request.GetJsonValue()),
-                                writer);
+    HandleGotoDefinitionRequest(
+        GotoDefinitionRequest(std::move(request.GetJsonValue())), writer);
   } else if (method == "workspace/didChangeWatchedFiles") {
     HandleChangeWatchedFilesNotification(
-        DidChangeWatchedFilesNotification(request.GetJsonValue()), writer);
+        DidChangeWatchedFilesNotification(std::move(request.GetJsonValue())),
+        writer);
   } else if (method == "textDocument/documentSymbol") {
-    HandleDocumentSymbolRequest(DocumentSymbolRequest(request.GetJsonValue()),
-                                writer);
+    HandleDocumentSymbolRequest(
+        DocumentSymbolRequest(std::move(request.GetJsonValue())), writer);
   } else {
     Logger::Log("[error] Message of type ", method, " is not handled!\n\n");
   }

src/torque/ls/message-handler.h

@@ -24,11 +24,9 @@ namespace ls {
 
 // The message handler might send responses or follow up requests.
 // To allow unit testing, the "sending" function is configurable.
-using MessageWriter = std::function<void(JsonValue&)>;
+using MessageWriter = std::function<void(JsonValue)>;
 
-V8_EXPORT_PRIVATE void HandleMessage(
-    JsonValue& raw_message,  // NOLINT(runtime/references)
-    MessageWriter);
+V8_EXPORT_PRIVATE void HandleMessage(JsonValue raw_message, MessageWriter);
 
 // Called when a compilation run finishes. Exposed for testability.
 V8_EXPORT_PRIVATE void CompilationFinished(TorqueCompilerResult result,

src/torque/ls/message-pipe.h

@@ -14,7 +14,7 @@ namespace torque {
 namespace ls {
 
 JsonValue ReadMessage();
-void WriteMessage(JsonValue& message);  // NOLINT(runtime/references)
+void WriteMessage(JsonValue message);
 
 }  // namespace ls
 }  // namespace torque

src/torque/ls/message.h

@@ -73,7 +73,7 @@ class Message : public BaseJsonAccessor {
     value_ = JsonValue::From(JsonObject{});
     set_jsonrpc("2.0");
   }
-  explicit Message(JsonValue& value) : value_(std::move(value)) {
+  explicit Message(JsonValue value) : value_(std::move(value)) {
     CHECK(value_.tag == JsonValue::OBJECT);
   }
 
@@ -323,7 +323,7 @@ class SymbolInformation : public NestedJsonAccessor {
 template <class T>
 class Request : public Message {
  public:
-  explicit Request(JsonValue& value) : Message(value) {}
+  explicit Request(JsonValue value) : Message(std::move(value)) {}
   Request() : Message() {}
 
   JSON_INT_ACCESSORS(id)
@@ -341,7 +341,7 @@ using DocumentSymbolRequest = Request<DocumentSymbolParams>;
 template <class T>
 class Response : public Message {
  public:
-  explicit Response(JsonValue& value) : Message(value) {}
+  explicit Response(JsonValue value) : Message(std::move(value)) {}
   Response() : Message() {}
 
   JSON_INT_ACCESSORS(id)
@@ -355,7 +355,7 @@ using GotoDefinitionResponse = Response<Location>;
 template <class T>
 class ResponseArrayResult : public Message {
  public:
-  explicit ResponseArrayResult(JsonValue& value) : Message(value) {}
+  explicit ResponseArrayResult(JsonValue value) : Message(std::move(value)) {}
   ResponseArrayResult() : Message() {}
 
   JSON_INT_ACCESSORS(id)

src/torque/ls/torque-language-server.cc

@@ -32,13 +32,13 @@ int WrappedMain(int argc, const char** argv) {
   }
 
   while (true) {
-    auto message = ReadMessage();
+    JsonValue message = ReadMessage();
 
     // TODO(szuend): We should probably offload the actual message handling
     //               (even the parsing) to a background thread, so we can
     //               keep receiving messages. We might also receive
     //               $/cancelRequests or contet updates, that require restarts.
-    HandleMessage(message, &WriteMessage);
+    HandleMessage(std::move(message), &WriteMessage);
   }
   return 0;
 }

src/torque/torque-parser.cc

@@ -392,7 +392,7 @@ base::Optional<ParseResult> MakeImplicitParameterList(
 }
 
 void AddParameter(ParameterList* parameter_list,
-                  NameAndTypeExpression& param) {  // NOLINT(runtime/references)
+                  const NameAndTypeExpression& param) {
   if (!IsLowerCamelCase(param.name->value)) {
     NamingConventionError("Parameter", param.name, "lowerCamelCase");
   }

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

@@ -21,8 +21,8 @@ TEST(LanguageServerMessage, InitializeRequest) {
   request.params();
 
   bool writer_called = false;
-  HandleMessage(request.GetJsonValue(), [&](JsonValue& raw_response) {
-    InitializeResponse response(raw_response);
+  HandleMessage(std::move(request.GetJsonValue()), [&](JsonValue raw_response) {
+    InitializeResponse response(std::move(raw_response));
 
     // Check that the response id matches up with the request id, and that
     // the language server signals its support for definitions.
@@ -41,8 +41,9 @@ TEST(LanguageServerMessage,
   notification.set_method("initialized");
 
   bool writer_called = false;
-  HandleMessage(notification.GetJsonValue(), [&](JsonValue& raw_request) {
-    RegistrationRequest request(raw_request);
+  HandleMessage(std::move(notification.GetJsonValue()), [&](JsonValue
+                                                                raw_request) {
+    RegistrationRequest request(std::move(raw_request));
 
     ASSERT_EQ(request.method(), "client/registerCapability");
     ASSERT_EQ(request.params().registrations_size(), (size_t)1);
@@ -69,8 +70,8 @@ TEST(LanguageServerMessage, GotoDefinitionUnkownFile) {
   request.params().textDocument().set_uri("file:///unknown.tq");
 
   bool writer_called = false;
-  HandleMessage(request.GetJsonValue(), [&](JsonValue& raw_response) {
-    GotoDefinitionResponse response(raw_response);
+  HandleMessage(std::move(request.GetJsonValue()), [&](JsonValue raw_response) {
+    GotoDefinitionResponse response(std::move(raw_response));
     EXPECT_EQ(response.id(), 42);
     EXPECT_TRUE(response.IsNull("result"));
 
@@ -97,8 +98,8 @@ TEST(LanguageServerMessage, GotoDefinition) {
   request.params().position().set_character(0);
 
   bool writer_called = false;
-  HandleMessage(request.GetJsonValue(), [&](JsonValue& raw_response) {
-    GotoDefinitionResponse response(raw_response);
+  HandleMessage(std::move(request.GetJsonValue()), [&](JsonValue raw_response) {
+    GotoDefinitionResponse response(std::move(raw_response));
     EXPECT_EQ(response.id(), 42);
     EXPECT_TRUE(response.IsNull("result"));
 
@@ -115,8 +116,8 @@ TEST(LanguageServerMessage, GotoDefinition) {
   request.params().position().set_character(5);
 
   writer_called = false;
-  HandleMessage(request.GetJsonValue(), [&](JsonValue& raw_response) {
-    GotoDefinitionResponse response(raw_response);
+  HandleMessage(std::move(request.GetJsonValue()), [&](JsonValue raw_response) {
+    GotoDefinitionResponse response(std::move(raw_response));
     EXPECT_EQ(response.id(), 43);
     ASSERT_FALSE(response.IsNull("result"));
 
@@ -144,8 +145,8 @@ TEST(LanguageServerMessage, CompilationErrorSendsDiagnostics) {
   result.source_file_map = SourceFileMap::Get();
 
   bool writer_called = false;
-  CompilationFinished(std::move(result), [&](JsonValue& raw_response) {
-    PublishDiagnosticsNotification notification(raw_response);
+  CompilationFinished(std::move(result), [&](JsonValue raw_response) {
+    PublishDiagnosticsNotification notification(std::move(raw_response));
 
     EXPECT_EQ(notification.method(), "textDocument/publishDiagnostics");
     ASSERT_FALSE(notification.IsNull("params"));
@@ -181,8 +182,8 @@ TEST(LanguageServerMessage, LintErrorSendsDiagnostics) {
   result.source_file_map = SourceFileMap::Get();
 
   bool writer_called = false;
-  CompilationFinished(std::move(result), [&](JsonValue& raw_response) {
-    PublishDiagnosticsNotification notification(raw_response);
+  CompilationFinished(std::move(result), [&](JsonValue raw_response) {
+    PublishDiagnosticsNotification notification(std::move(raw_response));
 
     EXPECT_EQ(notification.method(), "textDocument/publishDiagnostics");
     ASSERT_FALSE(notification.IsNull("params"));
@@ -209,7 +210,7 @@ TEST(LanguageServerMessage, CleanCompileSendsNoDiagnostics) {
   TorqueCompilerResult result;
   result.source_file_map = SourceFileMap::Get();
 
-  CompilationFinished(std::move(result), [](JsonValue& raw_response) {
+  CompilationFinished(std::move(result), [](JsonValue raw_response) {
     FAIL() << "Sending unexpected response!";
   });
 }
@@ -224,8 +225,8 @@ TEST(LanguageServerMessage, NoSymbolsSendsEmptyResponse) {
   request.params().textDocument().set_uri("file://test.tq");
 
   bool writer_called = false;
-  HandleMessage(request.GetJsonValue(), [&](JsonValue& raw_response) {
-    DocumentSymbolResponse response(raw_response);
+  HandleMessage(std::move(request.GetJsonValue()), [&](JsonValue raw_response) {
+    DocumentSymbolResponse response(std::move(raw_response));
     EXPECT_EQ(response.id(), 42);
     EXPECT_EQ(response.result_size(), static_cast<size_t>(0));