clayborg requested changes to this revision.
clayborg added a comment.

So there is a fundamental change needed for this patch to work. I tried to 
fully detail this in the inline comments. Let me know if you have any questions.



================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1498
+    reverseRequest.try_emplace("command", "runInTerminal");
+    reverseRequest.try_emplace("seq", 100);
+    llvm::json::Object runInTerminalArgs;
----------------
wallace wrote:
> does it work if you don't specify the "seq" number? If it works without it, I 
> would leave it empty
Remove this line. I will explain more in next inline comment!


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1501
+    runInTerminalArgs.try_emplace("kind", "integrated");
+    runInTerminalArgs.try_emplace("cwd", GetString(arguments, "cwd"));
+    std::vector<std::string> commands = GetStrings(arguments, "args");
----------------
wallace wrote:
> what if cwd is not specified in the arguments? I imagine this might break or 
> at least the IDE wouldn't be able to launch the target properly. 
> If "cwd" is not defined, then use `llvm::sys::fs::current_path()` instead. 
> Does this make sense @clayborg ?
yes, we need to set this correctly if it isn't set in the launch.json and your 
suggestion makes sense.


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1513
+    reverseRequest.try_emplace("arguments", 
llvm::json::Value(std::move(runInTerminalArgs)));
+    g_vsc.SendJSON(llvm::json::Value(std::move(reverseRequest)));
+    // g_vsc.SendJSON(llvm::json::Value(std::move(response)));
----------------
So I am not sure we need to do all of the response handler stuff since we need 
to get the result of this before we can proceed. So we need to be able to send 
this request and receive the response right here. So we need to add that 
ability to VSCode (the type definition of g_vsc).

This code should be:

```
llvm::json::Object reverseResponse = 
g_vsc.SendReverseRequest(llvm::json::Value(std::move(reverseRequest)));
```
Then we need to extract the pid from the response right in this function. Right 
now we don't have a thread that is listening for more packets, we just have the 
main thread getting the current packet ("launch" in this case) and we are 
currently handling this function, so there is no way we will get a response in 
this function if we don't do this.

You will need to write the "SendReverseRequest()" function in VSCode and it 
will rely on refactoring that I mentioned below in the main() function where we 
make VSCode::GetObject() and VSCode::HandleObject() and uses the new 
"PacketStatus" enum. The SendReverseRequest function will do something like:

```
struct VSCode {
...
  uint32_t reverse_request_seq = 0;  
  PacketStatus SendReverseRequest(const llvm::json::Value &request, 
llvm::json::Object &response) {
    // Put the right "seq" into the packet here, so we don't have to do it from
    // where we send the reverse request.
    request.try_emplace("seq", ++reverse_request_seq);
    SendJSON(request);
    bool got_response = false;
    while (!got_response) {
      llvm::json::Object response;
      Status status = GetObject(response);
      if (status == PacketStatus::Success) {
        const auto packet_type = GetString(response, "type");
        if (packet_type == "response")
          return status;
        // Not our response, we got another packet
       HandleObject(response)
      } else {
        return status;
      }
    }
  }
```

The "VSCode::GetResponse" is a new function as well that we will make from 



================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2987-3012
     std::string json = g_vsc.ReadJSON();
     if (json.empty())
       break;
 
     llvm::StringRef json_sref(json);
     llvm::Expected<llvm::json::Value> json_value = 
llvm::json::parse(json_sref);
     if (!json_value) {
----------------
We need to move these lines into VSCode.h and VSCode.cpp into a function called 
GetObject():

``` 
enum class PacketStatus {
  Success = 0
  EndOfFile,
  JSONMalformed,
  JSONNotObject
};

PacketStatus VSCode::GetObject(llvm::json::Object &object) {
  std::string json =ReadJSON();
  if (json.empty())
      return PacketStatus::EndOfFile;

  llvm::StringRef json_sref(json);
  llvm::Expected<llvm::json::Value> json_value = llvm::json::parse(json_sref);
  if (!json_value) {
    auto error = json_value.takeError();
    if (log) {
      std::string error_str;
      llvm::raw_string_ostream strm(error_str);
      strm << error;
      strm.flush();
      *log << "error: failed to parse JSON: " << error_str << std::endl
                 << json << std::endl;
    }
    return PacketStatus::JSONMalformed;
  }
  object = std::move(json_value->getAsObject());
  if (!object) {
    if (log)
      *log << "error: json packet isn't a object" << std::endl;
    return PacketStatus::JSONNotObject;
  }
  return PacketStatus::Success;
}
```
This will allow us to use this function in the VSCode::SendReverseRequest(). 
Then this code becomes:

```
llvm::json::Object object;
PacketStatus status = g_vsc.GetObject(object);
if (status == PacketStatus::EndOfFile)
  break;
if (status != PacketStatus::Success)
  return 1; // Fatal error
```

We need to do this so we can re-use the VSCode::GetObject in 
VSCode::SendReverseRequest().


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:3014-3023
     const auto packet_type = GetString(object, "type");
     if (packet_type == "request") {
       const auto command = GetString(object, "command");
       auto handler_pos = request_handlers.find(std::string(command));
       if (handler_pos != request_handlers.end()) {
         handler_pos->second(*object);
       } else {
----------------
This should also be moved into VSCode.h/VSCode.cpp:

bool VSCode::HandleObject(const llvm::json::Object& object) {
  const auto packet_type = GetString(object, "type");
  if (packet_type == "request") {
    const auto command = GetString(object, "command");
    auto handler_pos = request_handlers.find(std::string(command));
    if (handler_pos != request_handlers.end()) {
      handler_pos->second(*object);
      return true; // Success
  } else {
      if (log)
        *log << "error: unhandled command \"" << command.data() << std::endl;
      return false; // Fail
  }
}
```

Then this code becomes:

```
if (!g_vsc.HandleObject(object))
  return 1;
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84974/new/

https://reviews.llvm.org/D84974

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to