[Lldb-commits] [lldb] [lldb-dap] Refactoring breakpoints to not use the `g_dap` reference. (PR #115208)

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Thu Nov 7 01:13:45 PST 2024


================
@@ -4572,235 +4554,219 @@ void request__testGetTargetBreakpoints(const llvm::json::Object &request) {
 //           "(with reason `instruction breakpoint`) is generated.\nClients "
 //           "should only call this request if the corresponding capability "
 //           "`supportsInstructionBreakpoints` is true.",
-//       "properties" : {
-//         "command" : {"type" : "string", "enum" :
-//         ["setInstructionBreakpoints"]}, "arguments" :
-//             {"$ref" : "#/definitions/SetInstructionBreakpointsArguments"}
+//       "properties": {
+//         "command": { "type": "string", "enum": ["setInstructionBreakpoints"]
+//         }, "arguments": {"$ref":
+//         "#/definitions/SetInstructionBreakpointsArguments"}
 //       },
-//       "required" : [ "command", "arguments" ]
+//       "required": [ "command", "arguments" ]
 //     }
 //   ]
 // },
-//                                      "SetInstructionBreakpointsArguments"
-//     : {
-//       "type" : "object",
-//       "description" : "Arguments for `setInstructionBreakpoints` request",
-//       "properties" : {
-//         "breakpoints" : {
-//           "type" : "array",
-//           "items" : {"$ref" : "#/definitions/InstructionBreakpoint"},
-//           "description" : "The instruction references of the breakpoints"
-//         }
-//       },
-//       "required" : ["breakpoints"]
-//     },
-//       "SetInstructionBreakpointsResponse"
-//     : {
-//       "allOf" : [
-//         {"$ref" : "#/definitions/Response"}, {
-//           "type" : "object",
-//           "description" : "Response to `setInstructionBreakpoints` request",
-//           "properties" : {
-//             "body" : {
-//               "type" : "object",
-//               "properties" : {
-//                 "breakpoints" : {
-//                   "type" : "array",
-//                   "items" : {"$ref" : "#/definitions/Breakpoint"},
-//                   "description" :
-//                       "Information about the breakpoints. The array elements
-//                       " "correspond to the elements of the `breakpoints`
-//                       array."
-//                 }
-//               },
-//               "required" : ["breakpoints"]
+// "SetInstructionBreakpointsArguments": {
+//   "type": "object",
+//   "description": "Arguments for `setInstructionBreakpoints` request",
+//   "properties": {
+//     "breakpoints": {
+//       "type": "array",
+//       "items": {"$ref": "#/definitions/InstructionBreakpoint"},
+//       "description": "The instruction references of the breakpoints"
+//     }
+//   },
+//   "required": ["breakpoints"]
+// },
+// "SetInstructionBreakpointsResponse": {
+//   "allOf": [
+//     {"$ref": "#/definitions/Response"},
+//     {
+//       "type": "object",
+//       "description": "Response to `setInstructionBreakpoints` request",
+//       "properties": {
+//         "body": {
+//           "type": "object",
+//           "properties": {
+//             "breakpoints": {
+//               "type": "array",
+//               "items": {"$ref": "#/definitions/Breakpoint"},
+//               "description":
+//                   "Information about the breakpoints. The array elements
+//                   " "correspond to the elements of the `breakpoints`
+//                   array."
 //             }
 //           },
-//           "required" : ["body"]
+//           "required": ["breakpoints"]
 //         }
-//       ]
-//     },
-// "InstructionBreakpoint" : {
-//   "type" : "object",
-//   "description" : "Properties of a breakpoint passed to the "
+//       },
+//       "required": ["body"]
+//     }
+//   ]
+// },
+// "InstructionBreakpoint": {
+//   "type": "object",
+//   "description": "Properties of a breakpoint passed to the "
 //                   "`setInstructionBreakpoints` request",
-//   "properties" : {
-//     "instructionReference" : {
-//       "type" : "string",
+//   "properties": {
+//     "instructionReference": {
+//       "type": "string",
 //       "description" :
 //           "The instruction reference of the breakpoint.\nThis should be a "
 //           "memory or instruction pointer reference from an
 //           `EvaluateResponse`, "
 //           "`Variable`, `StackFrame`, `GotoTarget`, or `Breakpoint`."
 //     },
-//     "offset" : {
-//       "type" : "integer",
-//       "description" : "The offset from the instruction reference in "
+//     "offset": {
+//       "type": "integer",
+//       "description": "The offset from the instruction reference in "
 //                       "bytes.\nThis can be negative."
 //     },
-//     "condition" : {
-//       "type" : "string",
-//       "description" : "An expression for conditional breakpoints.\nIt is only
+//     "condition": {
+//       "type": "string",
+//       "description": "An expression for conditional breakpoints.\nIt is only
 //       "
 //                       "honored by a debug adapter if the corresponding "
 //                       "capability `supportsConditionalBreakpoints` is true."
 //     },
-//     "hitCondition" : {
-//       "type" : "string",
-//       "description" : "An expression that controls how many hits of the "
+//     "hitCondition": {
+//       "type": "string",
+//       "description": "An expression that controls how many hits of the "
 //                       "breakpoint are ignored.\nThe debug adapter is expected
 //                       " "to interpret the expression as needed.\nThe
 //                       attribute " "is only honored by a debug adapter if the
 //                       corresponding " "capability
 //                       `supportsHitConditionalBreakpoints` is true."
 //     },
-//     "mode" : {
-//       "type" : "string",
-//       "description" : "The mode of this breakpoint. If defined, this must be
+//     "mode": {
+//       "type": "string",
+//       "description": "The mode of this breakpoint. If defined, this must be
 //       "
 //                       "one of the `breakpointModes` the debug adapter "
 //                       "advertised in its `Capabilities`."
 //     }
 //   },
-//   "required" : ["instructionReference"]
+//   "required": ["instructionReference"]
 // },
-// "Breakpoint"
-//     : {
-//       "type" : "object",
+// "Breakpoint": {
+//   "type": "object",
+//   "description" :
+//       "Information about a breakpoint created in `setBreakpoints`, "
+//       "`setFunctionBreakpoints`, `setInstructionBreakpoints`, or "
+//       "`setDataBreakpoints` requests.",
+//   "properties": {
+//     "id": {
+//       "type": "integer",
 //       "description" :
-//           "Information about a breakpoint created in `setBreakpoints`, "
-//           "`setFunctionBreakpoints`, `setInstructionBreakpoints`, or "
-//           "`setDataBreakpoints` requests.",
-//       "properties" : {
-//         "id" : {
-//           "type" : "integer",
-//           "description" :
-//               "The identifier for the breakpoint. It is needed if breakpoint
-//               " "events are used to update or remove breakpoints."
-//         },
-//         "verified" : {
-//           "type" : "boolean",
-//           "description" : "If true, the breakpoint could be set (but not "
-//                           "necessarily at the desired location)."
-//         },
-//         "message" : {
-//           "type" : "string",
-//           "description" : "A message about the state of the breakpoint.\nThis
-//           "
-//                           "is shown to the user and can be used to explain
-//                           why " "a breakpoint could not be verified."
-//         },
-//         "source" : {
-//           "$ref" : "#/definitions/Source",
-//           "description" : "The source where the breakpoint is located."
-//         },
-//         "line" : {
-//           "type" : "integer",
-//           "description" :
-//               "The start line of the actual range covered by the breakpoint."
-//         },
-//         "column" : {
-//           "type" : "integer",
-//           "description" :
-//               "Start position of the source range covered by the breakpoint.
-//               " "It 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 the actual range covered by the breakpoint."
-//         },
-//         "endColumn" : {
-//           "type" : "integer",
-//           "description" :
-//               "End position of the source range covered by the breakpoint. It
-//               " "is measured in UTF-16 code units and the client capability "
-//               "`columnsStartAt1` determines whether it is 0- or 1-based.\nIf
-//               " "no end line is given, then the end column is assumed to be
-//               in " "the start line."
-//         },
-//         "instructionReference" : {
-//           "type" : "string",
-//           "description" : "A memory reference to where the breakpoint is
-//           set."
-//         },
-//         "offset" : {
-//           "type" : "integer",
-//           "description" : "The offset from the instruction reference.\nThis "
-//                           "can be negative."
-//         },
-//         "reason" : {
-//           "type" : "string",
-//           "description" :
-//               "A machine-readable explanation of why a breakpoint may not be
-//               " "verified. If a breakpoint is verified or a specific reason
-//               is " "not known, the adapter should omit this property.
-//               Possible " "values include:\n\n- `pending`: Indicates a
-//               breakpoint might be " "verified in the future, but the adapter
-//               cannot verify it in the " "current state.\n - `failed`:
-//               Indicates a breakpoint was not " "able to be verified, and the
-//               adapter does not believe it can be " "verified without
-//               intervention.",
-//           "enum" : [ "pending", "failed" ]
-//         }
-//       },
-//       "required" : ["verified"]
+//           "The identifier for the breakpoint. It is needed if breakpoint
+//           " "events are used to update or remove breakpoints."
 //     },
-
+//     "verified": {
+//       "type": "boolean",
+//       "description": "If true, the breakpoint could be set (but not "
+//                       "necessarily at the desired location)."
+//     },
+//     "message": {
+//       "type": "string",
+//       "description": "A message about the state of the breakpoint.\nThis
+//       "
+//                       "is shown to the user and can be used to explain
+//                       why " "a breakpoint could not be verified."
+//     },
+//     "source": {
+//       "$ref": "#/definitions/Source",
+//       "description": "The source where the breakpoint is located."
+//     },
+//     "line": {
+//       "type": "integer",
+//       "description" :
+//           "The start line of the actual range covered by the breakpoint."
+//     },
+//     "column": {
+//       "type": "integer",
+//       "description" :
+//           "Start position of the source range covered by the breakpoint.
+//           " "It 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 the actual range covered by the breakpoint."
+//     },
+//     "endColumn": {
+//       "type": "integer",
+//       "description" :
+//           "End position of the source range covered by the breakpoint. It
+//           " "is measured in UTF-16 code units and the client capability "
+//           "`columnsStartAt1` determines whether it is 0- or 1-based.\nIf
+//           " "no end line is given, then the end column is assumed to be
+//           in " "the start line."
+//     },
+//     "instructionReference": {
+//       "type": "string",
+//       "description": "A memory reference to where the breakpoint is
+//       set."
+//     },
+//     "offset": {
+//       "type": "integer",
+//       "description": "The offset from the instruction reference.\nThis "
+//                       "can be negative."
+//     },
+//     "reason": {
+//       "type": "string",
+//       "description" :
+//           "A machine-readable explanation of why a breakpoint may not be
+//           " "verified. If a breakpoint is verified or a specific reason
+//           is " "not known, the adapter should omit this property.
+//           Possible " "values include:\n\n- `pending`: Indicates a
+//           breakpoint might be " "verified in the future, but the adapter
+//           cannot verify it in the " "current state.\n - `failed`:
+//           Indicates a breakpoint was not " "able to be verified, and the
+//           adapter does not believe it can be " "verified without
+//           intervention.",
+//       "enum": [ "pending", "failed" ]
+//     }
+//   },
+//   "required": ["verified"]
+// },
 void request_setInstructionBreakpoints(const llvm::json::Object &request) {
   llvm::json::Object response;
   llvm::json::Array response_breakpoints;
   llvm::json::Object body;
   FillResponse(request, response);
 
-  auto arguments = request.getObject("arguments");
-  auto breakpoints = arguments->getArray("breakpoints");
+  const auto *arguments = request.getObject("arguments");
+  const auto *breakpoints = arguments->getArray("breakpoints");
 
-  // It holds active instruction breakpoint list received from DAP.
-  InstructionBreakpointMap request_ibp;
-  if (breakpoints) {
-    for (const auto &bp : *breakpoints) {
-      auto bp_obj = bp.getAsObject();
-      if (bp_obj) {
-        // Read instruction breakpoint request.
-        InstructionBreakpoint inst_bp(*bp_obj);
-        // Store them into map for reference.
-        request_ibp[inst_bp.instructionAddressReference] = std::move(inst_bp);
-      }
-    }
+  // Disable any instruction breakpoints that aren't in this request.
+  // There is no call to remove instruction breakpoints other than calling this
+  // function with a smaller or empty "breakpoints" list.
+  llvm::SetVector<lldb::addr_t> seen;
+  for (const auto &addr : g_dap.instruction_breakpoints)
+    seen.insert(addr.first);
 
-    // Iterate previous active instruction breakpoint list.
-    for (auto &prev_ibp : g_dap.instruction_breakpoints) {
-      // Find previous instruction breakpoint reference address in newly
-      // received instruction breakpoint list.
-      auto inst_reference = request_ibp.find(prev_ibp.first);
-      // Request for remove and delete the breakpoint, if the prev instruction
-      // breakpoint ID is not available in active instrcation breakpoint list.
-      // Means delete removed breakpoint instance.
-      if (inst_reference == request_ibp.end()) {
-        g_dap.target.BreakpointDelete(prev_ibp.second.id);
-        // Update Prev instruction breakpoint list.
-        g_dap.instruction_breakpoints.erase(prev_ibp.first);
-      } else {
-        // Instead of recreating breakpoint instance, update the breakpoint if
-        // there are any conditional changes.
-        prev_ibp.second.UpdateBreakpoint(inst_reference->second);
-        request_ibp.erase(inst_reference);
-        response_breakpoints.emplace_back(
-            CreateInstructionBreakpoint(&prev_ibp.second));
-      }
+  for (const auto &bp : *breakpoints) {
+    const auto *bp_obj = bp.getAsObject();
+    if (!bp_obj)
+      continue;
+    // Read instruction breakpoint request.
+    InstructionBreakpoint inst_bp(g_dap, *bp_obj);
+    const auto [kv, inserted] = g_dap.instruction_breakpoints.try_emplace(
+        inst_bp.instructionAddressReference, g_dap, *bp_obj);
+    if (inserted) {
----------------
labath wrote:

and braces

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


More information about the lldb-commits mailing list