Re: [Qemu-devel] [PATCH 2/9] qapi: Drop support for boxed alternate for commands, events

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Commands and events can define their argument type inline (default) or
> by referring to another type ('boxed': true, since commit c818408e44
> "qapi: Implement boxed types for commands/events", v2.7.0).  The
> unboxed inline definition is an (anonymous) struct type.  The boxed
> type may be a struct, union, or alternate type.
>
> The latter is problematic: docs/interop/qemu-spec.txt requires the
> value of the 'data' key to be a json-object, but any non-degenerate
> alternate type has at least one branch that isn't.
>
> Fortunately, we haven't made use of alternates in this context outside
> tests/.  Drop support for them.
>
> QAPISchemaAlternateType.is_empty() is now unused.  Drop it, too.
>
> Signed-off-by: Markus Armbruster <address@hidden>

Need to squash in

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index c54c148263..54d02458b5 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1711,15 +1711,13 @@ class QAPISchemaCommand(QAPISchemaEntity):
         QAPISchemaEntity.check(self, schema)
         if self._arg_type_name:
             self.arg_type = schema.lookup_type(self._arg_type_name)
-            assert (isinstance(self.arg_type, QAPISchemaObjectType) or
-                    isinstance(self.arg_type, QAPISchemaAlternateType))
+            assert isinstance(self.arg_type, QAPISchemaObjectType)
             self.arg_type.check(schema)
             if self.boxed:
                 if self.arg_type.is_empty():
                     raise QAPISemError(self.info,
                                        "Cannot use 'boxed' with empty type")
             else:
-                assert not isinstance(self.arg_type, QAPISchemaAlternateType)
                 assert not self.arg_type.variants
         elif self.boxed:
             raise QAPISemError(self.info, "Use of 'boxed' requires 'data'")
@@ -1747,15 +1745,13 @@ class QAPISchemaEvent(QAPISchemaEntity):
         QAPISchemaEntity.check(self, schema)
         if self._arg_type_name:
             self.arg_type = schema.lookup_type(self._arg_type_name)
-            assert (isinstance(self.arg_type, QAPISchemaObjectType) or
-                    isinstance(self.arg_type, QAPISchemaAlternateType))
+            assert isinstance(self.arg_type, QAPISchemaObjectType)
             self.arg_type.check(schema)
             if self.boxed:
                 if self.arg_type.is_empty():
                     raise QAPISemError(self.info,
                                        "Cannot use 'boxed' with empty type")
             else:
-                assert not isinstance(self.arg_type, QAPISchemaAlternateType)
                 assert not self.arg_type.variants
         elif self.boxed:
             raise QAPISemError(self.info, "Use of 'boxed' requires 'data'")


> ---
>  docs/devel/qapi-code-gen.txt            | 12 ++++++------
>  tests/qapi-schema/qapi-schema-test.json |  2 +-
>  scripts/qapi/common.py                  |  7 ++-----
>  tests/qapi-schema/qapi-schema-test.out  |  2 +-
>  4 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> index e8ec8ac1de..3d3931fb7a 100644
> --- a/docs/devel/qapi-code-gen.txt
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -612,9 +612,9 @@ the command.  Normally, 'data' is a dictionary for an 
> anonymous type,
>  or names a struct type (possibly empty, but not a union), and its
>  members are passed as separate arguments to this function.  If the
>  command definition includes a key 'boxed' with the boolean value true,
> -then 'data' is instead the name of any non-empty complex type
> -(struct, union, or alternate), and a pointer to that QAPI type is
> -passed as a single argument.
> +then 'data' is instead the name of any non-empty complex type (struct
> +or union), and a pointer to that QAPI type is passed as a single
> +argument.
>  
>  The generator also emits a marshalling function that extracts
>  arguments for the user's function out of an input QDict, calls the
> @@ -714,9 +714,9 @@ The generator emits a function to send the event.  
> Normally, 'data' is
>  a dictionary for an anonymous type, or names a struct type (possibly
>  empty, but not a union), and its members are passed as separate
>  arguments to this function.  If the event definition includes a key
> -'boxed' with the boolean value true, then 'data' is instead the name of
> -any non-empty complex type (struct, union, or alternate), and a
> -pointer to that QAPI type is passed as a single argument.
> +'boxed' with the boolean value true, then 'data' is instead the name
> +of any non-empty complex type (struct or union), and a pointer to that
> +QAPI type is passed as a single argument.
>  
>  
>  === Features ===
> diff --git a/tests/qapi-schema/qapi-schema-test.json 
> b/tests/qapi-schema/qapi-schema-test.json
> index c6d59acc3e..0fadb4ddd7 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -180,7 +180,7 @@
>  { 'event': 'EVENT_D',
>    'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3': 
> 'EnumOne' } }
>  { 'event': 'EVENT_E', 'boxed': true, 'data': 'UserDefZero' }
> -{ 'event': 'EVENT_F', 'boxed': true, 'data': 'UserDefAlternate' }
> +{ 'event': 'EVENT_F', 'boxed': true, 'data': 'UserDefFlatUnion' }
>  
>  # test that we correctly compile downstream extensions, as well as munge
>  # ticklish names
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 9aefcfe015..c54c148263 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -840,7 +840,7 @@ def check_command(expr, info):
>  
>      args_meta = ['struct']
>      if boxed:
> -        args_meta += ['union', 'alternate']
> +        args_meta += ['union']
>      check_type(info, "'data' for command '%s'" % name,
>                 expr.get('data'), allow_dict=not boxed,
>                 allow_metas=args_meta)
> @@ -858,7 +858,7 @@ def check_event(expr, info):
>  
>      meta = ['struct']
>      if boxed:
> -        meta += ['union', 'alternate']
> +        meta += ['union']
>      check_type(info, "'data' for event '%s'" % name,
>                 expr.get('data'), allow_dict=not boxed,
>                 allow_metas=meta)
> @@ -1690,9 +1690,6 @@ class QAPISchemaAlternateType(QAPISchemaType):
>          visitor.visit_alternate_type(self.name, self.info, self.ifcond,
>                                       self.variants)
>  
> -    def is_empty(self):
> -        return False
> -
>  
>  class QAPISchemaCommand(QAPISchemaEntity):
>      def __init__(self, name, info, doc, ifcond, arg_type, ret_type,
> diff --git a/tests/qapi-schema/qapi-schema-test.out 
> b/tests/qapi-schema/qapi-schema-test.out
> index 85d510bc00..5470a525f5 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -252,7 +252,7 @@ event EVENT_D q_obj_EVENT_D-arg
>     boxed=False
>  event EVENT_E UserDefZero
>     boxed=True
> -event EVENT_F UserDefAlternate
> +event EVENT_F UserDefFlatUnion
>     boxed=True
>  enum __org.qemu_x-Enum
>      member __org.qemu_x-value