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 <js...@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 <cr...@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
