From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44495) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAOX1-0003eF-8G for qemu-devel@nongnu.org; Wed, 01 Jul 2015 16:23:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZAOWv-0006BK-2H for qemu-devel@nongnu.org; Wed, 01 Jul 2015 16:22:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34929) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAOWu-0006Aa-As for qemu-devel@nongnu.org; Wed, 01 Jul 2015 16:22:48 -0400 From: Markus Armbruster Date: Wed, 1 Jul 2015 22:22:15 +0200 Message-Id: <1435782155-31412-28-git-send-email-armbru@redhat.com> In-Reply-To: <1435782155-31412-1-git-send-email-armbru@redhat.com> References: <1435782155-31412-1-git-send-email-armbru@redhat.com> Subject: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, berto@igalia.com, mdroth@linux.vnet.ibm.com Fixes flat unions to visit the base's base members (the previous commit merely added them to the struct). Same test case. Patch's effect on visit_type_UserDefFlatUnion(): static void visit_type_UserDefFlatUnion_fields(Visitor *m, UserDefFlatUnion **obj, Error **errp) { Error *err = NULL; + visit_type_int(m, &(*obj)->integer, "integer", &err); + if (err) { + goto out; + } visit_type_str(m, &(*obj)->string, "string", &err); if (err) { goto out; Test cases updated for the bug fix. Fixes alternates to generate a visitor for their implicit enumeration type. None of them are currently used, obviously. Example: block-core.json's BlockdevRef now generates visit_type_BlockdevRefKind(). Signed-off-by: Markus Armbruster --- scripts/qapi-visit.py | 254 ++++++++++++-------------------- tests/qapi-schema/qapi-schema-test.json | 3 - tests/test-qmp-input-strict.c | 2 +- tests/test-qmp-input-visitor.c | 4 +- 4 files changed, 100 insertions(+), 163 deletions(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index a52a572..135e7c1 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -12,7 +12,6 @@ # This work is licensed under the terms of the GNU GPL, version 2. # See the COPYING file in the top-level directory. -from ordereddict import OrderedDict from qapi import * import re @@ -24,13 +23,13 @@ def generate_visit_implicit_struct(type): return '' implicit_structs_seen.add(type) ret = '' - if type not in struct_fields_seen: + if type.name not in struct_fields_seen: # Need a forward declaration ret += mcgen(''' static void visit_type_%(c_type)s_fields(Visitor *m, %(c_type)s **obj, Error **errp); ''', - c_type=type_name(type)) + c_type=type.c_name()) ret += mcgen(''' @@ -46,7 +45,7 @@ static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error * error_propagate(errp, err); } ''', - c_type=type_name(type)) + c_type=type.c_name()) return ret def generate_visit_struct_fields(name, members, base = None): @@ -74,24 +73,24 @@ if (err) { goto out; } ''', - type=type_name(base), c_name=c_name('base')) + type=base.c_name(), c_name=c_name('base')) - for argname, argentry, optional in parse_args(members): - if optional: + for memb in members: + if memb.optional: ret += mcgen(''' visit_optional(m, &(*obj)->has_%(c_name)s, "%(name)s", &err); if (!err && (*obj)->has_%(c_name)s) { ''', - c_name=c_name(argname), name=argname) + c_name=c_name(memb.name), name=memb.name) push_indent() ret += mcgen(''' visit_type_%(type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err); ''', - type=type_name(argentry), c_name=c_name(argname), - name=argname) + type=memb.type.c_name(), c_name=c_name(memb.name), + name=memb.name) - if optional: + if memb.optional: pop_indent() ret += mcgen(''' } @@ -132,12 +131,7 @@ def generate_visit_struct_body(name): return ret -def generate_visit_struct(expr): - - name = expr['struct'] - members = expr['data'] - base = expr.get('base') - +def gen_visit_struct(name, base, members): ret = generate_visit_struct_fields(name, members, base) ret += mcgen(''' @@ -154,10 +148,10 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e ''') return ret -def generate_visit_list(name): +def gen_visit_list(name, element_type): return mcgen(''' -void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp) +void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp) { Error *err = NULL; GenericList *i, **prev; @@ -170,8 +164,8 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, E for (prev = (GenericList **)obj; !err && (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) { - %(name)sList *native_i = (%(name)sList *)i; - visit_type_%(name)s(m, &native_i->value, NULL, &err); + %(name)s *native_i = (%(name)s *)i; + visit_type_%(c_elt_type)s(m, &native_i->value, NULL, &err); } error_propagate(errp, err); @@ -181,7 +175,8 @@ out: error_propagate(errp, err); } ''', - name=type_name(name)) + name=c_name(name), + c_elt_type=element_type.c_name()) def generate_visit_enum(name): return mcgen(''' @@ -193,7 +188,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, Error ''', c_name=c_name(name), name=name) -def generate_visit_alternate(name, members): +def gen_visit_alternate(name, variants): ret = mcgen(''' void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp) @@ -212,25 +207,17 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e ''', name=c_name(name)) - # For alternate, always use the default enum type automatically generated - # as name + 'Kind' - disc_type = c_name(name) + 'Kind' - - for key in members: - assert (members[key] in builtin_types.keys() - or find_struct(members[key]) - or find_union(members[key]) - or find_enum(members[key])), "Invalid alternate member" - - enum_full_value = c_enum_const(disc_type, key) + for var in variants.variants: + enum_full_value = c_enum_const(variants.tag_member.type.name, + var.name) ret += mcgen(''' case %(enum_full_value)s: visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err); break; ''', enum_full_value = enum_full_value, - c_type = type_name(members[key]), - c_name = c_name(key)) + c_type=var.type.c_name(), + c_name=c_name(var.name)) ret += mcgen(''' default: @@ -247,35 +234,16 @@ out: return ret - -def generate_visit_union(expr): - - name = expr['union'] - members = expr['data'] - - base = expr.get('base') - discriminator = expr.get('discriminator') - - enum_define = discriminator_find_enum_define(expr) - if enum_define: - # Use the enum type as discriminator - ret = "" - disc_type = c_name(enum_define['enum_name']) - else: - # There will always be a discriminator in the C switch code, by default - # it is an enum type generated silently - ret = generate_visit_enum(name + 'Kind') - disc_type = c_name(name) + 'Kind' +def gen_visit_union(name, base, variants): + ret = '' if base: - assert discriminator - base_fields = find_struct(base)['data'].copy() - del base_fields[discriminator] - ret += generate_visit_struct_fields(name, base_fields) + members = [m for m in base.members if m != variants.tag_member] + ret += generate_visit_struct_fields(name, members) - if discriminator: - for key in members: - ret += generate_visit_implicit_struct(members[key]) + for var in variants.variants: + if var.flat: + ret += generate_visit_implicit_struct(var.type) ret += mcgen(''' @@ -300,41 +268,39 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error ''', name=c_name(name)) - if not discriminator: - tag = 'kind' - disc_key = "type" - else: - tag = discriminator - disc_key = discriminator + disc_key = variants.tag_member.name + if not variants.tag_name: + # we pointlessly use a different key for simple unions + disc_key = 'type' ret += mcgen(''' - visit_type_%(disc_type)s(m, &(*obj)->%(c_tag)s, "%(disc_key)s", &err); + visit_type_%(disc_type)s(m, &(*obj)->%(c_name)s, "%(disc_key)s", &err); if (err) { goto out_obj; } if (!visit_start_union(m, !!(*obj)->data, &err) || err) { goto out_obj; } - switch ((*obj)->%(c_tag)s) { + switch ((*obj)->%(c_name)s) { ''', - disc_type = disc_type, - c_tag=c_name(tag), + disc_type=variants.tag_member.type.c_name(), + c_name=c_name(variants.tag_member.name), disc_key = disc_key) - for key in members: - if not discriminator: + for var in variants.variants: + if not var.flat: fmt = 'visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err);' else: fmt = 'visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);' - enum_full_value = c_enum_const(disc_type, key) + enum_full_value = c_enum_const(variants.tag_member.type.name, var.name) ret += mcgen(''' case %(enum_full_value)s: ''' + fmt + ''' break; ''', enum_full_value = enum_full_value, - c_type=type_name(members[key]), - c_name=c_name(key)) + c_type=var.type.c_name(), + c_name=c_name(var.name)) ret += mcgen(''' default: @@ -355,37 +321,57 @@ out: return ret -def generate_declaration(name, builtin_type=False): - ret = "" - if not builtin_type: - name = c_name(name) - ret += mcgen(''' - -void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp); -''', - name=name) - - ret += mcgen(''' -void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp); -''', - name=name) - - return ret - -def generate_enum_declaration(name): - ret = mcgen(''' -void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp); -''', - name=c_name(name)) - - return ret - -def generate_decl_enum(name): +def gen_visit_decl(name, scalar=False): + c_type = c_name(name) + ' *' + if not scalar: + c_type += '*' return mcgen(''' - -void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **errp); +void visit_type_%(c_name)s(Visitor *m, %(c_type)sobj, const char *name, Error **errp); ''', - name=c_name(name)) + c_name=c_name(name), c_type=c_type) + +class QAPISchemaGenVisitVisitor(QAPISchemaVisitor): + def __init__(self): + self.decl = None + self.defn = None + self.btin = None + def visit_begin(self): + self.decl = '' + self.defn = '' + self.btin = guardstart('QAPI_VISIT_BUILTIN_VISITOR_DECL') + def visit_end(self): + # to avoid header dependency hell, we always generate + # declarations for built-in types in our header files and + # simply guard them + self.btin += guardend('QAPI_VISIT_BUILTIN_VISITOR_DECL') + self.decl = self.btin + self.decl + self.btin = None + # ...this doesn't work for cases where we link in multiple + # objects that have the functions defined, so we use + # do_builtins (option -b) to provide control + def visit_enum_type(self, name, info, values): + self.decl += gen_visit_decl(name, scalar=True) + self.defn += generate_visit_enum(name) + def visit_array_type(self, name, info, element_type): + decl = gen_visit_decl(name) + defn = gen_visit_list(name, element_type) + if isinstance(element_type, QAPISchemaBuiltinType): + self.btin += decl + if do_builtins: + self.defn += defn + else: + self.decl += decl + self.defn += defn + def visit_object_type(self, name, info, base, members, variants): + if info: + self.decl += gen_visit_decl(name) + if variants: + self.defn += gen_visit_union(name, base, variants) + else: + self.defn += gen_visit_struct(name, base, members) + def visit_alternate_type(self, name, info, variants): + self.decl += gen_visit_decl(name) + self.defn += gen_visit_alternate(name, variants) do_builtins = False @@ -442,56 +428,10 @@ fdecl.write(mcgen(''' ''', prefix=prefix)) -exprs = QAPISchema(input_file).get_exprs() - -# to avoid header dependency hell, we always generate declarations -# for built-in types in our header files and simply guard them -fdecl.write(guardstart("QAPI_VISIT_BUILTIN_VISITOR_DECL")) -for typename in builtin_types.keys(): - fdecl.write(generate_declaration(typename, builtin_type=True)) -fdecl.write(guardend("QAPI_VISIT_BUILTIN_VISITOR_DECL")) - -# ...this doesn't work for cases where we link in multiple objects that -# have the functions defined, so we use -b option to provide control -# over these cases -if do_builtins: - for typename in builtin_types.keys(): - fdef.write(generate_visit_list(typename)) - -for expr in exprs: - if expr.has_key('struct'): - ret = generate_visit_struct(expr) - ret += generate_visit_list(expr['struct']) - fdef.write(ret) - - ret = generate_declaration(expr['struct']) - fdecl.write(ret) - elif expr.has_key('union'): - ret = generate_visit_union(expr) - ret += generate_visit_list(expr['union']) - fdef.write(ret) - - enum_define = discriminator_find_enum_define(expr) - ret = "" - if not enum_define: - ret = generate_decl_enum('%sKind' % expr['union']) - ret += generate_declaration(expr['union']) - fdecl.write(ret) - elif expr.has_key('alternate'): - ret = generate_visit_alternate(expr['alternate'], expr['data']) - ret += generate_visit_list(expr['alternate']) - fdef.write(ret) - - ret = generate_decl_enum('%sKind' % expr['alternate']) - ret += generate_declaration(expr['alternate']) - fdecl.write(ret) - elif expr.has_key('enum'): - ret = generate_visit_list(expr['enum']) - ret += generate_visit_enum(expr['enum']) - fdef.write(ret) - - ret = generate_decl_enum(expr['enum']) - ret += generate_enum_declaration(expr['enum']) - fdecl.write(ret) +schema = QAPISchema(input_file) +gen = QAPISchemaGenVisitVisitor() +schema.visit(gen) +fdef.write(gen.defn) +fdecl.write(gen.decl) close_output(fdef, fdecl) diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 257b4d4..b182174 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -39,8 +39,6 @@ 'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefB', 'value3' : 'UserDefB' } } -# FIXME generated visit_type_UserDefFlatUnion_fields() fails to visit -# members of indirect base UserDefZero { 'struct': 'UserDefUnionBase', 'base': 'UserDefZero', @@ -57,7 +55,6 @@ { 'alternate': 'UserDefAlternate', 'data': { 'uda': 'UserDefA', 's': 'str', 'i': 'int' } } -# FIXME only a declaration of visit_type_UserDefAlternateKind() generated { 'struct': 'UserDefC', 'data': { 'string1': 'str', 'string2': 'str' } } diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c index 68f855b..00c3e29 100644 --- a/tests/test-qmp-input-strict.c +++ b/tests/test-qmp-input-strict.c @@ -167,9 +167,9 @@ static void test_validate_union_flat(TestInputVisitorData *data, v = validate_test_init(data, "{ 'enum1': 'value1', " + "'integer': 41, " "'string': 'str', " "'boolean': true }"); - /* TODO when generator bug is fixed, add 'integer': 41 */ visit_type_UserDefFlatUnion(v, &tmp, NULL, &err); g_assert(!err); diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c index b7a87ee..56da3c6 100644 --- a/tests/test-qmp-input-visitor.c +++ b/tests/test-qmp-input-visitor.c @@ -307,15 +307,15 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data, v = visitor_input_test_init(data, "{ 'enum1': 'value1', " + "'integer': 41, " "'string': 'str', " "'boolean': true }"); - /* TODO when generator bug is fixed, add 'integer': 41 */ visit_type_UserDefFlatUnion(v, &tmp, NULL, &err); g_assert(err == NULL); g_assert_cmpint(tmp->enum1, ==, ENUM_ONE_VALUE1); g_assert_cmpstr(tmp->string, ==, "str"); - /* TODO g_assert_cmpint(tmp->integer, ==, 41); */ + g_assert_cmpint(tmp->integer, ==, 41); g_assert_cmpint(tmp->value1->boolean, ==, true); qapi_free_UserDefFlatUnion(tmp); } -- 1.9.3