[Qemu-devel] [PATCH v7 25/31] qapi: Canonicalize missing object to :empty

[Eric Blake <eblake@redhat.com>]

Now that we elide unnecessary visits of empty types, we can
start using the special ':empty' type in more places.  By using
the empty type as the base class of every explicit struct or
union, and as the default data for any command or event, we can
simplify later logic in qapi-{visit,commands,event} by merely
checking whether the type is empty, without also having to worry
whether a type was even supplied.

Note that gen_object() in qapi-types still has to check for a
base, because it is also called for alternates (which have no
base).

No change to generated code.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v7: rebase to earlier changes
v6: new patch
---
 scripts/qapi-commands.py                |  7 ++---
 scripts/qapi-event.py                   |  5 ++--
 scripts/qapi-types.py                   |  4 +--
 scripts/qapi-visit.py                   | 12 +++++----
 scripts/qapi.py                         | 22 ++++++++--------
 tests/qapi-schema/event-case.out        |  2 +-
 tests/qapi-schema/flat-union-empty.out  |  1 +
 tests/qapi-schema/ident-with-escape.out |  1 +
 tests/qapi-schema/indented-expr.out     |  4 +--
 tests/qapi-schema/qapi-schema-test.out  | 45 ++++++++++++++++++++++++++++++---
 tests/qapi-schema/union-clash-data.out  |  2 ++
 tests/qapi-schema/union-empty.out       |  1 +
 12 files changed, 77 insertions(+), 29 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 38cbffc..0f3cc57 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -65,7 +65,8 @@ def gen_marshal_vars(arg_type, ret_type):
 ''',
                      c_type=ret_type.c_type())

-    if arg_type and not arg_type.is_empty():
+    assert arg_type
+    if not arg_type.is_empty():
         ret += mcgen('''
     QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
     QapiDeallocVisitor *qdv;
@@ -97,7 +98,7 @@ def gen_marshal_vars(arg_type, ret_type):
 def gen_marshal_input_visit(arg_type, dealloc=False):
     ret = ''

-    if not arg_type or arg_type.is_empty():
+    if arg_type.is_empty():
         return ret

     if dealloc:
@@ -177,7 +178,7 @@ def gen_marshal(name, arg_type, ret_type):

     # 'goto out' produced by gen_marshal_input_visit->gen_visit_fields()
     # for each arg_type member, and by gen_call() for ret_type
-    if (arg_type and not arg_type.is_empty()) or ret_type:
+    if not arg_type.is_empty() or ret_type:
         ret += mcgen('''

 out:
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index f2feaaf..6a29b6c 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -39,7 +39,8 @@ def gen_event_send(name, arg_type):
 ''',
                 proto=gen_event_send_proto(name, arg_type))

-    if arg_type and not arg_type.is_empty():
+    assert arg_type
+    if not arg_type.is_empty():
         ret += mcgen('''
     QObject *obj;
     QmpOutputVisitor *qov;
@@ -88,7 +89,7 @@ out_obj:
 ''',
                  c_enum=c_enum_const(event_enum_name, name))

-    if arg_type and not arg_type.is_empty():
+    if not arg_type.is_empty():
         ret += mcgen('''
 out:
     qmp_output_visitor_cleanup(qov);
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 8f57200..91c5ae0 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -58,7 +58,7 @@ struct %(c_name)s {
 ''',
                 c_name=c_name(name))

-    if base:
+    if base and not base.is_empty():
         ret += mcgen('''
     /* Members inherited from %(c_name)s: */
 ''',
@@ -222,7 +222,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
     def visit_object_type(self, name, info, base, members, variants):
         self._fwdecl += gen_fwd_object_or_array(name)
         self.decl += gen_object(name, base, members, variants)
-        if base:
+        if not base.is_implicit():
             self.decl += gen_upcast(name, base)
         self._gen_type_cleanup(name)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 4ec0d22..2f71dac 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -74,10 +74,11 @@ static void visit_type_implicit_%(c_type)s(Visitor *v, %(c_type)s **obj, Error *
 def gen_visit_struct_fields(name, base, members):
     ret = ''

-    if (not base or base.is_empty()) and not members:
+    assert base
+    if base.is_empty() and not members:
         return ret

-    if base and not base.is_empty():
+    if not base.is_empty():
         ret += gen_visit_fields_decl(base)

     struct_fields_seen.add(name)
@@ -90,7 +91,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e
 ''',
                  c_name=c_name(name))

-    if base and not base.is_empty():
+    if not base.is_empty():
         ret += mcgen('''
     visit_type_%(c_type)s_fields(v, (%(c_type)s **)obj, &err);
 ''',
@@ -246,7 +247,8 @@ out:
 def gen_visit_union(name, base, variants):
     ret = ''

-    if base:
+    assert base
+    if not base.is_empty():
         ret += gen_visit_fields_decl(base)

     for var in variants.variants:
@@ -270,7 +272,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
 ''',
                  c_name=c_name(name))

-    if base:
+    if not base.is_empty():
         ret += mcgen('''
     visit_type_%(c_name)s_fields(v, (%(c_name)s **)obj, &err);
 ''',
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8b388ee..814cb05 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -922,11 +922,11 @@ class QAPISchemaArrayType(QAPISchemaType):

 class QAPISchemaObjectType(QAPISchemaType):
     def __init__(self, name, info, base, local_members, variants):
-        # struct has local_members, optional base, and no variants
+        # struct has local_members, base, and no variants
         # flat union has base, variants, and no local_members
-        # simple union has local_members, variants, and no base
+        # simple union has local_members, variants, and base of :empty
         QAPISchemaType.__init__(self, name, info)
-        assert base is None or isinstance(base, str)
+        assert isinstance(base, str) or name == ':empty'
         for m in local_members:
             assert isinstance(m, QAPISchemaObjectTypeMember)
             m.set_owner(name)
@@ -1264,11 +1264,11 @@ class QAPISchema(object):

     def _make_implicit_object_type(self, name, info, role, members):
         if not members:
-            return None
+            return ':empty'
         # See also QAPISchemaObjectTypeMember._pretty_owner()
         name = ':obj-%s-%s' % (name, role)
         if not self.lookup_entity(name, QAPISchemaObjectType):
-            self._def_entity(QAPISchemaObjectType(name, info, None,
+            self._def_entity(QAPISchemaObjectType(name, info, ':empty',
                                                   members, None))
         return name

@@ -1295,7 +1295,7 @@ class QAPISchema(object):

     def _def_struct_type(self, expr, info):
         name = expr['struct']
-        base = expr.get('base')
+        base = expr.get('base', ':empty')
         data = expr['data']
         self._def_entity(QAPISchemaObjectType(name, info, base,
                                               self._make_members(data, info),
@@ -1315,7 +1315,7 @@ class QAPISchema(object):
     def _def_union_type(self, expr, info):
         name = expr['union']
         data = expr['data']
-        base = expr.get('base')
+        base = expr.get('base', ':empty')
         tag_name = expr.get('discriminator')
         tag_member = None
         if tag_name:
@@ -1349,11 +1349,11 @@ class QAPISchema(object):

     def _def_command(self, expr, info):
         name = expr['command']
-        data = expr.get('data')
+        data = expr.get('data', {})
         rets = expr.get('returns')
         gen = expr.get('gen', True)
         success_response = expr.get('success-response', True)
-        if isinstance(data, OrderedDict):
+        if isinstance(data, dict):
             data = self._make_implicit_object_type(
                 name, info, 'arg', self._make_members(data, info))
         if isinstance(rets, list):
@@ -1364,8 +1364,8 @@ class QAPISchema(object):

     def _def_event(self, expr, info):
         name = expr['event']
-        data = expr.get('data')
-        if isinstance(data, OrderedDict):
+        data = expr.get('data', {})
+        if isinstance(data, dict):
             data = self._make_implicit_object_type(
                 name, info, 'arg', self._make_members(data, info))
         self._def_entity(QAPISchemaEvent(name, info, data))
diff --git a/tests/qapi-schema/event-case.out b/tests/qapi-schema/event-case.out
index 6350d64..18866d9 100644
--- a/tests/qapi-schema/event-case.out
+++ b/tests/qapi-schema/event-case.out
@@ -1,4 +1,4 @@
 object :empty
 enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
     prefix QTYPE
-event oops None
+event oops :empty
diff --git a/tests/qapi-schema/flat-union-empty.out b/tests/qapi-schema/flat-union-empty.out
index eade2d5..1a58e07 100644
--- a/tests/qapi-schema/flat-union-empty.out
+++ b/tests/qapi-schema/flat-union-empty.out
@@ -1,5 +1,6 @@
 object :empty
 object Base
+    base :empty
     member type: Empty optional=False
 enum Empty []
 enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
diff --git a/tests/qapi-schema/ident-with-escape.out b/tests/qapi-schema/ident-with-escape.out
index 453e0b2..142cd19 100644
--- a/tests/qapi-schema/ident-with-escape.out
+++ b/tests/qapi-schema/ident-with-escape.out
@@ -1,5 +1,6 @@
 object :empty
 object :obj-fooA-arg
+    base :empty
     member bar1: str optional=False
 enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
     prefix QTYPE
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
index ce37ff5..5c25fcd 100644
--- a/tests/qapi-schema/indented-expr.out
+++ b/tests/qapi-schema/indented-expr.out
@@ -1,7 +1,7 @@
 object :empty
 enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
     prefix QTYPE
-command eins None -> None
+command eins :empty -> None
    gen=True success_response=True
-command zwei None -> None
+command zwei :empty -> None
    gen=True success_response=True
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index d8f9108..a2a3c8b 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -1,56 +1,78 @@
 object :empty
 object :obj-EVENT_C-arg
+    base :empty
     member a: int optional=True
     member b: UserDefOne optional=True
     member c: str optional=False
 object :obj-EVENT_D-arg
+    base :empty
     member a: EventStructOne optional=False
     member b: str optional=False
     member c: str optional=True
     member enum3: EnumOne optional=True
 object :obj-__org.qemu_x-command-arg
+    base :empty
     member a: __org.qemu_x-EnumList optional=False
     member b: __org.qemu_x-StructList optional=False
     member c: __org.qemu_x-Union2 optional=False
     member d: __org.qemu_x-Alt optional=False
 object :obj-anyList-wrapper
+    base :empty
     member data: anyList optional=False
 object :obj-boolList-wrapper
+    base :empty
     member data: boolList optional=False
 object :obj-guest-get-time-arg
+    base :empty
     member a: int optional=False
     member b: int optional=True
 object :obj-guest-sync-arg
+    base :empty
     member arg: any optional=False
 object :obj-int16List-wrapper
+    base :empty
     member data: int16List optional=False
 object :obj-int32List-wrapper
+    base :empty
     member data: int32List optional=False
 object :obj-int64List-wrapper
+    base :empty
     member data: int64List optional=False
 object :obj-int8List-wrapper
+    base :empty
     member data: int8List optional=False
 object :obj-intList-wrapper
+    base :empty
     member data: intList optional=False
 object :obj-numberList-wrapper
+    base :empty
     member data: numberList optional=False
 object :obj-sizeList-wrapper
+    base :empty
     member data: sizeList optional=False
 object :obj-str-wrapper
+    base :empty
     member data: str optional=False
 object :obj-strList-wrapper
+    base :empty
     member data: strList optional=False
 object :obj-uint16List-wrapper
+    base :empty
     member data: uint16List optional=False
 object :obj-uint32List-wrapper
+    base :empty
     member data: uint32List optional=False
 object :obj-uint64List-wrapper
+    base :empty
     member data: uint64List optional=False
 object :obj-uint8List-wrapper
+    base :empty
     member data: uint8List optional=False
 object :obj-user_def_cmd1-arg
+    base :empty
     member ud1a: UserDefOne optional=False
 object :obj-user_def_cmd2-arg
+    base :empty
     member ud1a: UserDefOne optional=False
     member ud1b: UserDefOne optional=True
 alternate AltIntNum
@@ -71,24 +93,28 @@ alternate AltStrInt
 alternate AltStrNum
     case s: str
     case n: number
-event EVENT_A None
-event EVENT_B None
+event EVENT_A :empty
+event EVENT_B :empty
 event EVENT_C :obj-EVENT_C-arg
 event EVENT_D :obj-EVENT_D-arg
 object Empty1
+    base :empty
 object Empty2
     base Empty1
 enum EnumOne ['value1', 'value2', 'value3']
 object EventStructOne
+    base :empty
     member struct1: UserDefOne optional=False
     member string: str optional=False
     member enum2: EnumOne optional=True
 object ForceArrays
+    base :empty
     member unused1: UserDefOneList optional=False
     member unused2: UserDefTwoList optional=False
     member unused3: TestStructList optional=False
 enum MyEnum []
 object NestedEnumsOne
+    base :empty
     member enum1: EnumOne optional=False
     member enum2: EnumOne optional=True
     member enum3: EnumOne optional=False
@@ -98,10 +124,12 @@ enum QEnumTwo ['value1', 'value2']
 enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
     prefix QTYPE
 object TestStruct
+    base :empty
     member integer: int optional=False
     member boolean: bool optional=False
     member string: str optional=False
 object UserDefA
+    base :empty
     member boolean: bool optional=False
     member a_b: int optional=True
 alternate UserDefAlternate
@@ -109,9 +137,11 @@ alternate UserDefAlternate
     case s: str
     case i: int
 object UserDefB
+    base :empty
     member intb: int optional=False
     member a-b: bool optional=True
 object UserDefC
+    base :empty
     member string1: str optional=False
     member string2: str optional=False
 object UserDefFlatUnion
@@ -127,6 +157,7 @@ object UserDefFlatUnion2
     case value2: UserDefB
     case value3: UserDefA
 object UserDefNativeListUnion
+    base :empty
     member type: UserDefNativeListUnionKind optional=False
     case integer: :obj-intList-wrapper
     case s8: :obj-int8List-wrapper
@@ -148,19 +179,23 @@ object UserDefOne
     member string: str optional=False
     member enum1: EnumOne optional=True
 object UserDefOptions
+    base :empty
     member i64: intList optional=True
     member u64: uint64List optional=True
     member u16: uint16List optional=True
     member i64x: int optional=True
     member u64x: uint64 optional=True
 object UserDefTwo
+    base :empty
     member string0: str optional=False
     member dict1: UserDefTwoDict optional=False
 object UserDefTwoDict
+    base :empty
     member string1: str optional=False
     member dict2: UserDefTwoDictDict optional=False
     member dict3: UserDefTwoDictDict optional=True
 object UserDefTwoDictDict
+    base :empty
     member userdef: UserDefOne optional=False
     member string: str optional=False
 object UserDefUnionBase
@@ -168,12 +203,14 @@ object UserDefUnionBase
     member string: str optional=False
     member enum1: EnumOne optional=False
 object UserDefZero
+    base :empty
     member integer: int optional=False
 event __ORG.QEMU_X-EVENT __org.qemu_x-Struct
 alternate __org.qemu_x-Alt
     case __org.qemu_x-branch: str
     case b: __org.qemu_x-Base
 object __org.qemu_x-Base
+    base :empty
     member __org.qemu_x-member1: __org.qemu_x-Enum optional=False
 enum __org.qemu_x-Enum ['__org.qemu_x-value']
 object __org.qemu_x-Struct
@@ -181,8 +218,10 @@ object __org.qemu_x-Struct
     member __org.qemu_x-member2: str optional=False
     member wchar-t: int optional=True
 object __org.qemu_x-Struct2
+    base :empty
     member array: __org.qemu_x-Union1List optional=False
 object __org.qemu_x-Union1
+    base :empty
     member type: __org.qemu_x-Union1Kind optional=False
     case __org.qemu_x-branch: :obj-str-wrapper
 enum __org.qemu_x-Union1Kind ['__org.qemu_x-branch']
@@ -196,7 +235,7 @@ command guest-get-time :obj-guest-get-time-arg -> int
    gen=True success_response=True
 command guest-sync :obj-guest-sync-arg -> any
    gen=True success_response=True
-command user_def_cmd None -> None
+command user_def_cmd :empty -> None
    gen=True success_response=True
 command user_def_cmd0 Empty2 -> Empty2
    gen=True success_response=True
diff --git a/tests/qapi-schema/union-clash-data.out b/tests/qapi-schema/union-clash-data.out
index f5752f4..19031a2 100644
--- a/tests/qapi-schema/union-clash-data.out
+++ b/tests/qapi-schema/union-clash-data.out
@@ -1,9 +1,11 @@
 object :empty
 object :obj-int-wrapper
+    base :empty
     member data: int optional=False
 enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
     prefix QTYPE
 object TestUnion
+    base :empty
     member type: TestUnionKind optional=False
     case data: :obj-int-wrapper
 enum TestUnionKind ['data']
diff --git a/tests/qapi-schema/union-empty.out b/tests/qapi-schema/union-empty.out
index bdf17e5..57fcbd1 100644
--- a/tests/qapi-schema/union-empty.out
+++ b/tests/qapi-schema/union-empty.out
@@ -2,5 +2,6 @@ object :empty
 enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
     prefix QTYPE
 object Union
+    base :empty
     member type: UnionKind optional=False
 enum UnionKind []
-- 
2.4.3