1. Patchew
  2. QEMU
  3. [PATCH 00/19] qapi: statically type schema.py
  4. View patch

[PATCH 10/19] qapi/schema: make QAPISchemaArrayType.element_type non-Optional

John Snow posted 19 patches 1 year ago
There is a newer version of this series
[PATCH 10/19] qapi/schema: make QAPISchemaArrayType.element_type non-Optional
Posted by John Snow 1 year ago 
This field should always be present and defined. Change this to be a
runtime @property that can emit an error if it's called prior to
check().

This helps simplify typing by avoiding the need to interrogate the value
for None at multiple callsites.

RFC: Yes, this is a slightly different technique than the one I used for
QAPISchemaObjectTypeMember.type; I think I prefer this one as being a
little less hokey, but it is more SLOC. Dealer's choice for which style
wins out -- now you have an example of both.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/schema.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index c9a194103e1..462acb2bb61 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -366,7 +366,16 @@ def __init__(self, name, info, element_type):
         super().__init__(name, info, None)
         assert isinstance(element_type, str)
         self._element_type_name = element_type
-        self.element_type = None
+        self._element_type: Optional[QAPISchemaType] = None
+
+    @property
+    def element_type(self) -> QAPISchemaType:
+        if self._element_type is None:
+            raise RuntimeError(
+                "QAPISchemaArray has no element_type until "
+                "after check() has been run."
+            )
+        return self._element_type
 
     def need_has_if_optional(self):
         # When FOO is an array, we still need has_FOO to distinguish
@@ -375,7 +384,7 @@ def need_has_if_optional(self):
 
     def check(self, schema):
         super().check(schema)
-        self.element_type = schema.resolve_type(
+        self._element_type = schema.resolve_type(
             self._element_type_name, self.info,
             self.info and self.info.defn_meta)
         assert not isinstance(self.element_type, QAPISchemaArrayType)
-- 
2.41.0
Re: [PATCH 10/19] qapi/schema: make QAPISchemaArrayType.element_type non-Optional
Posted by Markus Armbruster 1 year ago 
John Snow <jsnow@redhat.com> writes:

> This field should always be present and defined. Change this to be a
> runtime @property that can emit an error if it's called prior to
> check().
>
> This helps simplify typing by avoiding the need to interrogate the value
> for None at multiple callsites.
>
> RFC: Yes, this is a slightly different technique than the one I used for
> QAPISchemaObjectTypeMember.type;

In PATCH 04.

>                                  I think I prefer this one as being a
> little less hokey, but it is more SLOC. Dealer's choice for which style
> wins out -- now you have an example of both.

Thanks for letting us see both.

I believe all the extra lines accomplish is a different exception
RuntimeError with a custom message vs. plain AttributeError.

I don't think the more elaborate exception is worth the extra code.

> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/schema.py | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index c9a194103e1..462acb2bb61 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -366,7 +366,16 @@ def __init__(self, name, info, element_type):
>          super().__init__(name, info, None)
>          assert isinstance(element_type, str)
>          self._element_type_name = element_type
> -        self.element_type = None
> +        self._element_type: Optional[QAPISchemaType] = None
> +
> +    @property
> +    def element_type(self) -> QAPISchemaType:
> +        if self._element_type is None:
> +            raise RuntimeError(
> +                "QAPISchemaArray has no element_type until "
> +                "after check() has been run."
> +            )
> +        return self._element_type
>  
>      def need_has_if_optional(self):
>          # When FOO is an array, we still need has_FOO to distinguish
> @@ -375,7 +384,7 @@ def need_has_if_optional(self):
>  
>      def check(self, schema):
>          super().check(schema)
> -        self.element_type = schema.resolve_type(
> +        self._element_type = schema.resolve_type(
>              self._element_type_name, self.info,
>              self.info and self.info.defn_meta)
>          assert not isinstance(self.element_type, QAPISchemaArrayType)
Re: [PATCH 10/19] qapi/schema: make QAPISchemaArrayType.element_type non-Optional
Posted by John Snow 1 year ago 
On Tue, Nov 21, 2023, 9:28 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > This field should always be present and defined. Change this to be a
> > runtime @property that can emit an error if it's called prior to
> > check().
> >
> > This helps simplify typing by avoiding the need to interrogate the value
> > for None at multiple callsites.
> >
> > RFC: Yes, this is a slightly different technique than the one I used for
> > QAPISchemaObjectTypeMember.type;
>
> In PATCH 04.
>
> >                                  I think I prefer this one as being a
> > little less hokey, but it is more SLOC. Dealer's choice for which style
> > wins out -- now you have an example of both.
>
> Thanks for letting us see both.
>

My pleasure ;)


> I believe all the extra lines accomplish is a different exception
> RuntimeError with a custom message vs. plain AttributeError.
>
> I don't think the more elaborate exception is worth the extra code.
>

Hmm, shame. You're the boss :)


> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  scripts/qapi/schema.py | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> > index c9a194103e1..462acb2bb61 100644
> > --- a/scripts/qapi/schema.py
> > +++ b/scripts/qapi/schema.py
> > @@ -366,7 +366,16 @@ def __init__(self, name, info, element_type):
> >          super().__init__(name, info, None)
> >          assert isinstance(element_type, str)
> >          self._element_type_name = element_type
> > -        self.element_type = None
> > +        self._element_type: Optional[QAPISchemaType] = None
> > +
> > +    @property
> > +    def element_type(self) -> QAPISchemaType:
> > +        if self._element_type is None:
> > +            raise RuntimeError(
> > +                "QAPISchemaArray has no element_type until "
> > +                "after check() has been run."
> > +            )
> > +        return self._element_type
> >
> >      def need_has_if_optional(self):
> >          # When FOO is an array, we still need has_FOO to distinguish
> > @@ -375,7 +384,7 @@ def need_has_if_optional(self):
> >
> >      def check(self, schema):
> >          super().check(schema)
> > -        self.element_type = schema.resolve_type(
> > +        self._element_type = schema.resolve_type(
> >              self._element_type_name, self.info,
> >              self.info and self.info.defn_meta)
> >          assert not isinstance(self.element_type, QAPISchemaArrayType)
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.