-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[lldb-dap] Remove an incorrect assumption on reverse requests. #136210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reverse requests do have a 'seq' set still from VSCode. I incorrectly interpreted https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L65 to mean they have a 'seq' of '0', however the 'seq' is set in 'internalSend' here https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L178. Removing the check that 'seq=0' on reverse requests and updating the dap_server.py impl to also set the seq.
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesReverse requests do have a 'seq' set still from VSCode. I incorrectly interpreted https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L65 to mean they have a 'seq' of '0', however the 'seq' is set in 'internalSend' here https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L178. Removing the check that 'seq=0' on reverse requests and updating the dap_server.py impl to also set the seq. Full diff: https://github.com/llvm/llvm-project/pull/136210.diff 2 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 61d7fa94479b8..a9915ba2f6de6 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -343,25 +343,21 @@ def send_recv(self, command):
self.send_packet(
{
"type": "response",
- "seq": 0,
"request_seq": response_or_request["seq"],
"success": True,
"command": "runInTerminal",
"body": {},
},
- set_sequence=False,
)
elif response_or_request["command"] == "startDebugging":
self.send_packet(
{
"type": "response",
- "seq": 0,
"request_seq": response_or_request["seq"],
"success": True,
"command": "startDebugging",
"body": {},
},
- set_sequence=False,
)
else:
desc = 'unknown reverse request "%s"' % (
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
index bfd68448fb483..bc4fee4aa8b8d 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
@@ -163,11 +163,6 @@ bool fromJSON(json::Value const &Params, Response &R, json::Path P) {
return false;
}
- if (seq != 0) {
- P.field("seq").report("expected to be '0'");
- return false;
- }
-
if (R.command.empty()) {
P.field("command").report("expected to not be ''");
return false;
|
Yeah, according to the spec all protocol messages have a sequence number and the sequence number is 1-based:
The spec says "can" but the sequence number is not optional. |
…136210) Reverse requests do have a 'seq' set still from VSCode. I incorrectly interpreted https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L65 to mean they have a 'seq' of '0', however the 'seq' is set in 'internalSend' here https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L178. Removing the check that 'seq=0' on reverse requests and updating the dap_server.py impl to also set the seq.
…136210) Reverse requests do have a 'seq' set still from VSCode. I incorrectly interpreted https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L65 to mean they have a 'seq' of '0', however the 'seq' is set in 'internalSend' here https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L178. Removing the check that 'seq=0' on reverse requests and updating the dap_server.py impl to also set the seq.
…136210) Reverse requests do have a 'seq' set still from VSCode. I incorrectly interpreted https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L65 to mean they have a 'seq' of '0', however the 'seq' is set in 'internalSend' here https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L178. Removing the check that 'seq=0' on reverse requests and updating the dap_server.py impl to also set the seq.
Reverse requests do have a 'seq' set still from VSCode. I incorrectly interpreted https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L65 to mean they have a 'seq' of '0', however the 'seq' is set in 'internalSend' here https://github.com/microsoft/vscode/blob/dede7bb4b7e9c9ec69155a243bb84037a40588fe/src/vs/workbench/contrib/debug/common/abstractDebugAdapter.ts#L178.
Removing the check that 'seq=0' on reverse requests and updating the dap_server.py impl to also set the seq.