diff mbox series

[3/9] gdb: gdbarch.py: spell out parameters of _Component.__init__

Message ID 20230223221830.499934-4-simon.marchi@efficios.com
State Committed
Commit c4e1b10cc2e84eaf574842907e4c35ad51eb5792
Headers
Series Add typing annotations to gdbarch*.py and make-target-delegates.py |

Commit Message

Simon Marchi Feb. 23, 2023, 10:18 p.m. UTC 
  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(-)

Comments

Tom Tromey Feb. 24, 2023, 7:51 p.m. UTC | #1 
>>>>> "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
Simon Marchi Feb. 24, 2023, 8:31 p.m. UTC | #2 
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
Tom Tromey Feb. 24, 2023, 10:07 p.m. UTC | #3 
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
diff mbox series

Patch

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,