[Qemu-devel] [PATCH v6 07/15] qapi: Change alternate layout to use 'type'

[Eric Blake]

Prepare to simplify alternate layout by creating a dedicated
subclass for the generated tag type.  QMP does not transmit
the tag name, so we can name it whatever we want in C.  But
since the tag is closely tied to a qtype_code, this commit
renames the tag from 'kind' to 'type', so the next commit can
then further modify things to use a simpler 'qtype_code type;'
for the tag.  As part of this, a new member.c_type() method
will make it possible to express the tag type even if there
is no qapi entity type associated with the tag.

Signed-off-by: Eric Blake <address@hidden>

---
v6: new patch
---
 scripts/qapi-types.py                       |  2 +-
 scripts/qapi-visit.py                       |  4 ++--
 scripts/qapi.py                             | 20 +++++++++++++++++---
 tests/qapi-schema/alternate-clash-type.err  |  2 +-
 tests/qapi-schema/alternate-clash-type.json |  2 +-
 tests/test-qmp-input-visitor.c              | 22 +++++++++++-----------
 tests/test-qmp-output-visitor.c             |  2 +-
 7 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 34ea318..138920c 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -138,7 +138,7 @@ struct %(c_name)s {
         ret += mcgen('''
     %(c_type)s %(c_name)s;
 ''',
-                     c_type=variants.tag_member.type.c_name(),
+                     c_type=variants.tag_member.c_type(),
                      c_name=variants.tag_member.c_name())

     # FIXME: What purpose does data serve, besides preventing a union that
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 3f74302..d82e2a3 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -179,11 +179,11 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, 
const char *name, Error
     if (err) {
         goto out;
     }
-    visit_get_next_type(v, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, 
&err);
+    visit_get_next_type(v, (int*) &(*obj)->type, %(c_name)s_qtypes, name, 
&err);
     if (err) {
         goto out_obj;
     }
-    switch ((*obj)->kind) {
+    switch ((*obj)->type) {
 ''',
                 c_name=c_name(name))

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 7cf1db0..9a734a8 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1015,6 +1015,9 @@ class QAPISchemaObjectTypeMember(object):
     def c_name(self):
         return c_name(self.name)

+    def c_type(self):
+        return self.type.c_name()
+
     def describe(self):
         source = self.owner
         if source.startswith(':obj-'):
@@ -1119,6 +1122,14 @@ class QAPISchemaAlternateType(QAPISchemaType):
         visitor.visit_alternate_type(self.name, self.info, self.variants)


+class QAPISchemaAlternateTypeTag(QAPISchemaObjectTypeMember):
+    def __init__(self, enum_type):
+        QAPISchemaObjectTypeMember.__init__(self, 'type', enum_type, False)
+
+    def _describe(self):
+        return 'implicit tag'
+
+
 class QAPISchemaCommand(QAPISchemaEntity):
     def __init__(self, name, info, arg_type, ret_type, gen, success_response):
         QAPISchemaEntity.__init__(self, name, info)
@@ -1312,11 +1323,14 @@ class QAPISchema(object):
         data = expr['data']
         variants = [self._make_variant(key, value)
                     for (key, value) in data.iteritems()]
-        tag_enum = self._make_implicit_tag(name, info, variants)
+        # TODO simplify this to avoid the need for an implicit enum
+        tag_member = QAPISchemaAlternateTypeTag(
+            self._make_implicit_enum_type(name, info,
+                                          [v.name for v in variants]))
         self._def_entity(
             QAPISchemaAlternateType(name, info,
                                     QAPISchemaObjectTypeVariants(None,
-                                                                 tag_enum,
+                                                                 tag_member,
                                                                  variants)))

     def _def_command(self, expr, info):
@@ -1640,7 +1654,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, 
skiperr=False):
         ret += mcgen('''
     visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", 
%(errp)s);
 ''',
-                     c_type=memb.type.c_name(), prefix=prefix, cast=cast,
+                     c_type=memb.c_type(), prefix=prefix, cast=cast,
                      c_name=memb.c_name(), name=memb.name,
                      errp=errparg)
         ret += gen_err_check(skiperr=skiperr)
diff --git a/tests/qapi-schema/alternate-clash-type.err 
b/tests/qapi-schema/alternate-clash-type.err
index cdd2090..9d922fb 100644
--- a/tests/qapi-schema/alternate-clash-type.err
+++ b/tests/qapi-schema/alternate-clash-type.err
@@ -1 +1 @@
-tests/qapi-schema/alternate-clash-type.json:9: 'kind' (branch of Alt1) 
collides with 'kind' (implicit tag of Alt1)
+tests/qapi-schema/alternate-clash-type.json:9: 'type' (branch of Alt1) 
collides with 'type' (implicit tag of Alt1)
diff --git a/tests/qapi-schema/alternate-clash-type.json 
b/tests/qapi-schema/alternate-clash-type.json
index 629584b..0c7b497 100644
--- a/tests/qapi-schema/alternate-clash-type.json
+++ b/tests/qapi-schema/alternate-clash-type.json
@@ -1,6 +1,6 @@
 # Alternate branch 'type'
 # Reject this, because we would have a clash in generated C, between the
-# alternate's implicit tag member 'kind' and the branch name 'kind'
+# alternate's implicit tag member 'type' and the branch name 'type'
 # within the alternate.
 # TODO: Even if alternates are simplified in the future to use a simpler
 # 'qtype_code type' tag, rather than a full QAPISchemaObjectTypeMember,
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 532a31a..9540fa0 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -338,14 +338,14 @@ static void 
test_visitor_in_alternate(TestInputVisitorData *data,

     v = visitor_input_test_init(data, "42");
     visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
-    g_assert_cmpint(tmp->kind, ==, USER_DEF_ALTERNATE_KIND_I);
+    g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_I);
     g_assert_cmpint(tmp->i, ==, 42);
     qapi_free_UserDefAlternate(tmp);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "'string'");
     visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
-    g_assert_cmpint(tmp->kind, ==, USER_DEF_ALTERNATE_KIND_S);
+    g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_S);
     g_assert_cmpstr(tmp->s, ==, "string");
     qapi_free_UserDefAlternate(tmp);
     visitor_input_teardown(data, NULL);
@@ -385,7 +385,7 @@ static void 
test_visitor_in_alternate_number(TestInputVisitorData *data,
      * parse the same as ans */
     v = visitor_input_test_init(data, "42");
     visit_type_AltStrNum(v, &asn, NULL, &err);
-    /* FIXME g_assert_cmpint(asn->kind, == ALT_STR_NUM_KIND_N); */
+    /* FIXME g_assert_cmpint(asn->type, == ALT_STR_NUM_KIND_N); */
     /* FIXME g_assert_cmpfloat(asn->n, ==, 42); */
     g_assert(err);
     error_free(err);
@@ -395,28 +395,28 @@ static void 
test_visitor_in_alternate_number(TestInputVisitorData *data,

     v = visitor_input_test_init(data, "42");
     visit_type_AltNumStr(v, &ans, NULL, &error_abort);
-    g_assert_cmpint(ans->kind, ==, ALT_NUM_STR_KIND_N);
+    g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
     g_assert_cmpfloat(ans->n, ==, 42);
     qapi_free_AltNumStr(ans);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42");
     visit_type_AltStrInt(v, &asi, NULL, &error_abort);
-    g_assert_cmpint(asi->kind, ==, ALT_STR_INT_KIND_I);
+    g_assert_cmpint(asi->type, ==, ALT_STR_INT_KIND_I);
     g_assert_cmpint(asi->i, ==, 42);
     qapi_free_AltStrInt(asi);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42");
     visit_type_AltIntNum(v, &ain, NULL, &error_abort);
-    g_assert_cmpint(ain->kind, ==, ALT_INT_NUM_KIND_I);
+    g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_I);
     g_assert_cmpint(ain->i, ==, 42);
     qapi_free_AltIntNum(ain);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42");
     visit_type_AltNumInt(v, &ani, NULL, &error_abort);
-    g_assert_cmpint(ani->kind, ==, ALT_NUM_INT_KIND_I);
+    g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_I);
     g_assert_cmpint(ani->i, ==, 42);
     qapi_free_AltNumInt(ani);
     visitor_input_teardown(data, NULL);
@@ -433,14 +433,14 @@ static void 
test_visitor_in_alternate_number(TestInputVisitorData *data,

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltStrNum(v, &asn, NULL, &error_abort);
-    g_assert_cmpint(asn->kind, ==, ALT_STR_NUM_KIND_N);
+    g_assert_cmpint(asn->type, ==, ALT_STR_NUM_KIND_N);
     g_assert_cmpfloat(asn->n, ==, 42.5);
     qapi_free_AltStrNum(asn);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltNumStr(v, &ans, NULL, &error_abort);
-    g_assert_cmpint(ans->kind, ==, ALT_NUM_STR_KIND_N);
+    g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
     g_assert_cmpfloat(ans->n, ==, 42.5);
     qapi_free_AltNumStr(ans);
     visitor_input_teardown(data, NULL);
@@ -455,14 +455,14 @@ static void 
test_visitor_in_alternate_number(TestInputVisitorData *data,

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltIntNum(v, &ain, NULL, &error_abort);
-    g_assert_cmpint(ain->kind, ==, ALT_INT_NUM_KIND_N);
+    g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_N);
     g_assert_cmpfloat(ain->n, ==, 42.5);
     qapi_free_AltIntNum(ain);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltNumInt(v, &ani, NULL, &error_abort);
-    g_assert_cmpint(ani->kind, ==, ALT_NUM_INT_KIND_N);
+    g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_N);
     g_assert_cmpint(ani->n, ==, 42.5);
     qapi_free_AltNumInt(ani);
     visitor_input_teardown(data, NULL);
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 0ccae04..a710557 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -459,7 +459,7 @@ static void 
test_visitor_out_alternate(TestOutputVisitorData *data,
     Error *err = NULL;

     UserDefAlternate *tmp = g_malloc0(sizeof(UserDefAlternate));
-    tmp->kind = USER_DEF_ALTERNATE_KIND_I;
+    tmp->type = USER_DEF_ALTERNATE_KIND_I;
     tmp->i = 42;

     visit_type_UserDefAlternate(data->ov, &tmp, NULL, &err);
-- 
2.4.3