qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v11 13/28] qapi: Hoist tag collision check to Va


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v11 13/28] qapi: Hoist tag collision check to Variants.check()
Date: Wed, 11 Nov 2015 14:56:36 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Eric Blake <address@hidden> writes:

> Checking that a given QAPISchemaObjectTypeVariant.name is a
> member of the corresponding QAPISchemaEnumType of the owning
> QAPISchemaObjectTypeVariants.tag_member ensures that there are
> no collisions in the generated C union for those tag values
> (since the enum itself should have no collisions).
>
> However, this check was the only thing that Variant.check() was
> doing beyond the work of the superclass ObjectTypeMember.check(),

Since PATCH 05, actually.  Suggest to mention that.

> and resulted in a difference of the .check() signatures just to
> pass the enum type down.
>
> Simplify things by instead doing the tag name check as part of
> Variants.check(), at which point we can rely on inheritance
> instead of overriding Variant.check().
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v11: don't use tag_type local variable, rebase to v.type.check()
> v10: new patch
> ---
>  scripts/qapi.py | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 296b9bb..c6f3fce 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1059,7 +1059,8 @@ class QAPISchemaObjectTypeVariants(object):
>              self.tag_member = seen[self.tag_name]
>          assert isinstance(self.tag_member.type, QAPISchemaEnumType)
>          for v in self.variants:
> -            v.check(schema, self.tag_member.type)
> +            v.check(schema)
> +            assert v.name in self.tag_member.type.values
>              if isinstance(v.type, QAPISchemaObjectType):
>                  v.type.check(schema)
>
> @@ -1075,10 +1076,6 @@ class 
> QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
>      def __init__(self, name, typ):
>          QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
>
> -    def check(self, schema, tag_type):
> -        QAPISchemaObjectTypeMember.check(self, schema)
> -        assert self.name in tag_type.values
> -
>      # This function exists to support ugly simple union special cases
>      # TODO get rid of them, and drop the function
>      def simple_union_type(self):

QAPISchemaObjectTypeVariant is now an almost trivial variation of
QAPISchemaObjectTypeMember.  Differences:

* __init__() has no parameter optional

* Method simple_union_type(), which exists only to support pointless
  differences in code generation for simple unions, all marked TODO.

There's hope we can get rid of the class after the TODOs are resolved.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]