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>