Re: [Qemu-devel] [PATCH v12 11/36] qapi: Check for qapi collisions involving variant members

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> Right now, our ad hoc parser ensures that we cannot have a
> flat union that introduces any members that would clash with
> non-variant members inherited from the union's base type (see
> flat-union-clash-member.json).  We want ObjectType.check() to

Recommend not to abbreviate QAPISchemaObjectType.check().

> make the same check, so we can later reduce some of the ad
> hoc checks.
>
> We already have a map 'seen' of all non-variant members. We
> still need to check for collisions between each variant type's
> members and the non-variant ones.
>
> To know the variant type's members, we need to call
> variant.type.check().  This also detects when a type contains
> itself in a variant, exactly like the existing base.check()
> detects when a type contains itself as a base.  (Except that
> we currently forbid anything but a struct as the type of a
> variant, so we can't actually trigger this type of loop yet.)
>
> Slight complication: an alternate's variant can have arbitrary
> type, but only an object type's check() may be called outside
> QAPISchema.check(). We could either skip the call for variants
> of alternates, or skip it for non-object types.  For now, do
> the latter, because it's easier.
>
> Then we call each variant member's check_clash() with the
> appropriate 'seen' map.  Since members of different variants
> can't clash, we have to clone a fresh seen for each variant.
> Wrap this in a new helper method Variants.check_clash().

Likewise: QAPISchemaObjectTypeVariants.check_clash().

> Note that cloning 'seen' inside Variants.check_clash() resembles
> the one we just removed from Variants.check() in 'qapi: Drop

Here, we can say .check_clash() and .check(), and leave the class to
context.

> obsolete tag value collision assertions'; the difference here is
> that we are now checking for clashes among the qapi members of
> the variant type, rather than for a single clash with the variant
> tag name itself.
>
> Note that, by construction, collisions can't actually happen for
> simple unions: each variant's type is a wrapper with a single
> member 'data', which will never collide with the only non-variant
> member 'type'.
>
> For alternates, there's nothing for a variant object type's
> members to clash with, and therefore no need to call the new
> variants.check_clash().
>
> No change to generated code.
>
> Signed-off-by: Eric Blake <address@hidden>

Only commit message polishing, happy to do that in my tree.