[v6,04/12] qapi: Create simple union type member earlier

For simple unions, we were creating the implicit 'type' tag
member during the QAPISchemaObjectTypeVariants constructor.
This is different from every other implicit QAPISchemaEntity
object, which get created by QAPISchema methods.  Hoist the
creation to the caller, and pass the entity rather than the
string name, so that we have the nice property that no
entities are created as a side effect within a different
entity.  A later patch will then have an easier time of
associating location info with each entity creation.

No change to generated code.

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

---
v6: New patch
---
 scripts/qapi.py | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

Eric Blake <eblake@redhat.com> writes:

> For simple unions, we were creating the implicit 'type' tag
> member during the QAPISchemaObjectTypeVariants constructor.
> This is different from every other implicit QAPISchemaEntity
> object, which get created by QAPISchema methods.  Hoist the
> creation to the caller, and pass the entity rather than the
> string name, so that we have the nice property that no
> entities are created as a side effect within a different
> entity.  A later patch will then have an easier time of
> associating location info with each entity creation.
>
> No change to generated code.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v6: New patch
> ---
>  scripts/qapi.py | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 15640b6..255001a 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1010,18 +1010,16 @@ class QAPISchemaObjectTypeMember(object):
>
>
>  class QAPISchemaObjectTypeVariants(object):
> -    def __init__(self, tag_name, tag_enum, variants):
> +    def __init__(self, tag_name, tag_member, variants):
>          assert tag_name is None or isinstance(tag_name, str)
> -        assert tag_enum is None or isinstance(tag_enum, str)
> +        assert (tag_member is None or
> +                isinstance(tag_member, QAPISchemaObjectTypeMember))
>          for v in variants:
>              assert isinstance(v, QAPISchemaObjectTypeVariant)
>          self.tag_name = tag_name
>          if tag_name:
> -            assert not tag_enum
> -            self.tag_member = None
> -        else:
> -            self.tag_member = QAPISchemaObjectTypeMember('type', tag_enum,
> -                                                         False)
> +            assert tag_member is None

Since the conditional degenerates to just checking the "either tag_name
of tag_member" precondition, let's move it to right after the checking
of tag_name and tag_member, i.e. before the loop.

I'd also simplify to

        assert not tag_name != not tag_member

or

        bool(tag_name) != bool(tag_member)

> +        self.tag_member = tag_member
>          self.variants = variants

Before:

    __init__() preconditions involving the tag:
        if flat union:
            tag_name is the name of the tag member
            tag_enum is None
        else simple union or alternate:
            tag_name is None
            tag_enum is the name of the implicitly defined enumeration type

    __init__() postconditions:
        if flat union:
            self.tag_name is the name of the tag member
            self.tag_member is None
        else simple union or alternate:
            self.tag_name is None
            self.tag_member is the implicitly defined tag member

    check() postconditions:
        if flat union:
            self.tag_name is the name of the tag member
        else simple union or alternate:
            self.tag_name is None
        self.tag_member is the tag member

After:

    __init__() preconditions involving the tag:
        if flat union:
            tag_name is the name of the tag member
            tag_member is None
        else simple union or alternate:
            tag_name is None
            tag_member is the implicitly defined tag member

    __init__() postconditions:
        exactly as before

    check() postconditions:
        exactly as before

Okay.  Slightly simpler even.

>
>      def check(self, schema, members, seen):
> @@ -1226,8 +1224,9 @@ class QAPISchema(object):
>          return QAPISchemaObjectTypeVariant(case, typ)
>
>      def _make_tag_enum(self, type_name, variants):
> -        return self._make_implicit_enum_type(type_name,
> -                                             [v.name for v in variants])
> +        typ = self._make_implicit_enum_type(type_name,
> +                                            [v.name for v in variants])
> +        return QAPISchemaObjectTypeMember('type', typ, False)

I think this function should now be called _make_tag_member(), or
_make_implicit_tag_member(), or _make_implicit_tag().

>
>      def _def_union_type(self, expr, info):
>          name = expr['union']

On 10/02/2015 02:34 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> For simple unions, we were creating the implicit 'type' tag
>> member during the QAPISchemaObjectTypeVariants constructor.
>> This is different from every other implicit QAPISchemaEntity
>> object, which get created by QAPISchema methods.  Hoist the
>> creation to the caller, and pass the entity rather than the
>> string name, so that we have the nice property that no
>> entities are created as a side effect within a different
>> entity.  A later patch will then have an easier time of
>> associating location info with each entity creation.
>>
>> No change to generated code.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>

>>  class QAPISchemaObjectTypeVariants(object):
>> -    def __init__(self, tag_name, tag_enum, variants):
>> +    def __init__(self, tag_name, tag_member, variants):
>>          assert tag_name is None or isinstance(tag_name, str)
>> -        assert tag_enum is None or isinstance(tag_enum, str)
>> +        assert (tag_member is None or
>> +                isinstance(tag_member, QAPISchemaObjectTypeMember))
>>          for v in variants:
>>              assert isinstance(v, QAPISchemaObjectTypeVariant)
>>          self.tag_name = tag_name
>>          if tag_name:
>> -            assert not tag_enum
>> -            self.tag_member = None
>> -        else:
>> -            self.tag_member = QAPISchemaObjectTypeMember('type', tag_enum,
>> -                                                         False)
>> +            assert tag_member is None
> 
> Since the conditional degenerates to just checking the "either tag_name
> of tag_member" precondition, let's move it to right after the checking
> of tag_name and tag_member, i.e. before the loop.
> 
> I'd also simplify to
> 
>         assert not tag_name != not tag_member
> 
> or
> 
>         bool(tag_name) != bool(tag_member)

Sure, that works.  I'll also add comments on the preconditions.

>> @@ -1226,8 +1224,9 @@ class QAPISchema(object):
>>          return QAPISchemaObjectTypeVariant(case, typ)
>>
>>      def _make_tag_enum(self, type_name, variants):
>> -        return self._make_implicit_enum_type(type_name,
>> -                                             [v.name for v in variants])
>> +        typ = self._make_implicit_enum_type(type_name,
>> +                                            [v.name for v in variants])
>> +        return QAPISchemaObjectTypeMember('type', typ, False)
> 
> I think this function should now be called _make_tag_member(), or
> _make_implicit_tag_member(), or _make_implicit_tag().
> 

Makes sense.  I'll probably use _make_implicit_tag().