public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py
@ 2023-02-23 22:18 Simon Marchi
  2023-02-23 22:18 ` [PATCH 1/9] gdb: remove invalid / dead code from gdbarch.py Simon Marchi
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Simon Marchi @ 2023-02-23 22:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

While investigating something related to a gdbarch method, I was trying
to understand the purpose of the "invalid" Function parameter.  It is
sometimes passed a boolean and sometimes passed a string.  This is
indeed described in the comment at the top of gdbarch-components.py, but
I then thought that if we used type annotations, it would make it clear
that it's intended.  Also, I find type annotations (when paired with a
checker such as pyright or pylance) really handy to avoid all kinds of
mistake.  So I got a bit carried away, and this series is the result.

Simon Marchi (9):
  gdb: remove invalid / dead code from gdbarch.py
  gdb: reformat Python files with black 23.1.0
  gdb: gdbarch.py: spell out parameters of _Component.__init__
  gdb: pyproject.toml: set pyright typeCheckingMode = "strict"
  gdb: split gdbarch component types to gdbarch_types.py
  gdb: gdbarch*.py, copyright.py: add type annotations
  gdb: make-target-delegates.py: make one string raw
  gdb: make-target-delegates.py: add Entry type
  gdb: make-target-delegates.py: add type annotations

 gdb/gdbarch.py                                | 161 +--------------
 ...ch-components.py => gdbarch_components.py} |   8 +-
 gdb/gdbarch_types.py                          | 189 ++++++++++++++++++
 gdb/gdbcopyright.py                           |   2 +-
 gdb/make-target-delegates.py                  | 104 +++++++---
 gdb/pyproject.toml                            |   3 +
 gdb/python/lib/gdb/FrameDecorator.py          |   1 -
 gdb/python/lib/gdb/command/frame_filters.py   |   1 +
 gdb/python/lib/gdb/dap/server.py              |   2 +-
 gdb/python/lib/gdb/printing.py                |   2 +-
 .../gdb.multi/multi-target-info-inferiors.py  |   1 +
 gdb/testsuite/gdb.perf/backtrace.py           |   1 -
 .../gdb.python/py-framefilter-addr.py         |   1 +
 gdb/testsuite/gdb.python/py-framefilter.py    |   1 +
 gdb/testsuite/gdb.python/py-inferior-leak.py  |   2 +
 gdb/testsuite/gdb.python/py-prettyprint.py    |   1 +
 gdb/testsuite/gdb.python/py-recurse-unwind.py |   5 -
 gdb/testsuite/gdb.python/py-send-packet.py    |   1 +
 gdb/testsuite/gdb.python/tui-window.py        |   1 +
 19 files changed, 290 insertions(+), 197 deletions(-)
 rename gdb/{gdbarch-components.py => gdbarch_components.py} (99%)
 create mode 100755 gdb/gdbarch_types.py

-- 
2.39.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/9] gdb: remove invalid / dead code from gdbarch.py
  2023-02-23 22:18 [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Simon Marchi
@ 2023-02-23 22:18 ` Simon Marchi
  2023-02-23 22:18 ` [PATCH 2/9] gdb: reformat Python files with black 23.1.0 Simon Marchi
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Simon Marchi @ 2023-02-23 22:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

My editor flagged that the variable `c` (in the lines removed by this
patch) was unknown.  I guess it ends up working because there is a `c`
variable in the global scope.  I tried putting `assert False` inside
that if, and it is not hit, showing that we never enter this if.  So,
remove it.  There is no change in the generated files.

Change-Id: Id3b9f67719e88cada7c6fde673c8d7842ab13617
---
 gdb/gdbarch.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/gdb/gdbarch.py b/gdb/gdbarch.py
index 3ebc35980472..68c7bbae6618 100755
--- a/gdb/gdbarch.py
+++ b/gdb/gdbarch.py
@@ -64,8 +64,6 @@ class _Component:
         assert self.predicate and not isinstance(self.invalid, str)
         if self.predefault:
             predicate = f"gdbarch->{self.name} != {self.predefault}"
-        elif isinstance(c, Value):
-            predicate = f"gdbarch->{self.name} != 0"
         else:
             predicate = f"gdbarch->{self.name} != NULL"
         return predicate
-- 
2.39.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/9] gdb: reformat Python files with black 23.1.0
  2023-02-23 22:18 [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Simon Marchi
  2023-02-23 22:18 ` [PATCH 1/9] gdb: remove invalid / dead code from gdbarch.py Simon Marchi
@ 2023-02-23 22:18 ` Simon Marchi
  2023-02-23 22:18 ` [PATCH 3/9] gdb: gdbarch.py: spell out parameters of _Component.__init__ Simon Marchi
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Simon Marchi @ 2023-02-23 22:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

Change-Id: Ie8ec8870a16d71c5858f5d08958309d23c318302
---
 gdb/python/lib/gdb/FrameDecorator.py                   | 1 -
 gdb/python/lib/gdb/command/frame_filters.py            | 1 +
 gdb/python/lib/gdb/dap/server.py                       | 2 +-
 gdb/python/lib/gdb/printing.py                         | 2 +-
 gdb/testsuite/gdb.multi/multi-target-info-inferiors.py | 1 +
 gdb/testsuite/gdb.perf/backtrace.py                    | 1 -
 gdb/testsuite/gdb.python/py-framefilter-addr.py        | 1 +
 gdb/testsuite/gdb.python/py-framefilter.py             | 1 +
 gdb/testsuite/gdb.python/py-inferior-leak.py           | 2 ++
 gdb/testsuite/gdb.python/py-prettyprint.py             | 1 +
 gdb/testsuite/gdb.python/py-recurse-unwind.py          | 5 -----
 gdb/testsuite/gdb.python/py-send-packet.py             | 1 +
 gdb/testsuite/gdb.python/tui-window.py                 | 1 +
 13 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/gdb/python/lib/gdb/FrameDecorator.py b/gdb/python/lib/gdb/FrameDecorator.py
index 82be4fc4ecb7..6773780735b4 100644
--- a/gdb/python/lib/gdb/FrameDecorator.py
+++ b/gdb/python/lib/gdb/FrameDecorator.py
@@ -64,7 +64,6 @@ class FrameDecorator(object):
             or frame.type() == gdb.DUMMY_FRAME
             or frame.type() == gdb.SIGTRAMP_FRAME
         ):
-
             return True
 
         return False
diff --git a/gdb/python/lib/gdb/command/frame_filters.py b/gdb/python/lib/gdb/command/frame_filters.py
index 0fca0e78dcbf..a88de49cfea1 100644
--- a/gdb/python/lib/gdb/command/frame_filters.py
+++ b/gdb/python/lib/gdb/command/frame_filters.py
@@ -20,6 +20,7 @@ import sys
 import gdb
 import gdb.frames
 
+
 # GDB Commands.
 class SetFilterPrefixCmd(gdb.Command):
     """Prefix command for 'set' frame-filter related operations."""
diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index f8cb6170a969..7013cc3bff14 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -116,7 +116,7 @@ class Server:
             self._send_json(result)
             events = self.delayed_events
             self.delayed_events = []
-            for (event, body) in events:
+            for event, body in events:
                 self.send_event(event, body)
         # Got the terminate request.  This is handled by the
         # JSON-writing thread, so that we can ensure that all
diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py
index 740a96e60eb4..1f724eebb2b8 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -228,7 +228,7 @@ class _EnumInstance:
         flag_list = []
         v = int(self.val)
         any_found = False
-        for (e_name, e_value) in self.enumerators:
+        for e_name, e_value in self.enumerators:
             if v & e_value != 0:
                 flag_list.append(e_name)
                 v = v & ~e_value
diff --git a/gdb/testsuite/gdb.multi/multi-target-info-inferiors.py b/gdb/testsuite/gdb.multi/multi-target-info-inferiors.py
index 585fee3f183b..c58045e1866b 100644
--- a/gdb/testsuite/gdb.multi/multi-target-info-inferiors.py
+++ b/gdb/testsuite/gdb.multi/multi-target-info-inferiors.py
@@ -15,6 +15,7 @@
 
 import gdb
 
+
 # Take a gdb.TargetConnection and return the connection number.
 def conn_num(c):
     return c.num
diff --git a/gdb/testsuite/gdb.perf/backtrace.py b/gdb/testsuite/gdb.perf/backtrace.py
index 01c4eed4ce84..df94a592f687 100644
--- a/gdb/testsuite/gdb.perf/backtrace.py
+++ b/gdb/testsuite/gdb.perf/backtrace.py
@@ -33,7 +33,6 @@ class BackTrace(perftest.TestCaseWithBasicMeasurements):
             gdb.execute(do_test_command, False, True)
 
     def execute_test(self):
-
         line_size = 2
         for _ in range(1, 12):
             # Keep the total size of dcache unchanged, and increase the
diff --git a/gdb/testsuite/gdb.python/py-framefilter-addr.py b/gdb/testsuite/gdb.python/py-framefilter-addr.py
index a385abca69fd..f7702a2cf482 100644
--- a/gdb/testsuite/gdb.python/py-framefilter-addr.py
+++ b/gdb/testsuite/gdb.python/py-framefilter-addr.py
@@ -18,6 +18,7 @@ import itertools
 from gdb.FrameDecorator import FrameDecorator
 import copy
 
+
 # A FrameDecorator that just returns gdb.Frame.pc () from 'function'.
 # We want to ensure that GDB correctly handles this case.
 class Function_Returns_Address(FrameDecorator):
diff --git a/gdb/testsuite/gdb.python/py-framefilter.py b/gdb/testsuite/gdb.python/py-framefilter.py
index b7c18d3e4c79..c82b88b56cff 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.py
+++ b/gdb/testsuite/gdb.python/py-framefilter.py
@@ -139,6 +139,7 @@ class FrameElider:
 # thrown.
 name_error = RuntimeError
 
+
 # A simple decorator that gives an error when computing the function.
 class ErrorInName(FrameDecorator):
     def __init__(self, frame):
diff --git a/gdb/testsuite/gdb.python/py-inferior-leak.py b/gdb/testsuite/gdb.python/py-inferior-leak.py
index dbdf160dc6c2..c26a546943d3 100644
--- a/gdb/testsuite/gdb.python/py-inferior-leak.py
+++ b/gdb/testsuite/gdb.python/py-inferior-leak.py
@@ -21,6 +21,7 @@ import re
 # object sent to us in the new_inferior event.
 inf = None
 
+
 # Register the new_inferior event handler.
 def new_inferior_handler(event):
     global inf
@@ -33,6 +34,7 @@ gdb.events.new_inferior.connect(new_inferior_handler)
 # originating from this script.
 filters = [tracemalloc.Filter(True, "*py-inferior-leak.py")]
 
+
 # Add a new inferior, and return the number of the new inferior.
 def add_inferior():
     output = gdb.execute("add-inferior", False, True)
diff --git a/gdb/testsuite/gdb.python/py-prettyprint.py b/gdb/testsuite/gdb.python/py-prettyprint.py
index 20d31dc6dbf8..7dc02d84d9ad 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint.py
+++ b/gdb/testsuite/gdb.python/py-prettyprint.py
@@ -89,6 +89,7 @@ class ArrayPrinter(object):
 # Flag to make NoStringContainerPrinter throw an exception.
 exception_flag = False
 
+
 # Test a printer where to_string is None
 class NoStringContainerPrinter(object):
     def __init__(self, val):
diff --git a/gdb/testsuite/gdb.python/py-recurse-unwind.py b/gdb/testsuite/gdb.python/py-recurse-unwind.py
index 96d1badf6648..d73949310bcf 100644
--- a/gdb/testsuite/gdb.python/py-recurse-unwind.py
+++ b/gdb/testsuite/gdb.python/py-recurse-unwind.py
@@ -30,7 +30,6 @@ from gdb.unwinder import Unwinder
 
 
 class TestUnwinder(Unwinder):
-
     count = 0
 
     @classmethod
@@ -52,7 +51,6 @@ class TestUnwinder(Unwinder):
         self.recurse_level = 0
 
     def __call__(self, pending_frame):
-
         if self.recurse_level > 0:
             gdb.write("TestUnwinder: Recursion detected - returning early.\n")
             return None
@@ -61,19 +59,16 @@ class TestUnwinder(Unwinder):
         TestUnwinder.inc_count()
 
         if TestUnwinder.test == "check_user_reg_pc":
-
             pc = pending_frame.read_register("pc")
             pc_as_int = int(pc.cast(gdb.lookup_type("int")))
             # gdb.write("In unwinder: pc=%x\n" % pc_as_int)
 
         elif TestUnwinder.test == "check_pae_pc":
-
             pc = gdb.parse_and_eval("$pc")
             pc_as_int = int(pc.cast(gdb.lookup_type("int")))
             # gdb.write("In unwinder: pc=%x\n" % pc_as_int)
 
         elif TestUnwinder.test == "check_undefined_symbol":
-
             try:
                 val = gdb.parse_and_eval("undefined_symbol")
 
diff --git a/gdb/testsuite/gdb.python/py-send-packet.py b/gdb/testsuite/gdb.python/py-send-packet.py
index 08e2f1e7bef0..fd42f8b786a9 100644
--- a/gdb/testsuite/gdb.python/py-send-packet.py
+++ b/gdb/testsuite/gdb.python/py-send-packet.py
@@ -16,6 +16,7 @@
 import xml.etree.ElementTree as ET
 import gdb
 
+
 # Make use of gdb.RemoteTargetConnection.send_packet to fetch the
 # thread list from the remote target.
 #
diff --git a/gdb/testsuite/gdb.python/tui-window.py b/gdb/testsuite/gdb.python/tui-window.py
index 3d9ada56b5a2..401cb5197ff1 100644
--- a/gdb/testsuite/gdb.python/tui-window.py
+++ b/gdb/testsuite/gdb.python/tui-window.py
@@ -42,6 +42,7 @@ class TestWindow:
 
 gdb.register_window_type("test", TestWindow)
 
+
 # Call REMOVE_TITLE on the global window object.
 def delete_window_title():
     the_window.remove_title()
-- 
2.39.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/9] gdb: gdbarch.py: spell out parameters of _Component.__init__
  2023-02-23 22:18 [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Simon Marchi
  2023-02-23 22:18 ` [PATCH 1/9] gdb: remove invalid / dead code from gdbarch.py Simon Marchi
  2023-02-23 22:18 ` [PATCH 2/9] gdb: reformat Python files with black 23.1.0 Simon Marchi
@ 2023-02-23 22:18 ` Simon Marchi
  2023-02-24 19:51   ` Tom Tromey
  2023-02-23 22:18 ` [PATCH 4/9] gdb: pyproject.toml: set pyright typeCheckingMode = "strict" Simon Marchi
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Simon Marchi @ 2023-02-23 22:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

The way _Component uses kwargs is handy to save a few characters, but it
doesn't play well with static analysis.  When editing gdbarch.py, my
editor (which uses pylance under the hood) knows nothing about the
properties of components.  So it's full of squiggly lines, and typing
analysis (which I find really helpful) doesn't work.  I therefore think
it would be better to spell out the parameters.

Change-Id: Iaf561beb0d0fbe170ce1c79252a291e0945e1830
---
 gdb/gdbarch.py | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/gdb/gdbarch.py b/gdb/gdbarch.py
index 68c7bbae6618..63c3aee1dc0e 100755
--- a/gdb/gdbarch.py
+++ b/gdb/gdbarch.py
@@ -49,9 +49,34 @@ def join_params(params):
 class _Component:
     "Base class for all components."
 
-    def __init__(self, **kwargs):
-        for key in kwargs:
-            setattr(self, key, kwargs[key])
+    def __init__(
+        self,
+        name,
+        type,
+        printer,
+        comment=None,
+        predicate=False,
+        predefault=None,
+        postdefault=None,
+        invalid=None,
+        params=None,
+        param_checks=None,
+        result_checks=None,
+        implement=True,
+    ):
+        self.name = name
+        self.type = type
+        self.printer = printer
+        self.comment = comment
+        self.predicate = predicate
+        self.predefault = predefault
+        self.postdefault = postdefault
+        self.invalid = invalid
+        self.params = params
+        self.param_checks = param_checks
+        self.result_checks = result_checks
+        self.implement = implement
+
         components.append(self)
 
         # It doesn't make sense to have a check of the result value
@@ -87,7 +112,7 @@ class Value(_Component):
         name,
         type,
         comment=None,
-        predicate=None,
+        predicate=False,
         predefault=None,
         postdefault=None,
         invalid=None,
@@ -115,7 +140,7 @@ class Function(_Component):
         type,
         params,
         comment=None,
-        predicate=None,
+        predicate=False,
         predefault=None,
         postdefault=None,
         invalid=None,
-- 
2.39.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 4/9] gdb: pyproject.toml: set pyright typeCheckingMode = "strict"
  2023-02-23 22:18 [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Simon Marchi
                   ` (2 preceding siblings ...)
  2023-02-23 22:18 ` [PATCH 3/9] gdb: gdbarch.py: spell out parameters of _Component.__init__ Simon Marchi
@ 2023-02-23 22:18 ` Simon Marchi
  2023-02-23 22:18 ` [PATCH 5/9] gdb: split gdbarch component types to gdbarch_types.py Simon Marchi
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Simon Marchi @ 2023-02-23 22:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

While working on other projects, I found the pyright type checker very
helpful when editing Python code.  I don't think I have to explain the
advantages of type checking to a crowd used to C/C++.

Setting typeCheckingMode to "strict" makes pyright flag a bit more type
issues than the default of "basic".

Change-Id: I38818ec59f7f73c2ab020cc9226286cdd485abc7
---
 gdb/pyproject.toml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdb/pyproject.toml b/gdb/pyproject.toml
index 58ed2f9d5564..4469f1cd61df 100644
--- a/gdb/pyproject.toml
+++ b/gdb/pyproject.toml
@@ -1,2 +1,5 @@
 [tool.black]
 include = "\\.py(\\.in)?$"
+
+[tool.pyright]
+typeCheckingMode = "strict"
-- 
2.39.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 5/9] gdb: split gdbarch component types to gdbarch_types.py
  2023-02-23 22:18 [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Simon Marchi
                   ` (3 preceding siblings ...)
  2023-02-23 22:18 ` [PATCH 4/9] gdb: pyproject.toml: set pyright typeCheckingMode = "strict" Simon Marchi
@ 2023-02-23 22:18 ` Simon Marchi
  2023-02-23 22:18 ` [PATCH 6/9] gdb: gdbarch*.py, copyright.py: add type annotations Simon Marchi
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Simon Marchi @ 2023-02-23 22:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

Editing gdbarch-components.py is not an experience in an editor that is
minimally smart about Python.  Because gdbarch-components.py is read and
exec'd by gdbarch.py, it doesn't import the  Info / Method / Function /
Value types.  And because these types are defined in gdbarch.py, it
can't import them, as that would make a cyclic dependency.

Solve this by introducing a third file, gdbarch_types.py, to define
these types.  Make gdbarch.py and gdbarch-components.py import it.
Also, replace the read & exec of gdbarch-components.py by a regular
import.  For this to work though, gdbarch-components.py needs to be
renamed to gdbarch_components.py.

Change-Id: Ibe994d56ef9efcc0698b3ca9670d4d9bf8bbb853
---
 gdb/gdbarch.py                                | 178 +----------------
 ...ch-components.py => gdbarch_components.py} |   2 +
 gdb/gdbarch_types.py                          | 188 ++++++++++++++++++
 3 files changed, 195 insertions(+), 173 deletions(-)
 rename gdb/{gdbarch-components.py => gdbarch_components.py} (99%)
 create mode 100755 gdb/gdbarch_types.py

diff --git a/gdb/gdbarch.py b/gdb/gdbarch.py
index 63c3aee1dc0e..d1ac414d0eb3 100755
--- a/gdb/gdbarch.py
+++ b/gdb/gdbarch.py
@@ -20,10 +20,12 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import textwrap
-import gdbcopyright
 
-# All the components created in gdbarch-components.py.
-components = []
+# gdbarch_components is imported only for its side-effect of filling
+# `gdbarch_types.components`.
+import gdbarch_components  # noqa: F401 # type: ignore
+import gdbcopyright
+from gdbarch_types import Function, Info, Value, components
 
 
 def indentation(n_columns):
@@ -31,176 +33,6 @@ def indentation(n_columns):
     return "\t" * (n_columns // 8) + " " * (n_columns % 8)
 
 
-def join_type_and_name(t, n):
-    "Combine the type T and the name N into a C declaration."
-    if t.endswith("*") or t.endswith("&"):
-        return t + n
-    else:
-        return t + " " + n
-
-
-def join_params(params):
-    """Given a sequence of (TYPE, NAME) pairs, generate a comma-separated
-    list of declarations."""
-    params = [join_type_and_name(p[0], p[1]) for p in params]
-    return ", ".join(params)
-
-
-class _Component:
-    "Base class for all components."
-
-    def __init__(
-        self,
-        name,
-        type,
-        printer,
-        comment=None,
-        predicate=False,
-        predefault=None,
-        postdefault=None,
-        invalid=None,
-        params=None,
-        param_checks=None,
-        result_checks=None,
-        implement=True,
-    ):
-        self.name = name
-        self.type = type
-        self.printer = printer
-        self.comment = comment
-        self.predicate = predicate
-        self.predefault = predefault
-        self.postdefault = postdefault
-        self.invalid = invalid
-        self.params = params
-        self.param_checks = param_checks
-        self.result_checks = result_checks
-        self.implement = implement
-
-        components.append(self)
-
-        # It doesn't make sense to have a check of the result value
-        # for a function or method with void return type.
-        if self.type == "void" and self.result_checks:
-            raise Exception("can't have result checks with a void return type")
-
-    def get_predicate(self):
-        "Return the expression used for validity checking."
-        assert self.predicate and not isinstance(self.invalid, str)
-        if self.predefault:
-            predicate = f"gdbarch->{self.name} != {self.predefault}"
-        else:
-            predicate = f"gdbarch->{self.name} != NULL"
-        return predicate
-
-
-class Info(_Component):
-    "An Info component is copied from the gdbarch_info."
-
-    def __init__(self, *, name, type, printer=None):
-        super().__init__(name=name, type=type, printer=printer)
-        # This little hack makes the generator a bit simpler.
-        self.predicate = None
-
-
-class Value(_Component):
-    "A Value component is just a data member."
-
-    def __init__(
-        self,
-        *,
-        name,
-        type,
-        comment=None,
-        predicate=False,
-        predefault=None,
-        postdefault=None,
-        invalid=None,
-        printer=None,
-    ):
-        super().__init__(
-            comment=comment,
-            name=name,
-            type=type,
-            predicate=predicate,
-            predefault=predefault,
-            postdefault=postdefault,
-            invalid=invalid,
-            printer=printer,
-        )
-
-
-class Function(_Component):
-    "A Function component is a function pointer member."
-
-    def __init__(
-        self,
-        *,
-        name,
-        type,
-        params,
-        comment=None,
-        predicate=False,
-        predefault=None,
-        postdefault=None,
-        invalid=None,
-        printer=None,
-        param_checks=None,
-        result_checks=None,
-        implement=True,
-    ):
-        super().__init__(
-            comment=comment,
-            name=name,
-            type=type,
-            predicate=predicate,
-            predefault=predefault,
-            postdefault=postdefault,
-            invalid=invalid,
-            printer=printer,
-            params=params,
-            param_checks=param_checks,
-            result_checks=result_checks,
-            implement=implement,
-        )
-
-    def ftype(self):
-        "Return the name of the function typedef to use."
-        return f"gdbarch_{self.name}_ftype"
-
-    def param_list(self):
-        "Return the formal parameter list as a string."
-        return join_params(self.params)
-
-    def set_list(self):
-        """Return the formal parameter list of the caller function,
-        as a string.  This list includes the gdbarch."""
-        arch_arg = ("struct gdbarch *", "gdbarch")
-        arch_tuple = [arch_arg]
-        return join_params(arch_tuple + list(self.params))
-
-    def actuals(self):
-        "Return the actual parameters to forward, as a string."
-        return ", ".join([p[1] for p in self.params])
-
-
-class Method(Function):
-    "A Method is like a Function but passes the gdbarch through."
-
-    def param_list(self):
-        "See superclass."
-        return self.set_list()
-
-    def actuals(self):
-        "See superclass."
-        result = ["gdbarch"] + [p[1] for p in self.params]
-        return ", ".join(result)
-
-
-# Read the components.
-with open("gdbarch-components.py") as fd:
-    exec(fd.read())
-
 copyright = gdbcopyright.copyright(
     "gdbarch.py", "Dynamic architecture support for GDB, the GNU debugger."
 )
diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch_components.py
similarity index 99%
rename from gdb/gdbarch-components.py
rename to gdb/gdbarch_components.py
index 76ad2832d8a2..fe5c3b3b4bcd 100644
--- a/gdb/gdbarch-components.py
+++ b/gdb/gdbarch_components.py
@@ -117,6 +117,8 @@
 # * "implement" - optional, a boolean.  If True (the default), a
 # wrapper function for this function will be emitted.
 
+from gdbarch_types import Function, Info, Method, Value
+
 Info(
     type="const struct bfd_arch_info *",
     name="bfd_arch_info",
diff --git a/gdb/gdbarch_types.py b/gdb/gdbarch_types.py
new file mode 100755
index 000000000000..95d9b5e65bc5
--- /dev/null
+++ b/gdb/gdbarch_types.py
@@ -0,0 +1,188 @@
+# Architecture commands for GDB, the GNU debugger.
+#
+# Copyright (C) 1998-2023 Free Software Foundation, Inc.
+#
+# This file is part of GDB.
+#
+# 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
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+def join_type_and_name(t, n):
+    "Combine the type T and the name N into a C declaration."
+    if t.endswith("*") or t.endswith("&"):
+        return t + n
+    else:
+        return t + " " + n
+
+
+def join_params(params):
+    """Given a sequence of (TYPE, NAME) pairs, generate a comma-separated
+    list of declarations."""
+    params = [join_type_and_name(p[0], p[1]) for p in params]
+    return ", ".join(params)
+
+
+class _Component:
+    "Base class for all components."
+
+    def __init__(
+        self,
+        name,
+        type,
+        printer,
+        comment=None,
+        predicate=False,
+        predefault=None,
+        postdefault=None,
+        invalid=None,
+        params=None,
+        param_checks=None,
+        result_checks=None,
+        implement=True,
+    ):
+        self.name = name
+        self.type = type
+        self.printer = printer
+        self.comment = comment
+        self.predicate = predicate
+        self.predefault = predefault
+        self.postdefault = postdefault
+        self.invalid = invalid
+        self.params = params
+        self.param_checks = param_checks
+        self.result_checks = result_checks
+        self.implement = implement
+
+        components.append(self)
+
+        # It doesn't make sense to have a check of the result value
+        # for a function or method with void return type.
+        if self.type == "void" and self.result_checks:
+            raise Exception("can't have result checks with a void return type")
+
+    def get_predicate(self):
+        "Return the expression used for validity checking."
+        assert self.predicate and not isinstance(self.invalid, str)
+        if self.predefault:
+            predicate = f"gdbarch->{self.name} != {self.predefault}"
+        else:
+            predicate = f"gdbarch->{self.name} != NULL"
+        return predicate
+
+
+class Info(_Component):
+    "An Info component is copied from the gdbarch_info."
+
+    def __init__(self, *, name, type, printer=None):
+        super().__init__(name=name, type=type, printer=printer)
+        # This little hack makes the generator a bit simpler.
+        self.predicate = None
+
+
+class Value(_Component):
+    "A Value component is just a data member."
+
+    def __init__(
+        self,
+        *,
+        name,
+        type,
+        comment=None,
+        predicate=False,
+        predefault=None,
+        postdefault=None,
+        invalid=None,
+        printer=None,
+    ):
+        super().__init__(
+            comment=comment,
+            name=name,
+            type=type,
+            predicate=predicate,
+            predefault=predefault,
+            postdefault=postdefault,
+            invalid=invalid,
+            printer=printer,
+        )
+
+
+class Function(_Component):
+    "A Function component is a function pointer member."
+
+    def __init__(
+        self,
+        *,
+        name,
+        type,
+        params,
+        comment=None,
+        predicate=False,
+        predefault=None,
+        postdefault=None,
+        invalid=None,
+        printer=None,
+        param_checks=None,
+        result_checks=None,
+        implement=True,
+    ):
+        super().__init__(
+            comment=comment,
+            name=name,
+            type=type,
+            predicate=predicate,
+            predefault=predefault,
+            postdefault=postdefault,
+            invalid=invalid,
+            printer=printer,
+            params=params,
+            param_checks=param_checks,
+            result_checks=result_checks,
+            implement=implement,
+        )
+
+    def ftype(self):
+        "Return the name of the function typedef to use."
+        return f"gdbarch_{self.name}_ftype"
+
+    def param_list(self):
+        "Return the formal parameter list as a string."
+        return join_params(self.params)
+
+    def set_list(self):
+        """Return the formal parameter list of the caller function,
+        as a string.  This list includes the gdbarch."""
+        arch_arg = ("struct gdbarch *", "gdbarch")
+        arch_tuple = [arch_arg]
+        return join_params(arch_tuple + list(self.params))
+
+    def actuals(self):
+        "Return the actual parameters to forward, as a string."
+        return ", ".join([p[1] for p in self.params])
+
+
+class Method(Function):
+    "A Method is like a Function but passes the gdbarch through."
+
+    def param_list(self):
+        "See superclass."
+        return self.set_list()
+
+    def actuals(self):
+        "See superclass."
+        result = ["gdbarch"] + [p[1] for p in self.params]
+        return ", ".join(result)
+
+
+# All the components created in gdbarch-components.py.
+components: list[_Component] = []
-- 
2.39.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 6/9] gdb: gdbarch*.py, copyright.py: add type annotations
  2023-02-23 22:18 [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Simon Marchi
                   ` (4 preceding siblings ...)
  2023-02-23 22:18 ` [PATCH 5/9] gdb: split gdbarch component types to gdbarch_types.py Simon Marchi
@ 2023-02-23 22:18 ` Simon Marchi
  2023-02-23 22:18 ` [PATCH 7/9] gdb: make-target-delegates.py: make one string raw Simon Marchi
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Simon Marchi @ 2023-02-23 22:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

Add type annotations to gdbarch*.py to fix all errors shown by pyright.
There is one change in copyright.py too, to fix this one:

    /home/simark/src/binutils-gdb/gdb/gdbarch.py
      /home/simark/src/binutils-gdb/gdb/gdbarch.py:206:13 - error: Type of "copyright" is partially unknown
        Type of "copyright" is "(tool: Unknown, description: Unknown) -> str" (reportUnknownMemberType)

Change-Id: Ia109b53e267f6e2f5bd79a1288d0d5c9508c9ac4
---
 gdb/gdbarch.py            |  8 ++--
 gdb/gdbarch_components.py |  6 +--
 gdb/gdbarch_types.py      | 87 ++++++++++++++++++++-------------------
 gdb/gdbcopyright.py       |  2 +-
 4 files changed, 52 insertions(+), 51 deletions(-)

diff --git a/gdb/gdbarch.py b/gdb/gdbarch.py
index d1ac414d0eb3..93b1e8bf84e2 100755
--- a/gdb/gdbarch.py
+++ b/gdb/gdbarch.py
@@ -25,10 +25,10 @@ import textwrap
 # `gdbarch_types.components`.
 import gdbarch_components  # noqa: F401 # type: ignore
 import gdbcopyright
-from gdbarch_types import Function, Info, Value, components
+from gdbarch_types import Component, Function, Info, Value, components
 
 
-def indentation(n_columns):
+def indentation(n_columns: int):
     """Return string with tabs and spaces to indent line to N_COLUMNS."""
     return "\t" * (n_columns // 8) + " " * (n_columns % 8)
 
@@ -38,12 +38,12 @@ copyright = gdbcopyright.copyright(
 )
 
 
-def info(c):
+def info(c: Component):
     "Filter function to only allow Info components."
     return type(c) is Info
 
 
-def not_info(c):
+def not_info(c: Component):
     "Filter function to omit Info components."
     return type(c) is not Info
 
diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
index fe5c3b3b4bcd..c956586e82c0 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -165,14 +165,14 @@ Number of bits in an int or unsigned int for the target machine.
     predefault="4*TARGET_CHAR_BIT",
     invalid=False,
 )
-
+long_bit_predefault = "4*TARGET_CHAR_BIT"
 long_bit = Value(
     comment="""
 Number of bits in a long or unsigned long for the target machine.
 """,
     type="int",
     name="long_bit",
-    predefault="4*TARGET_CHAR_BIT",
+    predefault=long_bit_predefault,
     invalid=False,
 )
 
@@ -183,7 +183,7 @@ machine.
 """,
     type="int",
     name="long_long_bit",
-    predefault="2*" + long_bit.predefault,
+    predefault="2*" + long_bit_predefault,
     invalid=False,
 )
 
diff --git a/gdb/gdbarch_types.py b/gdb/gdbarch_types.py
index 95d9b5e65bc5..6b1483c77331 100755
--- a/gdb/gdbarch_types.py
+++ b/gdb/gdbarch_types.py
@@ -17,8 +17,10 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+from typing import Optional, Union
 
-def join_type_and_name(t, n):
+
+def join_type_and_name(t: str, n: str):
     "Combine the type T and the name N into a C declaration."
     if t.endswith("*") or t.endswith("&"):
         return t + n
@@ -26,30 +28,29 @@ def join_type_and_name(t, n):
         return t + " " + n
 
 
-def join_params(params):
+def join_params(params: list[tuple[str, str]]):
     """Given a sequence of (TYPE, NAME) pairs, generate a comma-separated
     list of declarations."""
-    params = [join_type_and_name(p[0], p[1]) for p in params]
-    return ", ".join(params)
+    return ", ".join([join_type_and_name(p[0], p[1]) for p in params])
 
 
-class _Component:
+class Component:
     "Base class for all components."
 
     def __init__(
         self,
-        name,
-        type,
-        printer,
-        comment=None,
-        predicate=False,
-        predefault=None,
-        postdefault=None,
-        invalid=None,
-        params=None,
-        param_checks=None,
-        result_checks=None,
-        implement=True,
+        name: str,
+        type: str,
+        printer: Optional[str],
+        comment: Optional[str] = None,
+        predicate: bool = False,
+        predefault: Optional[str] = None,
+        postdefault: Optional[str] = None,
+        invalid: Optional[Union[bool, str]] = None,
+        params: Optional[list[tuple[str, str]]] = None,
+        param_checks: Optional[list[str]] = None,
+        result_checks: Optional[list[str]] = None,
+        implement: bool = True,
     ):
         self.name = name
         self.type = type
@@ -59,7 +60,7 @@ class _Component:
         self.predefault = predefault
         self.postdefault = postdefault
         self.invalid = invalid
-        self.params = params
+        self.params = params or []
         self.param_checks = param_checks
         self.result_checks = result_checks
         self.implement = implement
@@ -81,29 +82,29 @@ class _Component:
         return predicate
 
 
-class Info(_Component):
+class Info(Component):
     "An Info component is copied from the gdbarch_info."
 
-    def __init__(self, *, name, type, printer=None):
+    def __init__(self, *, name: str, type: str, printer: Optional[str] = None):
         super().__init__(name=name, type=type, printer=printer)
         # This little hack makes the generator a bit simpler.
         self.predicate = None
 
 
-class Value(_Component):
+class Value(Component):
     "A Value component is just a data member."
 
     def __init__(
         self,
         *,
-        name,
-        type,
-        comment=None,
-        predicate=False,
-        predefault=None,
-        postdefault=None,
-        invalid=None,
-        printer=None,
+        name: str,
+        type: str,
+        comment: Optional[str] = None,
+        predicate: bool = False,
+        predefault: Optional[str] = None,
+        postdefault: Optional[str] = None,
+        invalid: Optional[Union[bool, str]] = None,
+        printer: Optional[str] = None,
     ):
         super().__init__(
             comment=comment,
@@ -117,24 +118,24 @@ class Value(_Component):
         )
 
 
-class Function(_Component):
+class Function(Component):
     "A Function component is a function pointer member."
 
     def __init__(
         self,
         *,
-        name,
-        type,
-        params,
-        comment=None,
-        predicate=False,
-        predefault=None,
-        postdefault=None,
-        invalid=None,
-        printer=None,
-        param_checks=None,
-        result_checks=None,
-        implement=True,
+        name: str,
+        type: str,
+        params: list[tuple[str, str]],
+        comment: Optional[str] = None,
+        predicate: bool = False,
+        predefault: Optional[str] = None,
+        postdefault: Optional[str] = None,
+        invalid: Optional[Union[bool, str]] = None,
+        printer: Optional[str] = None,
+        param_checks: Optional[list[str]] = None,
+        result_checks: Optional[list[str]] = None,
+        implement: bool = True,
     ):
         super().__init__(
             comment=comment,
@@ -185,4 +186,4 @@ class Method(Function):
 
 
 # All the components created in gdbarch-components.py.
-components: list[_Component] = []
+components: list[Component] = []
diff --git a/gdb/gdbcopyright.py b/gdb/gdbcopyright.py
index 9a52bc1d847b..44c88e07cae7 100644
--- a/gdb/gdbcopyright.py
+++ b/gdb/gdbcopyright.py
@@ -18,7 +18,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
-def copyright(tool, description):
+def copyright(tool: str, description: str):
     # Search the tool source itself for the correct copyright years.
     with open(tool, "r") as f:
         for line in f:
-- 
2.39.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 7/9] gdb: make-target-delegates.py: make one string raw
  2023-02-23 22:18 [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Simon Marchi
                   ` (5 preceding siblings ...)
  2023-02-23 22:18 ` [PATCH 6/9] gdb: gdbarch*.py, copyright.py: add type annotations Simon Marchi
@ 2023-02-23 22:18 ` Simon Marchi
  2023-02-23 22:18 ` [PATCH 8/9] gdb: make-target-delegates.py: add Entry type Simon Marchi
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Simon Marchi @ 2023-02-23 22:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

Fixes the following flake8 warning:

  make-target-delegates.py:36:39: W605 invalid escape sequence '\s'

Change-Id: I25eeb296f55765e17e5217a2d1e49018f63a3acd
---
 gdb/make-target-delegates.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/make-target-delegates.py b/gdb/make-target-delegates.py
index 969f21baa017..2df84cd458c6 100755
--- a/gdb/make-target-delegates.py
+++ b/gdb/make-target-delegates.py
@@ -33,7 +33,7 @@ ENDER = re.compile(r"^\s*};$")
 # Match a C symbol.
 SYMBOL = "[a-zA-Z_][a-zA-Z0-9_]*"
 # Match the name part of a method in struct target_ops.
-NAME_PART = r"(?P<name>" + SYMBOL + ")\s"
+NAME_PART = r"(?P<name>" + SYMBOL + r")\s"
 # Match the arguments to a method.
 ARGS_PART = r"(?P<args>\(.*\))"
 # We strip the indentation so here we only need the caret.
-- 
2.39.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 8/9] gdb: make-target-delegates.py: add Entry type
  2023-02-23 22:18 [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Simon Marchi
                   ` (6 preceding siblings ...)
  2023-02-23 22:18 ` [PATCH 7/9] gdb: make-target-delegates.py: make one string raw Simon Marchi
@ 2023-02-23 22:18 ` Simon Marchi
  2023-02-23 22:18 ` [PATCH 9/9] gdb: make-target-delegates.py: add type annotations Simon Marchi
  2023-02-24 19:54 ` [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Tom Tromey
  9 siblings, 0 replies; 14+ messages in thread
From: Simon Marchi @ 2023-02-23 22:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

Add the Entry type and use it in the `entries` map, rather than using an
ad-hoc str -> str map that comes from the re.match.  This will make it
easier to make typing work in a subsequent patch, but it also helps
readers know what attributes exist for entries, which is not clear
currently.

Change-Id: I5b58dee1ed7ae85987b99bd417e641ede718624c
---
 gdb/make-target-delegates.py | 52 +++++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/gdb/make-target-delegates.py b/gdb/make-target-delegates.py
index 2df84cd458c6..aa0919e51b13 100755
--- a/gdb/make-target-delegates.py
+++ b/gdb/make-target-delegates.py
@@ -92,6 +92,16 @@ ARGTYPES = re.compile(
 TARGET_DEBUG_PRINTER = r"\s*TARGET_DEBUG_PRINTER\s*\((?P<arg>[^)]*)\)\s*"
 
 
+class Entry:
+    def __init__(
+        self, argtypes: list[str], return_type: str, style: str, default_arg: str
+    ):
+        self.argtypes = argtypes
+        self.return_type = return_type
+        self.style = style
+        self.default_arg = default_arg
+
+
 def scan_target_h():
     found_trigger = False
     all_the_text = ""
@@ -295,10 +305,9 @@ def print_class(f, class_name, delegators, entries):
     print("", file=f)
 
     for name in delegators:
-        return_type = entries[name]["return_type"]
-        argtypes = entries[name]["argtypes"]
         print("  ", file=f, end="")
-        write_declaration(f, name, return_type, argtypes)
+        entry = entries[name]
+        write_declaration(f, name, entry.return_type, entry.argtypes)
 
     print("};\n", file=f)
 
@@ -313,11 +322,14 @@ for current_line in scan_target_h():
     if not m:
         continue
     data = m.groupdict()
-    data["argtypes"] = parse_argtypes(data["args"])
-    data["return_type"] = data["return_type"].strip()
-    entries[data["name"]] = data
+    name = data["name"]
+    argtypes = parse_argtypes(data["args"])
+    return_type = data["return_type"].strip()
+    style = data["style"]
+    default_arg = data["default_arg"]
+    entries[name] = Entry(argtypes, return_type, style, default_arg)
 
-    delegators.append(data["name"])
+    delegators.append(name)
 
 with open("target-delegates.c", "w") as f:
     print(
@@ -330,11 +342,21 @@ with open("target-delegates.c", "w") as f:
     print_class(f, "debug_target", delegators, entries)
 
     for name in delegators:
-        tdefault = entries[name]["default_arg"]
-        return_type = entries[name]["return_type"]
-        style = entries[name]["style"]
-        argtypes = entries[name]["argtypes"]
-
-        write_delegator(f, name, return_type, argtypes)
-        write_tdefault(f, tdefault, style, name, return_type, argtypes)
-        write_debugmethod(f, tdefault, name, return_type, argtypes)
+        entry = entries[name]
+
+        write_delegator(f, name, entry.return_type, entry.argtypes)
+        write_tdefault(
+            f,
+            entry.default_arg,
+            entry.style,
+            name,
+            entry.return_type,
+            entry.argtypes,
+        )
+        write_debugmethod(
+            f,
+            entry.default_arg,
+            name,
+            entry.return_type,
+            entry.argtypes,
+        )
-- 
2.39.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 9/9] gdb: make-target-delegates.py: add type annotations
  2023-02-23 22:18 [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Simon Marchi
                   ` (7 preceding siblings ...)
  2023-02-23 22:18 ` [PATCH 8/9] gdb: make-target-delegates.py: add Entry type Simon Marchi
@ 2023-02-23 22:18 ` Simon Marchi
  2023-02-24 19:54 ` [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Tom Tromey
  9 siblings, 0 replies; 14+ messages in thread
From: Simon Marchi @ 2023-02-23 22:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

Fixes all warnings given by pyright.

Change-Id: I480521bfc62960c4eccd9d32c886392b05a1ddaa
---
 gdb/make-target-delegates.py | 50 +++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/gdb/make-target-delegates.py b/gdb/make-target-delegates.py
index aa0919e51b13..eb6c5de7cbc1 100755
--- a/gdb/make-target-delegates.py
+++ b/gdb/make-target-delegates.py
@@ -22,6 +22,7 @@
 
 import re
 import gdbcopyright
+import typing
 
 
 # The line we search for in target.h that marks where we should start
@@ -121,7 +122,7 @@ def scan_target_h():
                 line = re.split("//", line)[0]
                 all_the_text = all_the_text + " " + line
     if not found_trigger:
-        raise "Could not find trigger line"
+        raise RuntimeError("Could not find trigger line")
     # Now strip out the C comments.
     all_the_text = re.sub(r"/\*(.*?)\*/", "", all_the_text)
     # Replace sequences whitespace with a single space character.
@@ -142,10 +143,10 @@ def scan_target_h():
 
 
 # Parse arguments into a list.
-def parse_argtypes(typestr):
+def parse_argtypes(typestr: str):
     # Remove the outer parens.
     typestr = re.sub(r"^\((.*)\)$", r"\1", typestr)
-    result = []
+    result: list[str] = []
     for item in re.split(r",\s*", typestr):
         if item == "void" or item == "":
             continue
@@ -163,7 +164,9 @@ def parse_argtypes(typestr):
 
 # Write function header given name, return type, and argtypes.
 # Returns a list of actual argument names.
-def write_function_header(f, decl, name, return_type, argtypes):
+def write_function_header(
+    f: typing.TextIO, decl: bool, name: str, return_type: str, argtypes: list[str]
+):
     print(return_type, file=f, end="")
     if decl:
         if not return_type.endswith("*"):
@@ -171,8 +174,8 @@ def write_function_header(f, decl, name, return_type, argtypes):
     else:
         print("", file=f)
     print(name + " (", file=f, end="")
-    argdecls = []
-    actuals = []
+    argdecls: list[str] = []
+    actuals: list[str] = []
     for i in range(len(argtypes)):
         val = re.sub(TARGET_DEBUG_PRINTER, "", argtypes[i])
         if not val.endswith("*") and not val.endswith("&"):
@@ -190,12 +193,14 @@ def write_function_header(f, decl, name, return_type, argtypes):
 
 
 # Write out a declaration.
-def write_declaration(f, name, return_type, argtypes):
+def write_declaration(
+    f: typing.TextIO, name: str, return_type: str, argtypes: list[str]
+):
     write_function_header(f, True, name, return_type, argtypes)
 
 
 # Write out a delegation function.
-def write_delegator(f, name, return_type, argtypes):
+def write_delegator(f: typing.TextIO, name: str, return_type: str, argtypes: list[str]):
     names = write_function_header(
         f, False, "target_ops::" + name, return_type, argtypes
     )
@@ -209,7 +214,14 @@ def write_delegator(f, name, return_type, argtypes):
 
 
 # Write out a default function.
-def write_tdefault(f, content, style, name, return_type, argtypes):
+def write_tdefault(
+    f: typing.TextIO,
+    content: str,
+    style: str,
+    name: str,
+    return_type: str,
+    argtypes: list[str],
+):
     name = "dummy_target::" + name
     names = write_function_header(f, False, name, return_type, argtypes)
     if style == "FUNC":
@@ -227,11 +239,11 @@ def write_tdefault(f, content, style, name, return_type, argtypes):
         # Nothing.
         pass
     else:
-        raise "unrecognized style: " + style
+        raise RuntimeError("unrecognized style: " + style)
     print("}\n", file=f)
 
 
-def munge_type(typename):
+def munge_type(typename: str):
     m = re.search(TARGET_DEBUG_PRINTER, typename)
     if m:
         return m.group("arg")
@@ -250,7 +262,9 @@ def munge_type(typename):
 
 
 # Write out a debug method.
-def write_debugmethod(f, content, name, return_type, argtypes):
+def write_debugmethod(
+    f: typing.TextIO, content: str, name: str, return_type: str, argtypes: list[str]
+):
     debugname = "debug_target::" + name
     names = write_function_header(f, False, debugname, return_type, argtypes)
     if return_type != "void":
@@ -296,7 +310,12 @@ def write_debugmethod(f, content, name, return_type, argtypes):
     print("}\n", file=f)
 
 
-def print_class(f, class_name, delegators, entries):
+def print_class(
+    f: typing.TextIO,
+    class_name: str,
+    delegators: list[str],
+    entries: dict[str, Entry],
+):
     print("struct " + class_name + " : public target_ops", file=f)
     print("{", file=f)
     print("  const target_info &info () const override;", file=f)
@@ -312,8 +331,9 @@ def print_class(f, class_name, delegators, entries):
     print("};\n", file=f)
 
 
-delegators = []
-entries = {}
+delegators: list[str] = []
+entries: dict[str, Entry] = {}
+
 for current_line in scan_target_h():
     # See comments in scan_target_h.  Here we strip away the leading
     # and trailing whitespace.
-- 
2.39.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/9] gdb: gdbarch.py: spell out parameters of _Component.__init__
  2023-02-23 22:18 ` [PATCH 3/9] gdb: gdbarch.py: spell out parameters of _Component.__init__ Simon Marchi
@ 2023-02-24 19:51   ` Tom Tromey
  2023-02-24 20:31     ` Simon Marchi
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2023-02-24 19:51 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi, Simon Marchi

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
Simon> The way _Component uses kwargs is handy to save a few characters, but it
Simon> doesn't play well with static analysis.  When editing gdbarch.py, my
Simon> editor (which uses pylance under the hood) knows nothing about the
Simon> properties of components.  So it's full of squiggly lines, and typing
Simon> analysis (which I find really helpful) doesn't work.  I therefore think
Simon> it would be better to spell out the parameters.

Simon> @@ -87,7 +112,7 @@ class Value(_Component):
Simon>          name,
Simon>          type,
Simon>          comment=None,
Simon> -        predicate=None,
Simon> +        predicate=False,

This changes the type of predicate, which is fine, but there's still a
spot that does:

        # This little hack makes the generator a bit simpler.
        self.predicate = None

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py
  2023-02-23 22:18 [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Simon Marchi
                   ` (8 preceding siblings ...)
  2023-02-23 22:18 ` [PATCH 9/9] gdb: make-target-delegates.py: add type annotations Simon Marchi
@ 2023-02-24 19:54 ` Tom Tromey
  9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-02-24 19:54 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> While investigating something related to a gdbarch method, I was trying
Simon> to understand the purpose of the "invalid" Function parameter.  It is
Simon> sometimes passed a boolean and sometimes passed a string.  This is
Simon> indeed described in the comment at the top of gdbarch-components.py, but
Simon> I then thought that if we used type annotations, it would make it clear
Simon> that it's intended.  Also, I find type annotations (when paired with a
Simon> checker such as pyright or pylance) really handy to avoid all kinds of
Simon> mistake.  So I got a bit carried away, and this series is the result.

This all seems fine to me, though I found one it.

Reviewed-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/9] gdb: gdbarch.py: spell out parameters of _Component.__init__
  2023-02-24 19:51   ` Tom Tromey
@ 2023-02-24 20:31     ` Simon Marchi
  2023-02-24 22:07       ` Tom Tromey
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Marchi @ 2023-02-24 20:31 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: Simon Marchi

On 2/24/23 14:51, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
> Simon> The way _Component uses kwargs is handy to save a few characters, but it
> Simon> doesn't play well with static analysis.  When editing gdbarch.py, my
> Simon> editor (which uses pylance under the hood) knows nothing about the
> Simon> properties of components.  So it's full of squiggly lines, and typing
> Simon> analysis (which I find really helpful) doesn't work.  I therefore think
> Simon> it would be better to spell out the parameters.
> 
> Simon> @@ -87,7 +112,7 @@ class Value(_Component):
> Simon>          name,
> Simon>          type,
> Simon>          comment=None,
> Simon> -        predicate=None,
> Simon> +        predicate=False,
> 
> This changes the type of predicate, which is fine, but there's still a
> spot that does:
> 
>         # This little hack makes the generator a bit simpler.
>         self.predicate = None

Huh, it's unfortunate that the type checker does not understand at this
point that predicate (set in _Component.__init__ is a bool).  It does
understand it at other places.

I don't understand why this assignment to None is there though.  Wasn't
predicate None by default anyway?  Can we remove it, and in turn remove
Info.__init__?  If I do that, I see no diff in the generated files.

Simon

Simon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/9] gdb: gdbarch.py: spell out parameters of _Component.__init__
  2023-02-24 20:31     ` Simon Marchi
@ 2023-02-24 22:07       ` Tom Tromey
  0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2023-02-24 22:07 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi via Gdb-patches, Simon Marchi

Simon> Huh, it's unfortunate that the type checker does not understand at this
Simon> point that predicate (set in _Component.__init__ is a bool).  It does
Simon> understand it at other places.

Simon> I don't understand why this assignment to None is there though.  Wasn't
Simon> predicate None by default anyway?  Can we remove it, and in turn remove
Simon> Info.__init__?  If I do that, I see no diff in the generated files.

I don't really know, I was wondering if there's some intervening class
in the hierarchy.  But if removing it has no effect, then that seems best.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-02-24 22:07 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-23 22:18 [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Simon Marchi
2023-02-23 22:18 ` [PATCH 1/9] gdb: remove invalid / dead code from gdbarch.py Simon Marchi
2023-02-23 22:18 ` [PATCH 2/9] gdb: reformat Python files with black 23.1.0 Simon Marchi
2023-02-23 22:18 ` [PATCH 3/9] gdb: gdbarch.py: spell out parameters of _Component.__init__ Simon Marchi
2023-02-24 19:51   ` Tom Tromey
2023-02-24 20:31     ` Simon Marchi
2023-02-24 22:07       ` Tom Tromey
2023-02-23 22:18 ` [PATCH 4/9] gdb: pyproject.toml: set pyright typeCheckingMode = "strict" Simon Marchi
2023-02-23 22:18 ` [PATCH 5/9] gdb: split gdbarch component types to gdbarch_types.py Simon Marchi
2023-02-23 22:18 ` [PATCH 6/9] gdb: gdbarch*.py, copyright.py: add type annotations Simon Marchi
2023-02-23 22:18 ` [PATCH 7/9] gdb: make-target-delegates.py: make one string raw Simon Marchi
2023-02-23 22:18 ` [PATCH 8/9] gdb: make-target-delegates.py: add Entry type Simon Marchi
2023-02-23 22:18 ` [PATCH 9/9] gdb: make-target-delegates.py: add type annotations Simon Marchi
2023-02-24 19:54 ` [PATCH 0/9] Add typing annotations to gdbarch*.py and make-target-delegates.py Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).