mirror of
https://github.com/xemu-project/xemu.git
synced 2024-11-24 03:59:52 +00:00
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 <armbru@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
parent
2b162ccbe8
commit
441cbac0c7
@ -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)
|
||||
|
@ -44,8 +44,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',
|
||||
@ -62,7 +60,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' } }
|
||||
|
@ -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);
|
||||
@ -272,7 +272,7 @@ static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
|
||||
Visitor *v;
|
||||
|
||||
/* test situation where discriminator field ('enum1' here) is missing */
|
||||
v = validate_test_init(data, "{ 'string': 'c', 'string1': 'd', 'string2': 'e' }");
|
||||
v = validate_test_init(data, "{ 'integer': 42, 'string': 'c', 'string1': 'd', 'string2': 'e' }");
|
||||
|
||||
visit_type_UserDefFlatUnion2(v, &tmp, NULL, &err);
|
||||
g_assert(err);
|
||||
|
@ -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);
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user