[Lldb-commits] [lldb] [llvm] [lldb-dap] Support column breakpoints (PR #113787)

[Adrian Vogelsgesang via lldb-commits]

================
@@ -910,6 +911,183 @@ void request_attach(const llvm::json::Object &request) {
   }
 }
 
+// "BreakpointLocationsRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+//     "type": "object",
+//     "description": "The `breakpointLocations` request returns all possible
+//     locations for source breakpoints in a given range.\nClients should only
+//     call this request if the corresponding capability
+//     `supportsBreakpointLocationsRequest` is true.",
+//     "properties": {
+//       "command": {
+//         "type": "string",
+//         "enum": [ "breakpointLocations" ]
+//       },
+//       "arguments": {
+//         "$ref": "#/definitions/BreakpointLocationsArguments"
+//       }
+//     },
+//     "required": [ "command" ]
+//   }]
+// },
+// "BreakpointLocationsArguments": {
+//   "type": "object",
+//   "description": "Arguments for `breakpointLocations` request.",
+//   "properties": {
+//     "source": {
+//       "$ref": "#/definitions/Source",
+//       "description": "The source location of the breakpoints; either
+//       `source.path` or `source.sourceReference` must be specified."
+//     },
+//     "line": {
+//       "type": "integer",
+//       "description": "Start line of range to search possible breakpoint
+//       locations in. If only the line is specified, the request returns all
+//       possible locations in that line."
+//     },
+//     "column": {
+//       "type": "integer",
+//       "description": "Start position within `line` to search possible
+//       breakpoint locations in. It is measured in UTF-16 code units and the
+//       client capability `columnsStartAt1` determines whether it is 0- or
+//       1-based. If no column is given, the first position in the start line is
+//       assumed."
+//     },
+//     "endLine": {
+//       "type": "integer",
+//       "description": "End line of range to search possible breakpoint
+//       locations in. If no end line is given, then the end line is assumed to
+//       be the start line."
+//     },
+//     "endColumn": {
+//       "type": "integer",
+//       "description": "End position within `endLine` to search possible
+//       breakpoint locations in. It is measured in UTF-16 code units and the
+//       client capability `columnsStartAt1` determines whether it is 0- or
+//       1-based. If no end column is given, the last position in the end line
+//       is assumed."
+//     }
+//   },
+//   "required": [ "source", "line" ]
+// },
+// "BreakpointLocationsResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+//     "type": "object",
+//     "description": "Response to `breakpointLocations` request.\nContains
+//     possible locations for source breakpoints.",
+//     "properties": {
+//       "body": {
+//         "type": "object",
+//         "properties": {
+//           "breakpoints": {
+//             "type": "array",
+//             "items": {
+//               "$ref": "#/definitions/BreakpointLocation"
+//             },
+//             "description": "Sorted set of possible breakpoint locations."
+//           }
+//         },
+//         "required": [ "breakpoints" ]
+//       }
+//     },
+//     "required": [ "body" ]
+//   }]
+// },
+// "BreakpointLocation": {
+//   "type": "object",
+//   "description": "Properties of a breakpoint location returned from the
+//   `breakpointLocations` request.",
+//   "properties": {
+//     "line": {
+//       "type": "integer",
+//       "description": "Start line of breakpoint location."
+//     },
+//     "column": {
+//       "type": "integer",
+//       "description": "The start position of a breakpoint location. Position
+//       is measured in UTF-16 code units and the client capability
+//       `columnsStartAt1` determines whether it is 0- or 1-based."
+//     },
+//     "endLine": {
+//       "type": "integer",
+//       "description": "The end line of breakpoint location if the location
+//       covers a range."
+//     },
+//     "endColumn": {
+//       "type": "integer",
+//       "description": "The end position of a breakpoint location (if the
+//       location covers a range). Position is measured in UTF-16 code units and
+//       the client capability `columnsStartAt1` determines whether it is 0- or
+//       1-based."
+//     }
+//   },
+//   "required": [ "line" ]
+// },
+void request_breakpointLocations(const llvm::json::Object &request) {
----------------
vogelsgesang wrote:

VSCode makes this request after setting a normal line-breakpoint. In particular, it does not send this request on hovers.

As such, we should be fine as long as the performance of this call is in the same ball park as the performance of the `SetBreakpoint` call itself.

The high-level algorithm in this PR should be fine, given that the `SetBreakpoint` itself also iterates the list in a very similar way (see `LineTable::FindLineEntryIndexByFileIndexImpl`.

The implementation of that algorithm might be sub-optimal. Given that we go through the `SB*` interfaces, we are allocating a lot of `shared_ptr`s afaict. It might make sense to implement a `FindPotentialBreakPointsInRange` method directly in the `LineTable`. There we could iterate and filter the line-table without allocating a `SBLineEntry` instance for each entry in the LineTable.

I would appreciate some guidance on the 2nd point: Should I introduce a `LineTable::FindPotentialBreakPointsInRange`?

CC @JDevlieghere 

https://github.com/llvm/llvm-project/pull/113787