Implement type checking for DAP breakpoint requests
authorTom Tromey <tromey@adacore.com>
Thu, 25 May 2023 18:25:43 +0000 (12:25 -0600)
committerTom Tromey <tromey@adacore.com>
Thu, 22 Jun 2023 15:46:23 +0000 (09:46 -0600)
I realized that with a small refactoring, it is possible to type-check
the parameters to the various DAP breakpoint requests.  This would
have caught the earlier bug involving hitCondition.

gdb/python/lib/gdb/dap/breakpoint.py

index 3f40cfb7f6393fbe4dc841aa0330ca2a0156359c..227085ad73ec3e5a9b5623433934c849f09861dd 100644 (file)
@@ -143,19 +143,28 @@ def _set_breakpoints(kind, specs):
     return _set_breakpoints_callback(kind, specs, gdb.Breakpoint)
 
 
-# Turn a DAP SourceBreakpoint, FunctionBreakpoint, or
-# InstructionBreakpoint into a "spec" that is used by
-# _set_breakpoints.  SPEC is a dictionary of parameters that is used
-# as the base of the result; it is modified in place.
-def _basic_spec(bp_info, spec):
-    for name in ("condition", "hitCondition"):
-        if name in bp_info:
-            spec[name] = bp_info[name]
-    return spec
+# A helper function that rewrites a SourceBreakpoint into the internal
+# form passed to the creator.  This function also allows for
+# type-checking of each SourceBreakpoint.
+@type_check
+def _rewrite_src_breakpoint(
+    *,
+    # This is a Source but we don't type-check it.
+    source,
+    line: int,
+    condition: Optional[str] = None,
+    hitCondition: Optional[str] = None,
+    **args,
+):
+    return {
+        "source": source["path"],
+        "line": line,
+        "condition": condition,
+        "hitCondition": hitCondition,
+    }
 
 
 # FIXME we do not specify a type for 'source'.
-# FIXME 'breakpoints' is really a list[SourceBreakpoint].
 @request("setBreakpoints")
 @capability("supportsHitConditionalBreakpoints")
 @capability("supportsConditionalBreakpoints")
@@ -163,17 +172,7 @@ def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
     if "path" not in source:
         result = []
     else:
-        specs = []
-        for obj in breakpoints:
-            specs.append(
-                _basic_spec(
-                    obj,
-                    {
-                        "source": source["path"],
-                        "line": obj["line"],
-                    },
-                )
-            )
+        specs = [_rewrite_src_breakpoint(source=source, **bp) for bp in breakpoints]
         # Be sure to include the path in the key, so that we only
         # clear out breakpoints coming from this same source.
         key = "source:" + source["path"]
@@ -183,45 +182,64 @@ def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
     }
 
 
+# A helper function that rewrites a FunctionBreakpoint into the
+# internal form passed to the creator.  This function also allows for
+# type-checking of each FunctionBreakpoint.
+@type_check
+def _rewrite_fn_breakpoint(
+    *,
+    name: str,
+    condition: Optional[str] = None,
+    hitCondition: Optional[str] = None,
+    **args,
+):
+    return {
+        "function": name,
+        "condition": condition,
+        "hitCondition": hitCondition,
+    }
+
+
 @request("setFunctionBreakpoints")
 @capability("supportsFunctionBreakpoints")
 def set_fn_breakpoint(*, breakpoints: Sequence, **args):
-    specs = []
-    for bp in breakpoints:
-        specs.append(
-            _basic_spec(
-                bp,
-                {
-                    "function": bp["name"],
-                },
-            )
-        )
+    specs = [_rewrite_fn_breakpoint(**bp) for bp in breakpoints]
     result = send_gdb_with_response(lambda: _set_breakpoints("function", specs))
     return {
         "breakpoints": result,
     }
 
 
+# A helper function that rewrites an InstructionBreakpoint into the
+# internal form passed to the creator.  This function also allows for
+# type-checking of each InstructionBreakpoint.
+@type_check
+def _rewrite_insn_breakpoint(
+    *,
+    instructionReference: str,
+    offset: Optional[int] = None,
+    condition: Optional[str] = None,
+    hitCondition: Optional[str] = None,
+    **args,
+):
+    # There's no way to set an explicit address breakpoint from
+    # Python, so we rely on "spec" instead.
+    val = "*" + instructionReference
+    if offset is not None:
+        val = val + " + " + str(offset)
+    return {
+        "spec": val,
+        "condition": condition,
+        "hitCondition": hitCondition,
+    }
+
+
 @request("setInstructionBreakpoints")
 @capability("supportsInstructionBreakpoints")
 def set_insn_breakpoints(
     *, breakpoints: Sequence, offset: Optional[int] = None, **args
 ):
-    specs = []
-    for bp in breakpoints:
-        # There's no way to set an explicit address breakpoint
-        # from Python, so we rely on "spec" instead.
-        val = "*" + bp["instructionReference"]
-        if offset is not None:
-            val = val + " + " + str(offset)
-        specs.append(
-            _basic_spec(
-                bp,
-                {
-                    "spec": val,
-                },
-            )
-        )
+    specs = [_rewrite_insn_breakpoint(**bp) for bp in breakpoints]
     result = send_gdb_with_response(lambda: _set_breakpoints("instruction", specs))
     return {
         "breakpoints": result,
@@ -249,6 +267,23 @@ def _set_exception_catchpoints(filter_options):
     return _set_breakpoints_callback("exception", filter_options, _catch_exception)
 
 
+# A helper function that rewrites an ExceptionFilterOptions into the
+# internal form passed to the creator.  This function also allows for
+# type-checking of each ExceptionFilterOptions.
+@type_check
+def _rewrite_exception_breakpoint(
+    *,
+    filterId: str,
+    condition: Optional[str] = None,
+    # Note that exception breakpoints do not support a hit count.
+    **args,
+):
+    return {
+        "filterId": filterId,
+        "condition": condition,
+    }
+
+
 @request("setExceptionBreakpoints")
 @capability("supportsExceptionFilterOptions")
 @capability(
@@ -272,6 +307,7 @@ def set_exception_breakpoints(
     # Convert the 'filters' to the filter-options style.
     options = [{"filterId": filter} for filter in filters]
     options.extend(filterOptions)
+    options = [_rewrite_exception_breakpoint(**bp) for bp in options]
     result = send_gdb_with_response(lambda: _set_exception_catchpoints(options))
     return {
         "breakpoints": result,