[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 28/28] qapi: Drop support for inline nested t
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v5 28/28] qapi: Drop support for inline nested types |
Date: |
Fri, 27 Mar 2015 11:45:33 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> writes:
> A future patch will be using a 'name':{dictionary} entry in the
> QAPI schema to specify a default value for an optional argument;
> but existing use of inline nested structs conflicts with that goal.
> Now that all commands have been changed to avoid inline nested
> structs, nuke support for them, and turn it into a hard error.
> Update the testsuite to reflect tighter parsing rules.
>
> Signed-off-by: Eric Blake <address@hidden>
> ---
> scripts/qapi-commands.py | 8 +++---
> scripts/qapi-event.py | 4 +--
> scripts/qapi-types.py | 9 ++-----
> scripts/qapi-visit.py | 37
> ++++------------------------
> scripts/qapi.py | 18 +++++---------
> tests/qapi-schema/event-nest-struct.err | 2 +-
> tests/qapi-schema/nested-struct-data.err | 1 +
> tests/qapi-schema/nested-struct-data.exit | 2 +-
> tests/qapi-schema/nested-struct-data.json | 2 +-
> tests/qapi-schema/nested-struct-data.out | 3 ---
> tests/qapi-schema/nested-struct-returns.err | 1 +
> tests/qapi-schema/nested-struct-returns.exit | 2 +-
> tests/qapi-schema/nested-struct-returns.json | 2 +-
> tests/qapi-schema/nested-struct-returns.out | 3 ---
> 14 files changed, 26 insertions(+), 68 deletions(-)
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 053ba85..db81044 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -28,7 +28,7 @@ def type_visitor(name):
>
> def generate_command_decl(name, args, ret_type):
> arglist=""
> - for argname, argtype, optional, structured in parse_args(args):
> + for argname, argtype, optional in parse_args(args):
> argtype = c_type(argtype, is_param=True)
> if optional:
> arglist += "bool has_%s, " % c_var(argname)
> @@ -53,7 +53,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
> retval=""
> if ret_type:
> retval = "retval = "
> - for argname, argtype, optional, structured in parse_args(args):
> + for argname, argtype, optional in parse_args(args):
> if optional:
> arglist += "has_%s, " % c_var(argname)
> arglist += "%s, " % (c_var(argname))
> @@ -96,7 +96,7 @@ Visitor *v;
> def gen_visitor_input_vars_decl(args):
> ret = ""
> push_indent()
> - for argname, argtype, optional, structured in parse_args(args):
> + for argname, argtype, optional in parse_args(args):
> if optional:
> ret += mcgen('''
> bool has_%(argname)s = false;
> @@ -139,7 +139,7 @@ v = qapi_dealloc_get_visitor(md);
> v = qmp_input_get_visitor(mi);
> ''')
>
> - for argname, argtype, optional, structured in parse_args(args):
> + for argname, argtype, optional in parse_args(args):
> if optional:
> ret += mcgen('''
> visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> index 601e307..47dc041 100644
> --- a/scripts/qapi-event.py
> +++ b/scripts/qapi-event.py
> @@ -21,7 +21,7 @@ def _generate_event_api_name(event_name, params):
> l = len(api_name)
>
> if params:
> - for argname, argentry, optional, structured in parse_args(params):
> + for argname, argentry, optional in parse_args(params):
> if optional:
> api_name += "bool has_%s,\n" % c_var(argname)
> api_name += "".ljust(l)
> @@ -93,7 +93,7 @@ def generate_event_implement(api_name, event_name, params):
> """,
> event_name = event_name)
>
> - for argname, argentry, optional, structured in parse_args(params):
> + for argname, argentry, optional in parse_args(params):
> if optional:
> ret += mcgen("""
> if (has_%(var)s) {
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 9c8d68c..4789528 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -63,18 +63,13 @@ typedef struct %(name)sList
> def generate_struct_fields(members):
> ret = ''
>
> - for argname, argentry, optional, structured in parse_args(members):
> + for argname, argentry, optional in parse_args(members):
> if optional:
> ret += mcgen('''
> bool has_%(c_name)s;
> ''',
> c_name=c_var(argname))
> - if structured:
> - push_indent()
> - ret += generate_struct({ "field": argname, "data": argentry})
> - pop_indent()
> - else:
> - ret += mcgen('''
> + ret += mcgen('''
> %(c_type)s %(c_name)s;
> ''',
> c_type=c_type(argentry), c_name=c_var(argname))
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 3e11089..10b08c6 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -51,27 +51,6 @@ def generate_visit_struct_fields(name, field_prefix,
> fn_prefix, members, base =
> else:
> full_name = "%s_%s" % (name, fn_prefix)
>
> - for argname, argentry, optional, structured in parse_args(members):
> - if structured:
> - if not fn_prefix:
> - nested_fn_prefix = argname
> - else:
> - nested_fn_prefix = "%s_%s" % (fn_prefix, argname)
> -
> - nested_field_prefix = "%s%s." % (field_prefix, argname)
> - ret += generate_visit_struct_fields(name, nested_field_prefix,
> - nested_fn_prefix, argentry)
> - ret += mcgen('''
> -
> -static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s
> **obj, Error **errp)
> -{
> -''',
> - name=name, full_name=full_name,
> c_name=c_var(argname))
> - ret += generate_visit_struct_body(full_name, argname, argentry)
> - ret += mcgen('''
> -}
> -''')
> -
> if base:
> ret += generate_visit_implicit_struct(base)
>
> @@ -94,7 +73,7 @@ if (err) {
> c_prefix=c_var(field_prefix),
> type=type_name(base), c_name=c_var('base'))
>
> - for argname, argentry, optional, structured in parse_args(members):
> + for argname, argentry, optional in parse_args(members):
> if optional:
> ret += mcgen('''
> visit_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", &err);
> @@ -104,18 +83,12 @@ if (!err && (*obj)->%(prefix)shas_%(c_name)s) {
> c_name=c_var(argname), name=argname)
> push_indent()
>
> - if structured:
> - ret += mcgen('''
> -visit_type_%(full_name)s_field_%(c_name)s(m, obj, &err);
> -''',
> - full_name=full_name, c_name=c_var(argname))
> - else:
> - ret += mcgen('''
> + ret += mcgen('''
> visit_type_%(type)s(m, &(*obj)->%(c_prefix)s%(c_name)s, "%(name)s", &err);
> ''',
> - c_prefix=c_var(field_prefix), prefix=field_prefix,
> - type=type_name(argentry), c_name=c_var(argname),
> - name=argname)
> + c_prefix=c_var(field_prefix), prefix=field_prefix,
> + type=type_name(argentry), c_name=c_var(argname),
> + name=argname)
>
> if optional:
> pop_indent()
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 800e8e4..8e5b4ad 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -368,11 +368,13 @@ def check_type(expr_info, source, data, allow_array =
> False,
> for (key, value) in data.items():
> check_name(expr_info, "Member of %s" % source, key,
> allow_optional=allow_optional)
> + # Todo: allow dictionaries to represent default values of
> + # an optional argument.
> check_type(expr_info, "Member '%s' of %s" % (key, source), value,
> allow_array=True,
> allowed_metas=['built-in', 'union', 'alternate', 'struct',
> 'enum'],
> - allow_dict=True, allow_optional=True,
> allow_star=allow_star)
> + allow_dict=False, allow_star=allow_star)
check_type() uses allow_optional only when allow_dict. Okay.
>
> def check_command(expr, expr_info):
> name = expr['command']
> @@ -404,13 +406,6 @@ def check_event(expr, expr_info):
> check_type(expr_info, "'data' for event '%s'" % name,
> expr.get('data'), allowed_metas=['union', 'struct'],
> allow_optional=True)
> - if params:
> - for argname, argentry, optional, structured in parse_args(params):
> - if structured:
> - raise QAPIExprError(expr_info,
> - "Nested structure define in event is not
> "
> - "supported, event '%s', argname '%s'"
> - % (expr['event'], argname))
>
> def check_union(expr, expr_info):
> name = expr['union']
> @@ -667,13 +662,12 @@ def parse_args(typeinfo):
> argname = member
> argentry = typeinfo[member]
> optional = False
> - structured = False
> if member.startswith('*'):
> argname = member[1:]
> optional = True
> - if isinstance(argentry, OrderedDict):
> - structured = True
> - yield (argname, argentry, optional, structured)
> + # Todo: allow argentry to be OrderedDict, for providing the
> + # value of an optional argument.
> + yield (argname, argentry, optional)
>
> def de_camel_case(name):
> new_name = ''
> diff --git a/tests/qapi-schema/event-nest-struct.err
> b/tests/qapi-schema/event-nest-struct.err
> index 91bde1c..5a42701 100644
> --- a/tests/qapi-schema/event-nest-struct.err
> +++ b/tests/qapi-schema/event-nest-struct.err
> @@ -1 +1 @@
> -tests/qapi-schema/event-nest-struct.json:1: Nested structure define in event
> is not supported, event 'EVENT_A', argname 'a'
> +tests/qapi-schema/event-nest-struct.json:1: Member 'a' of 'data' for event
> 'EVENT_A' should be a type name
> diff --git a/tests/qapi-schema/nested-struct-data.err
> b/tests/qapi-schema/nested-struct-data.err
> index e69de29..da767ba 100644
> --- a/tests/qapi-schema/nested-struct-data.err
> +++ b/tests/qapi-schema/nested-struct-data.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/nested-struct-data.json:2: Member 'a' of 'data' for
> command 'foo' should be a type name
> diff --git a/tests/qapi-schema/nested-struct-data.exit
> b/tests/qapi-schema/nested-struct-data.exit
> index 573541a..d00491f 100644
> --- a/tests/qapi-schema/nested-struct-data.exit
> +++ b/tests/qapi-schema/nested-struct-data.exit
> @@ -1 +1 @@
> -0
> +1
> diff --git a/tests/qapi-schema/nested-struct-data.json
> b/tests/qapi-schema/nested-struct-data.json
> index 0247c8c..3d52d2b 100644
> --- a/tests/qapi-schema/nested-struct-data.json
> +++ b/tests/qapi-schema/nested-struct-data.json
> @@ -1,4 +1,4 @@
> -# FIXME: inline subtypes collide with our desired future use of defaults
Seen only now: I'd mark this TODO rather than FIXME in PATCH 19, because
it's not actually broken. Only if you respin anyway, of course.
> +# inline subtypes collide with our desired future use of defaults
> { 'command': 'foo',
> 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' },
> 'returns': {} }
> diff --git a/tests/qapi-schema/nested-struct-data.out
> b/tests/qapi-schema/nested-struct-data.out
> index 999cbb8..e69de29 100644
> --- a/tests/qapi-schema/nested-struct-data.out
> +++ b/tests/qapi-schema/nested-struct-data.out
> @@ -1,3 +0,0 @@
> -[OrderedDict([('command', 'foo'), ('data', OrderedDict([('a',
> OrderedDict([('string', 'str'), ('integer', 'int')])), ('b', 'str')])),
> ('returns', OrderedDict())])]
> -[]
> -[]
> diff --git a/tests/qapi-schema/nested-struct-returns.err
> b/tests/qapi-schema/nested-struct-returns.err
> index e69de29..5238d07 100644
> --- a/tests/qapi-schema/nested-struct-returns.err
> +++ b/tests/qapi-schema/nested-struct-returns.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/nested-struct-returns.json:2: Member 'a' of 'returns' for
> command 'foo' should be a type name
> diff --git a/tests/qapi-schema/nested-struct-returns.exit
> b/tests/qapi-schema/nested-struct-returns.exit
> index 573541a..d00491f 100644
> --- a/tests/qapi-schema/nested-struct-returns.exit
> +++ b/tests/qapi-schema/nested-struct-returns.exit
> @@ -1 +1 @@
> -0
> +1
> diff --git a/tests/qapi-schema/nested-struct-returns.json
> b/tests/qapi-schema/nested-struct-returns.json
> index 5a46840..d2cd047 100644
> --- a/tests/qapi-schema/nested-struct-returns.json
> +++ b/tests/qapi-schema/nested-struct-returns.json
> @@ -1,3 +1,3 @@
> -# FIXME: inline subtypes collide with our desired future use of defaults
> +# inline subtypes collide with our desired future use of defaults
> { 'command': 'foo',
> 'returns': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
> diff --git a/tests/qapi-schema/nested-struct-returns.out
> b/tests/qapi-schema/nested-struct-returns.out
> index c53d23b..e69de29 100644
> --- a/tests/qapi-schema/nested-struct-returns.out
> +++ b/tests/qapi-schema/nested-struct-returns.out
> @@ -1,3 +0,0 @@
> -[OrderedDict([('command', 'foo'), ('returns', OrderedDict([('a',
> OrderedDict([('string', 'str'), ('integer', 'int')])), ('b', 'str')]))])]
> -[]
> -[]
Reviewed-by: Markus Armbruster <address@hidden>
- Re: [Qemu-devel] [PATCH v5 21/28] qapi: Require valid names, (continued)
Re: [Qemu-devel] [PATCH v5 21/28] qapi: Require valid names, Markus Armbruster, 2015/03/27
Re: [Qemu-devel] [PATCH v5 21/28] qapi: Require valid names, Markus Armbruster, 2015/03/29
[Qemu-devel] [PATCH v5 27/28] qapi: Drop inline nested types in query-pci, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 28/28] qapi: Drop support for inline nested types, Eric Blake, 2015/03/24
- Re: [Qemu-devel] [PATCH v5 28/28] qapi: Drop support for inline nested types,
Markus Armbruster <=
[Qemu-devel] [PATCH v5 24/28] qapi: Merge UserDefTwo and UserDefNested in tests, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 25/28] qapi: Drop tests for inline nested structs, Eric Blake, 2015/03/24
Re: [Qemu-devel] [PATCH v5 00/28] drop qapi nested structs, Markus Armbruster, 2015/03/27
Re: [Qemu-devel] [PATCH v5 00/28] drop qapi nested structs, Markus Armbruster, 2015/03/29