[Lldb-commits] [PATCH] D88513: [lldb-vscode] Allow an empty 'breakpoints' field to clear breakpoints.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
rupprecht marked an inline comment as done.
Closed by commit rGad865d9d10b8: [lldb-vscode] Allow an empty
breakpoints field to clear breakpoints. (authored by rupprecht).
Changed prior to commit:
https://reviews.llvm.org/D88513?vs=295124=295364#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88513/new/
https://reviews.llvm.org/D88513
Files:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1936,27 +1936,32 @@
// Decode the source breakpoint infos for this "setBreakpoints" request
SourceBreakpointMap request_bps;
- for (const auto : *breakpoints) {
-auto bp_obj = bp.getAsObject();
-if (bp_obj) {
- SourceBreakpoint src_bp(*bp_obj);
- request_bps[src_bp.line] = src_bp;
-
- // We check if this breakpoint already exists to update it
- auto existing_source_bps = g_vsc.source_breakpoints.find(path);
- if (existing_source_bps != g_vsc.source_breakpoints.end()) {
-const auto _bp = existing_source_bps->second.find(src_bp.line);
-if (existing_bp != existing_source_bps->second.end()) {
- existing_bp->second.UpdateBreakpoint(src_bp);
- AppendBreakpoint(existing_bp->second.bp, response_breakpoints, path,
- src_bp.line);
- continue;
+ // "breakpoints" may be unset, in which case we treat it the same as being set
+ // to an empty array.
+ if (breakpoints) {
+for (const auto : *breakpoints) {
+ auto bp_obj = bp.getAsObject();
+ if (bp_obj) {
+SourceBreakpoint src_bp(*bp_obj);
+request_bps[src_bp.line] = src_bp;
+
+// We check if this breakpoint already exists to update it
+auto existing_source_bps = g_vsc.source_breakpoints.find(path);
+if (existing_source_bps != g_vsc.source_breakpoints.end()) {
+ const auto _bp =
+ existing_source_bps->second.find(src_bp.line);
+ if (existing_bp != existing_source_bps->second.end()) {
+existing_bp->second.UpdateBreakpoint(src_bp);
+AppendBreakpoint(existing_bp->second.bp, response_breakpoints, path,
+ src_bp.line);
+continue;
+ }
}
+// At this point the breakpoint is new
+src_bp.SetBreakpoint(path.data());
+AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
+g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
}
- // At this point the breakpoint is new
- src_bp.SetBreakpoint(path.data());
- AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
- g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
}
}
Index: lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
===
--- lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
+++ lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
@@ -219,6 +219,48 @@
self.assertTrue(breakpoint['verified'],
"expect breakpoint still verified")
+@skipIfWindows
+@skipIfRemote
+def test_clear_breakpoints_unset_breakpoints(self):
+'''Test clearing breakpoints like test_set_and_clear, but clear
+ breakpoints by omitting the breakpoints array instead of sending an
+ empty one.'''
+lines = [line_number('main.cpp', 'break 12'),
+ line_number('main.cpp', 'break 13')]
+
+# Visual Studio Code Debug Adaptors have no way to specify the file
+# without launching or attaching to a process, so we must start a
+# process in order to be able to set breakpoints.
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+
+# Set one breakpoint and verify that it got set correctly.
+response = self.vscode.request_setBreakpoints(self.main_path, lines)
+line_to_id = {}
+breakpoints = response['body']['breakpoints']
+self.assertEquals(len(breakpoints), len(lines),
+"expect %u source breakpoints" % (len(lines)))
+for (breakpoint, index) in zip(breakpoints, range(len(lines))):
+line = breakpoint['line']
+self.assertTrue(line, lines[index])
+# Store the "id" of the breakpoint that was set for later
+line_to_id[line] = breakpoint['id']
+
[Lldb-commits] [PATCH] D88513: [lldb-vscode] Allow an empty 'breakpoints' field to clear breakpoints.
wallace accepted this revision.
wallace added a comment.
This revision is now accepted and ready to land.
Very good! Thank you
Comment at:
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py:228
+ empty one.'''
+lines = [line_number('main.cpp', 'break 12')]
+
you might want to add one additional breakpoint here to make the test more
robust
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88513/new/
https://reviews.llvm.org/D88513
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D88513: [lldb-vscode] Allow an empty 'breakpoints' field to clear breakpoints.
rupprecht created this revision.
rupprecht added reviewers: wallace, clayborg, labath.
Herald added a reviewer: JDevlieghere.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
rupprecht requested review of this revision.
Per the DAP spec for SetBreakpoints [1], the way to clear breakpoints is: `To
clear all breakpoint for a source, specify an empty array.`
However, leaving the breakpoints field unset is also a well formed request
(note the `breakpoints?:` in the `SetBreakpointsArguments` definition). If it's
unset, we have a couple choices:
1. Crash (current behavior)
2. Clear breakpoints
3. Return an error response that the breakpoints field is missing.
I propose we do (2) instead of (1), and treat an unset breakpoints field the
same as an empty breakpoints field.
[1]
https://microsoft.github.io/debug-adapter-protocol/specification#Requests_SetBreakpoints
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D88513
Files:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1936,27 +1936,32 @@
// Decode the source breakpoint infos for this "setBreakpoints" request
SourceBreakpointMap request_bps;
- for (const auto : *breakpoints) {
-auto bp_obj = bp.getAsObject();
-if (bp_obj) {
- SourceBreakpoint src_bp(*bp_obj);
- request_bps[src_bp.line] = src_bp;
-
- // We check if this breakpoint already exists to update it
- auto existing_source_bps = g_vsc.source_breakpoints.find(path);
- if (existing_source_bps != g_vsc.source_breakpoints.end()) {
-const auto _bp = existing_source_bps->second.find(src_bp.line);
-if (existing_bp != existing_source_bps->second.end()) {
- existing_bp->second.UpdateBreakpoint(src_bp);
- AppendBreakpoint(existing_bp->second.bp, response_breakpoints, path,
- src_bp.line);
- continue;
+ // "breakpoints" may be unset, in which case we treat it the same as being set
+ // to an empty array.
+ if (breakpoints) {
+for (const auto : *breakpoints) {
+ auto bp_obj = bp.getAsObject();
+ if (bp_obj) {
+SourceBreakpoint src_bp(*bp_obj);
+request_bps[src_bp.line] = src_bp;
+
+// We check if this breakpoint already exists to update it
+auto existing_source_bps = g_vsc.source_breakpoints.find(path);
+if (existing_source_bps != g_vsc.source_breakpoints.end()) {
+ const auto _bp =
+ existing_source_bps->second.find(src_bp.line);
+ if (existing_bp != existing_source_bps->second.end()) {
+existing_bp->second.UpdateBreakpoint(src_bp);
+AppendBreakpoint(existing_bp->second.bp, response_breakpoints, path,
+ src_bp.line);
+continue;
+ }
}
+// At this point the breakpoint is new
+src_bp.SetBreakpoint(path.data());
+AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
+g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
}
- // At this point the breakpoint is new
- src_bp.SetBreakpoint(path.data());
- AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
- g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
}
}
Index: lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
===
--- lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
+++ lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
@@ -219,6 +219,47 @@
self.assertTrue(breakpoint['verified'],
"expect breakpoint still verified")
+@skipIfWindows
+@skipIfRemote
+def test_clear_breakpoints_unset_breakpoints(self):
+'''Test clearing breakpoints like test_set_and_clear, but clear
+ breakpoints by omitting the breakpoints array instead of sending an
+ empty one.'''
+lines = [line_number('main.cpp', 'break 12')]
+
+# Visual Studio Code Debug Adaptors have no way to specify the file
+# without launching or attaching to a process, so we must start a
+# process in order to be able to set breakpoints.
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+
+# Set one breakpoint and verify that it got set correctly.
+response = self.vscode.request_setBreakpoints(self.main_path, lines)
+line_to_id = {}
+breakpoints = response['body']['breakpoints']
+self.assertEquals(len(breakpoints),
