Re: [Qemu-devel] [PATCH RFC v2 40/47] qapi: Introduce a first class 'any' type

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4930 bytes --]

On 07/01/2015 02:22 PM, Markus Armbruster wrote:
> It's first class, because unlike '**', it actually works, i.e. doesn't
> require 'gen': false.

Generated code grows in order to accommodate visiting the new anyList type:

 qapi-types.c                        |   15 +++++++++++++++
 qapi-types.h                        |   13 +++++++++++++
 qapi-visit.c                        |   24 ++++++++++++++++++++++++
 qapi-visit.h                        |    1 +
 qga/qapi-generated/qga-qapi-types.h |   13 +++++++++++++
 qga/qapi-generated/qga-qapi-visit.h |    1 +
 6 files changed, 67 insertions(+)

But do we really need anyList as a representation of qapi ['any'] ?  Or
will a later pass that minimizes array creation only to places where it
is needed help?

> 
> '**' will go away next.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/qapi-code-gen.txt            |  1 +
>  include/qapi/visitor-impl.h       |  2 ++
>  include/qapi/visitor.h            |  1 +
>  qapi/qapi-dealloc-visitor.c       |  9 +++++++
>  qapi/qapi-visit-core.c            |  6 +++++
>  qapi/qmp-input-visitor.c          | 11 ++++++++
>  qapi/qmp-output-visitor.c         |  9 +++++++
>  scripts/qapi-types.py             |  1 +
>  scripts/qapi.py                   |  9 ++++---
>  tests/qapi-schema/type-bypass.out |  4 +--
>  tests/test-qmp-input-visitor.c    | 45 +++++++++++++++++++++++++++++++++
>  tests/test-qmp-output-visitor.c   | 53 +++++++++++++++++++++++++++++++++++++++
>  12 files changed, 146 insertions(+), 5 deletions(-)

Touches a bit more this time, but that's okay.

> 
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index cb0fe75..bf9e854 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -158,6 +158,7 @@ The following types are predefined, and map to C as follows:
>    size      uint64_t   like uint64_t, except StringInputVisitor
>                         accepts size suffixes
>    bool      bool       JSON true or false
> +  any       QObject *  any JSON value

And with this, an 'alternate' type is just a special subset of 'any'
that limits the set of valid JSON values according to the qapi description.

> +++ b/qapi/qapi-visit-core.c
> @@ -260,6 +260,12 @@ void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
>      v->type_number(v, obj, name, errp);
>  }
>  
> +void visit_type_any(Visitor *v, QObject **obj, const char *name,
> +                         Error **errp)

Indentation looks off.

> @@ -1048,8 +1048,9 @@ class QAPISchema(object):
>                    ('uint64', 'int',     'uint64_t', '0'),
>                    ('size',   'int',     'uint64_t', '0'),
>                    ('bool',   'boolean', 'bool',     'false'),
> -                  ('**',     'value',   None,       None)]:
> +                  ('any',    'value',   'QObject' + pointer_suffix , 'NULL')]:
>              self._def_builtin_type(*t)
> +        self.entity_dict['**'] = self.lookup_type('any') # TODO drop this alias
>  
>      def _make_implicit_enum_type(self, name, values):
>          name = name + 'Kind'
> @@ -1190,6 +1191,8 @@ class QAPISchema(object):
>      def visit(self, visitor):
>          visitor.visit_begin()
>          for name in sorted(self.entity_dict.keys()):
> +            if self.entity_dict[name].name != name:
> +                continue        # ignore alias TODO drop alias and remove

Nice back-compat hacks while you transition into using it.

> +++ b/tests/test-qmp-input-visitor.c
> @@ -298,6 +298,49 @@ static void test_visitor_in_list(TestInputVisitorData *data,
>      qapi_free_UserDefOneList(head);
>  }
>  
> +static void test_visitor_in_any(TestInputVisitorData *data,
> +                                const void *unused)
> +{

> +
> +    v = visitor_input_test_init(data, "-42");

So we prove it accepts ints,...

> +    v = visitor_input_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }");

objects,...

> +    visit_type_any(v, &res, NULL, &err);
> +    g_assert(!err);
> +    qdict = qobject_to_qdict(res);
> +    g_assert(qdict);

worth asserting the dictionary has 3 keys?

> +}

...but no test of string, boolean, or array?

> +++ b/tests/test-qmp-output-visitor.c
> @@ -428,6 +428,57 @@ static void test_visitor_out_list_qapi_free(TestOutputVisitorData *data,
>      qapi_free_UserDefTwoList(head);
>  }
>  
> +static void test_visitor_out_any(TestOutputVisitorData *data,
> +                                 const void *unused)
> +{

> +}

Same tests of 'int' and 'object' but not of other JSON types.

Incomplete tests are still better than no tests, so what you have is a
good start.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]