Eric Blake <ebl...@redhat.com> writes:
> What's more meta than using qapi to define qapi? :)
>
> Convert QType into a full-fledged[*] builtin qapi enum type, so
> that a subsequent patch can then use it as the discriminator
> type of qapi alternate types. Fortunately, the judicious use of
> 'prefix' in the qapi definition avoids churn to the spelling of
> the enum constants.
>
> To avoid circular definitions, we have to flip the order of
> inclusion between "qobject.h" vs. "qapi-types.h". Back in commit
> 28770e0, we had the latter include the former, so that we could
> use 'QObject *' for our implementation of 'any'. But that usage
> also works with only a forward declaration, whereas the
> definition of QObject requires QType to be a complete type.
>
> [*] The type has to be builtin, rather than declared in
> qapi/common.json, because we want to use it for alternates even
> when common.json is not included. But since it is the first
> builtin enum type, we have to add special cases to qapi-types
> and qapi-visit to only emit definitions once, even when two
> qapi files are being compiled into the same binary (the way we
> already handled builtin list types like 'intList'). We may
> need to revisit how multiple qapi files share common types,
> but that's a project for another day.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v13: change title, use typedefs.h, rebase to cleaner QObject,
> rebase to Markus' typedefs.h re-sort
> v12: split out preparatory renames, retitle patch, use info rather
> than name comparison
> v11: new patch
> ---
> docs/qapi-code-gen.txt | 1 +
> include/qapi/qmp/qobject.h | 17 +++--------------
> include/qemu/typedefs.h | 1 +
> qobject/qobject.c | 4 ++--
> scripts/qapi-types.py | 15 +++++++++++----
> scripts/qapi-visit.py | 11 +++++++++--
> scripts/qapi.py | 6 ++++++
> tests/qapi-schema/alternate-empty.out | 2 ++
> tests/qapi-schema/comments.out | 2 ++
> tests/qapi-schema/empty.out | 2 ++
> tests/qapi-schema/event-case.out | 2 ++
> tests/qapi-schema/flat-union-empty.out | 2 ++
> tests/qapi-schema/ident-with-escape.out | 2 ++
> tests/qapi-schema/include-relpath.out | 2 ++
> tests/qapi-schema/include-repetition.out | 2 ++
> tests/qapi-schema/include-simple.out | 2 ++
> tests/qapi-schema/indented-expr.out | 2 ++
> tests/qapi-schema/qapi-schema-test.out | 2 ++
> tests/qapi-schema/union-clash-data.out | 2 ++
> tests/qapi-schema/union-empty.out | 2 ++
> 20 files changed, 59 insertions(+), 22 deletions(-)
>
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 2becba9..79bf072 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -160,6 +160,7 @@ The following types are predefined, and map to C as
> follows:
> accepts size suffixes
> bool bool JSON true or false
> any QObject * any JSON value
> + QType QType JSON string matching enum QType values
>
>
> === Includes ===
> diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
> index 525fb39..295aa76 100644
> --- a/include/qapi/qmp/qobject.h
> +++ b/include/qapi/qmp/qobject.h
> @@ -34,23 +34,12 @@
>
> #include <stddef.h>
> #include <assert.h>
> +#include "qapi-types.h"
Needed for QType. Risk for circular inclusion. We're currently fine,
because generated qapi-types.h includes only "qemu/typedefs.h" (visible
below). Should we add a comment to qapi-types.py?
>
> -typedef enum {
> - QTYPE_NONE, /* sentinel value, no QObject has this type code */
> - QTYPE_QNULL,
> - QTYPE_QINT,
> - QTYPE_QSTRING,
> - QTYPE_QDICT,
> - QTYPE_QLIST,
> - QTYPE_QFLOAT,
> - QTYPE_QBOOL,
> - QTYPE_MAX,
> -} QType;
> -
> -typedef struct QObject {
> +struct QObject {
> QType type;
> size_t refcnt;
> -} QObject;
> +};
>
> typedef void (*QDestroy)(QObject *);
>
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 3eedcf4..78fe6e8 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -80,6 +80,7 @@ typedef struct QEMUSGList QEMUSGList;
> typedef struct QEMUSizedBuffer QEMUSizedBuffer;
> typedef struct QEMUTimer QEMUTimer;
> typedef struct QEMUTimerListGroup QEMUTimerListGroup;
> +typedef struct QObject QObject;
> typedef struct RAMBlock RAMBlock;
> typedef struct Range Range;
> typedef struct SerialState SerialState;
> diff --git a/qobject/qobject.c b/qobject/qobject.c
> index 5325447..d0eb7f1 100644
> --- a/qobject/qobject.c
> +++ b/qobject/qobject.c
> @@ -15,7 +15,7 @@
> #include "qapi/qmp/qlist.h"
> #include "qapi/qmp/qstring.h"
>
> -static QDestroy qdestroy[QTYPE_MAX] = {
> +static QDestroy qdestroy[QTYPE__MAX] = {
> [QTYPE_NONE] = NULL, /* No such object exists */
> [QTYPE_QNULL] = NULL, /* qnull_ is indestructible */
> [QTYPE_QINT] = qint_destroy_obj,
> @@ -29,6 +29,6 @@ static QDestroy qdestroy[QTYPE_MAX] = {
> void qobject_destroy(QObject *obj)
> {
> assert(!obj->refcnt);
> - assert(QTYPE_QNULL < obj->type && obj->type < QTYPE_MAX);
> + assert(QTYPE_QNULL < obj->type && obj->type < QTYPE__MAX);
> qdestroy[obj->type](obj);
> }
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 2f2f7df..82635b0 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -112,7 +112,7 @@ extern const int %(c_name)s_qtypes[];
> def gen_alternate_qtypes(name, variants):
> ret = mcgen('''
>
> -const int %(c_name)s_qtypes[QTYPE_MAX] = {
> +const int %(c_name)s_qtypes[QTYPE__MAX] = {
> ''',
> c_name=c_name(name))
>
> @@ -233,8 +233,15 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
> self.defn += gen_type_cleanup(name)
>
> def visit_enum_type(self, name, info, values, prefix):
> - self._fwdecl += gen_enum(name, values, prefix)
> - self._fwdefn += gen_enum_lookup(name, values, prefix)
> + # Special case for our lone builtin enum type
> + # TODO use something cleaner than existence of info
> + if not info:
> + self._btin += gen_enum(name, values, prefix)
> + if do_builtins:
> + self.defn += gen_enum_lookup(name, values, prefix)
> + else:
> + self._fwdecl += gen_enum(name, values, prefix)
> + self._fwdefn += gen_enum_lookup(name, values, prefix)
Odd: gen_enum_lookup() goes into .defn for built-ins, but ._fwdefn for
user-defineds. Makes me suspect it ._fwdefn isn't needed anymore. A
quick test compile is happy with .defn for both.
If we want to keep ._fwdefn for some reason, we should use for built-ins
as well.
>
> def visit_array_type(self, name, info, element_type):
> if isinstance(element_type, QAPISchemaBuiltinType):
> @@ -319,7 +326,7 @@ fdef.write(mcgen('''
> fdecl.write(mcgen('''
> #include <stdbool.h>
> #include <stdint.h>
> -#include "qapi/qmp/qobject.h"
> +#include "qemu/typedefs.h"
> '''))
>
Here you can see the #include generated into qapi-types.h.
> schema = QAPISchema(input_file)
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 94cd113..7ceda18 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -347,8 +347,15 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
> isinstance(entity, QAPISchemaObjectType))
>
> def visit_enum_type(self, name, info, values, prefix):
> - self.decl += gen_visit_decl(name, scalar=True)
> - self.defn += gen_visit_enum(name)
> + # Special case for our lone builtin enum type
> + # TODO use something cleaner than existence of info
> + if not info:
> + self._btin += gen_visit_decl(name, scalar=True)
> + if do_builtins:
> + self.defn += gen_visit_enum(name)
> + else:
> + self.decl += gen_visit_decl(name, scalar=True)
> + self.defn += gen_visit_enum(name)
>
> def visit_array_type(self, name, info, element_type):
> decl = gen_visit_decl(name)
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 9fe9c95..3c8cb13 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -34,6 +34,7 @@ builtin_types = {
> 'uint64': 'QTYPE_QINT',
> 'size': 'QTYPE_QINT',
> 'any': None, # any QType possible, actually
> + 'QType': 'QTYPE_QSTRING',
> }
>
> # Whitelist of commands allowed to return a non-dictionary
> @@ -1243,6 +1244,11 @@ class QAPISchema(object):
> self.the_empty_object_type = QAPISchemaObjectType(':empty', None,
> None,
> [], None)
> self._def_entity(self.the_empty_object_type)
> + self._def_entity(QAPISchemaEnumType('QType', None,
> + ['none', 'qnull', 'qint',
> + 'qstring', 'qdict', 'qlist',
> + 'qfloat', 'qbool'],
> + 'QTYPE'))
>
> def _make_implicit_enum_type(self, name, info, values):
> name = name + 'Kind' # Use namespace reserved by add_name()
[...]
