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