From 441cbac0c7e641780decbc674a9a68c6a5200f71 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 16 Sep 2015 13:06:10 +0200 Subject: qapi-visit: Convert to QAPISchemaVisitor, fixing bugs 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(). Code is generated in a different order now, and therefore has got a few new forward declarations. Doesn't matter. The guard QAPI_VISIT_BUILTIN_VISITOR_DECL is renamed to QAPI_VISIT_BUILTIN. The previous commit's two ugly special cases exist here, too. Mark both TODO. Signed-off-by: Markus Armbruster Reviewed-by: Daniel P. Berrange Reviewed-by: Eric Blake --- scripts/qapi-visit.py | 272 +++++++++++++++++++++----------------------------- 1 file changed, 114 insertions(+), 158 deletions(-) (limited to 'scripts/qapi-visit.py') diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index c493964..8e6e8ca 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(''' } @@ -136,12 +135,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(''' @@ -158,10 +152,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; @@ -174,8 +168,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); @@ -185,7 +179,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(''' @@ -197,7 +192,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) @@ -216,25 +211,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: @@ -251,35 +238,17 @@ 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: + # Ugly special case for simple union TODO get rid of it + if not var.simple_union_type(): + ret += generate_visit_implicit_struct(var.type) ret += mcgen(''' @@ -304,41 +273,44 @@ 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(), + # TODO ugly special case for simple union + # Use same tag name in C as on the wire to get rid of + # it, then: c_name=c_name(variants.tag_member.name) + c_name=c_name(variants.tag_name or 'kind'), disc_key = disc_key) - for key in members: - if not discriminator: + for var in variants.variants: + # TODO ugly special case for simple union + simple_union_type = var.simple_union_type() + if simple_union_type: 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=(simple_union_type or var.type).c_name(), + c_name=c_name(var.name)) ret += mcgen(''' default: @@ -359,38 +331,68 @@ 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, schema): + self.decl = '' + self.defn = '' + self._btin = guardstart('QAPI_VISIT_BUILTIN') + + 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. See also do_builtins (command line + # option -b). + self._btin += guardend('QAPI_VISIT_BUILTIN') + self.decl = self._btin + self.decl + self._btin = None + + def visit_enum_type(self, name, info, values, prefix): + 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: + assert not members # not implemented + 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) + +# If you link code generated from multiple schemata, you want only one +# instance of the code for built-in types. Generate it only when +# do_builtins, enabled by command line option -b. See also +# QAPISchemaGenVisitVisitor.visit_end(). do_builtins = False (input_file, output_dir, do_c, do_h, prefix, opts) = \ @@ -446,56 +448,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) -- cgit v1.1