[PATCH] Ensure all DAP requests are keyword-only

[PATCH] Ensure all DAP requests are keyword-only

[Tom Tromey]

Python functions implementing DAP requests should not use positional
parameters -- it only makes sense to call them with keyword arguments.
This patch changes the few remaining cases to start with the special
"*" parameter, following this rule.
---
 gdb/python/lib/gdb/dap/breakpoint.py | 6 +++---
 gdb/python/lib/gdb/dap/evaluate.py   | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 502beb0478e..f0e1f103d1b 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -1,4 +1,4 @@
-# Copyright 2022 Free Software Foundation, Inc.
+# Copyright 2022, 2023 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -85,7 +85,7 @@ def _set_breakpoints(kind, specs):
 
 
 @request("setBreakpoints")
-def set_breakpoint(source, *, breakpoints=[], **args):
+def set_breakpoint(*, source, breakpoints=[], **args):
     if "path" not in source:
         result = []
     else:
@@ -108,7 +108,7 @@ def set_breakpoint(source, *, breakpoints=[], **args):
 
 @request("setFunctionBreakpoints")
 @capability("supportsFunctionBreakpoints")
-def set_fn_breakpoint(breakpoints, **args):
+def set_fn_breakpoint(*, breakpoints, **args):
     specs = []
     for bp in breakpoints:
         specs.append(
diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index c05e62d17a3..f01bf0f33c9 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -1,4 +1,4 @@
-# Copyright 2022 Free Software Foundation, Inc.
+# Copyright 2022, 2023 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -33,7 +33,7 @@ def _evaluate(expr, frame_id):
 # FIXME return a structured response using pretty-printers / varobj
 # FIXME supportsVariableType handling
 @request("evaluate")
-def eval_request(expression, *, frameId=None, **args):
+def eval_request(*, expression, frameId=None, **args):
     result = send_gdb_with_response(lambda: _evaluate(expression, frameId))
     return {
         "result": result,
-- 
2.39.1

Re: [PATCH] Ensure all DAP requests are keyword-only

[Simon Marchi]

On 2/10/23 14:00, Tom Tromey via Gdb-patches wrote:
> Python functions implementing DAP requests should not use positional
> parameters -- it only makes sense to call them with keyword arguments.
> This patch changes the few remaining cases to start with the special
> "*" parameter, following this rule.
> ---
>  gdb/python/lib/gdb/dap/breakpoint.py | 6 +++---
>  gdb/python/lib/gdb/dap/evaluate.py   | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
> index 502beb0478e..f0e1f103d1b 100644
> --- a/gdb/python/lib/gdb/dap/breakpoint.py
> +++ b/gdb/python/lib/gdb/dap/breakpoint.py
> @@ -1,4 +1,4 @@
> -# Copyright 2022 Free Software Foundation, Inc.
> +# Copyright 2022, 2023 Free Software Foundation, Inc.
>  
>  # This program is free software; you can redistribute it and/or modify
>  # it under the terms of the GNU General Public License as published by
> @@ -85,7 +85,7 @@ def _set_breakpoints(kind, specs):
>  
>  
>  @request("setBreakpoints")
> -def set_breakpoint(source, *, breakpoints=[], **args):
> +def set_breakpoint(*, source, breakpoints=[], **args):
>      if "path" not in source:
>          result = []
>      else:

IIUC , this function is called magically when receiving a setBreakpoints
request, and "source" comes from the request arguments?

If so, the change makes sense to me, as there are no positional
arguments in DAP requests.

Simon

Re: [PATCH] Ensure all DAP requests are keyword-only

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> -def set_breakpoint(source, *, breakpoints=[], **args):
>> +def set_breakpoint(*, source, breakpoints=[], **args):
>> if "path" not in source:
>> result = []
>> else:

Simon> IIUC , this function is called magically when receiving a setBreakpoints
Simon> request, and "source" comes from the request arguments?

Yes, that's right.  server.py does:

            body = _commands[params["command"]](**args)

Now, in theory it's possible to just call functions like this directly
from some other python code.  However, I think we should generally
prohibit this, and if it's needed, refactor to have some helper
function, to avoid combining protocol-layer code with implementation
code.

Simon> If so, the change makes sense to me, as there are no positional
Simon> arguments in DAP requests.

Thanks, I'm going to check it in.

Tom