[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 23/30] qapi: Check for qapi collisions of fl
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v10 23/30] qapi: Check for qapi collisions of flat union branches |
Date: |
Mon, 09 Nov 2015 13:56:24 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> Right now, our ad hoc parser ensures that we cannot have a
> flat union that introduces any qapi member names that would
> conflict with the non-variant qapi members already present
> from the union's base type (see flat-union-clash-member.json).
> We want QAPISchemaObjectType.check() to make the same check,
> so we can later reduce some of the ad hoc checks.
>
> We already ensure that all branches of a flat union are qapi
> structs with no variants, at which point those members appear
> in the same JSON object as all non-variant members. And we
> already have a map 'seen' of all non-variant members. All
> we need is a new QAPISchemaObjectTypeVariants.check_clash(),
> which clones the seen map then checks for clashes with each
> member of the variant's qapi type.
>
> Note that the clone of seen inside Variants.check_clash()
> resembles the one we just removed from Variants.check(); 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.
>
> In general, a type used as a branch of a flat union cannot
> also be the base type of the flat union, so even though we are
> adding a call to variant.type.check() in order to populate
> variant.type.members, this is merely a case of gaining
> topological sorting of how types are visited (and type.check()
> is already set up to allow multiple calls due to base types).
Yes, a type cannot contain itself, neither as base nor as variant.
We have tests covering attempts to do the former
(struct-cycle-direct.json, struct-cycle-indirect.json). As far as I can
see, we don't have tests covering the latter. Do we catch it?
> For simple unions, the same code happens to work by design,
> because of our synthesized wrapper classes (however, the
> wrapper has a single member 'data' which will never collide
> with the one non-variant member 'type', so it doesn't really
> matter).
>
> There is no impact to alternates, which intentionally do not
> need to call variants.check_clash() (there, at most one of
> the variant's branches will be an ObjectType, and even if one
> exists, we are not inlining the qapi members of that object
> into a parent object, the way we do for unions).
>
> No change to generated code.
>
> Signed-off-by: Eric Blake <address@hidden>
Patch looks good.
- Re: [Qemu-devel] [PATCH v10 24/30] qapi: Factor out QAPISchemaObjectType.check_clash(), (continued)
Re: [Qemu-devel] [PATCH v10 24/30] qapi: Factor out QAPISchemaObjectType.check_clash(), Markus Armbruster, 2015/11/09
[Qemu-devel] [PATCH v10 19/30] qapi: Fix up commit 7618b91's clash sanity checking change, Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 18/30] qapi: Clean up after previous commit, Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 23/30] qapi: Check for qapi collisions of flat union branches, Eric Blake, 2015/11/06
Re: [Qemu-devel] [PATCH v10 23/30] qapi: Check for qapi collisions of flat union branches, Eric Blake, 2015/11/10
Re: [Qemu-devel] [PATCH v10 23/30] qapi: Check for qapi collisions of flat union branches, Markus Armbruster, 2015/11/11
[Qemu-devel] [PATCH v10 27/30] qapi: Track owner of each object member, Eric Blake, 2015/11/06