[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 36/38] qapi/visit.py: assert tag_member contains a QAPISch
From: |
Cleber Rosa |
Subject: |
Re: [PATCH v2 36/38] qapi/visit.py: assert tag_member contains a QAPISchemaEnumType |
Date: |
Thu, 24 Sep 2020 19:52:02 -0400 |
On Thu, Sep 24, 2020 at 03:36:23PM -0400, John Snow wrote:
> On 9/24/20 3:10 PM, Cleber Rosa wrote:
> > On Tue, Sep 22, 2020 at 05:00:59PM -0400, John Snow wrote:
> > > Signed-off-by: John Snow <jsnow@redhat.com>
> > > ---
> > > scripts/qapi/visit.py | 15 ++++++++++-----
> > > 1 file changed, 10 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> > > index 4edaee33e3..180c140180 100644
> > > --- a/scripts/qapi/visit.py
> > > +++ b/scripts/qapi/visit.py
> > > @@ -22,7 +22,10 @@
> > > indent,
> > > )
> > > from .gen import QAPISchemaModularCVisitor, ifcontext
> > > -from .schema import QAPISchemaObjectType
> > > +from .schema import (
> > > + QAPISchemaEnumType,
> > > + QAPISchemaObjectType,
> > > +)
> > > def gen_visit_decl(name, scalar=False):
> > > @@ -84,15 +87,17 @@ def gen_visit_object_members(name, base, members,
> > > variants):
> > > ret += gen_endif(memb.ifcond)
> > > if variants:
> > > + tag_member = variants.tag_member
> > > + assert isinstance(tag_member.type, QAPISchemaEnumType)
> > > +
> >
> > I'd be interested in knowing why this wasn't left to be handled by the
> > type checking only. Anyway,
> >
>
> QAPISchemaVariants is a container type that is used to house a number of
> QAPISchemaVariant types; but it (can) also take a tag_member to identify one
> of the fields in the variants that can be used to differentiate them.
>
> Now, we assert that tag_member must be a QAPISchemaObjectTypeMember.
> QAPISchemaVariant is also a QAPISchemaObjectTypeMember.
>
> a QAPISchemaObjectTypeMember is a QAPISchemaMember. a QAPISchemaMember
> describes one 'member' of either an enum, a features list, or an object
> member.
>
> Now, the QAPISchemaObjectTypeMember (and not the QAPISchemaMember!) serves
> as a container for a QAPISchemaType -- this is a wrapper type, effectively.
> That contained type can be *anything*, because object members can be
> *anything*.
>
> Oops, but if we zoom back out, we are only able to constrain tag_member to
> being a QAPISchemaObjectTypeMember, we have no expressive power over its
> contained type.
>
> That's why you need the assertion here; because of a loss of specificity
> when we declare tag_member.
>
>
> "Wow, John, it sounds like you should use a Generic type to be able to
> describe the inner type of a QAPISchemaObjectTypeMember?"
>
> Uh, yup, you're right! I should. But it's complicated, because
> QAPISchemaMember does not have a contained type. Further, the contained type
> of a Member may or may not be known at construction time right now.
>
> It's fixable, and probably involves adding something like a "string
> constant" dummy type to allow QAPISchemaMember to have a contained type.
>
> "Hey, all of that sounds very messy. What if we just added in a few
> assertions for right now while we get the preliminary types in, and then we
> can make adjustments based on what makes the code prettier?"
>
> Sounds like a plan, hypothetical quote person.
>
> > Reviewed-by: Cleber Rosa <crosa@redhat.com>
> >
I did not attempt to learn the type names by heart (mental sanity
first) but I get the big picture. Thanks!
- Cleber.
signature.asc
Description: PGP signature
- Re: [PATCH v2 30/38] qapi/introspect.py: Add a typed 'extra' structure, (continued)
[PATCH v2 34/38] qapi/types.py: add type hint annotations, John Snow, 2020/09/22
[PATCH v2 38/38] qapi/visit.py: add type hint annotations, John Snow, 2020/09/22
[PATCH v2 31/38] qapi/introspect.py: add _gen_features helper, John Snow, 2020/09/22