[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/3] qapi: Do not generate empty enum
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v3 2/3] qapi: Do not generate empty enum |
Date: |
Thu, 16 Mar 2023 14:42:35 +0000 |
User-agent: |
Mutt/2.2.9 (2022-11-12) |
On Thu, Mar 16, 2023 at 03:39:59PM +0100, Juan Quintela wrote:
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> > On Thu, Mar 16, 2023 at 01:31:04PM +0100, Markus Armbruster wrote:
> >> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> >>
> >> > Per the C++ standard, empty enum are ill-formed. Do not generate
> >> > them in order to avoid:
> >> >
> >> > In file included from qga/qga-qapi-emit-events.c:14:
> >> > qga/qga-qapi-emit-events.h:20:1: error: empty enum is invalid
> >> > 20 | } qga_QAPIEvent;
> >> > | ^
> >> >
> >> > Reported-by: Markus Armbruster <armbru@redhat.com>
> >> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >>
> >> Two failures in "make check-qapi-schema" (which is run by "make check"):
> >>
> >> 1. Positive test case qapi-schema-test
> >>
> >> --- /work/armbru/qemu/bld-x86/../tests/qapi-schema/qapi-schema-test.out
> >> +++
> >> @@ -19,7 +19,6 @@
> >> member enum2: EnumOne optional=True
> >> member enum3: EnumOne optional=False
> >> member enum4: EnumOne optional=True
> >> -enum MyEnum
> >> object Empty1
> >> object Empty2
> >> base Empty1
> >>
> >> You forgot to update expected test output. No big deal.
> >>
> >> 2. Negative test case union-empty
> >>
> >> --- /work/armbru/qemu/bld-x86/../tests/qapi-schema/union-empty.err
> >> +++
> >> @@ -1,2 +1,2 @@
> >> -union-empty.json: In union 'Union':
> >> -union-empty.json:4: union has no branches
> >> +union-empty.json: In struct 'Base':
> >> +union-empty.json:3: member 'type' uses unknown type 'Empty'
> >> stderr:
> >> qapi-schema-test FAIL
> >> union-empty FAIL
> >>
> >> The error message regresses.
> >>
> >> I can see two ways to fix this:
> >>
> >> (A) You can't just drop empty enumeration types on the floor. To not
> >> generate code for them, you need to skip them wherever we
> >> generate code for enumeration types.
> >>
> >> (B) Outlaw empty enumeration types.
> >>
> >> I recommend to give (B) a try, it's likely simpler.
> >
> > Possible trap-door with (B), if we have any enums where *every*
> > member is conditionalized on a CONFIG_XXX rule, there might be
> > certain build scenarios where an enum suddenly becomes empty.
>
> Do we have an example for this?
> Because it looks really weird. I would expect that the "container" unit
> of that enumeration is #ifdef out of compilation somehow.
I'm not sure if such an example physically exists. I know the audio
code gets close, with all but 2 options conditional:
{ 'enum': 'AudiodevDriver',
'data': [ 'none',
{ 'name': 'alsa', 'if': 'CONFIG_AUDIO_ALSA' },
{ 'name': 'coreaudio', 'if': 'CONFIG_AUDIO_COREAUDIO' },
{ 'name': 'dbus', 'if': 'CONFIG_DBUS_DISPLAY' },
{ 'name': 'dsound', 'if': 'CONFIG_AUDIO_DSOUND' },
{ 'name': 'jack', 'if': 'CONFIG_AUDIO_JACK' },
{ 'name': 'oss', 'if': 'CONFIG_AUDIO_OSS' },
{ 'name': 'pa', 'if': 'CONFIG_AUDIO_PA' },
{ 'name': 'sdl', 'if': 'CONFIG_AUDIO_SDL' },
{ 'name': 'sndio', 'if': 'CONFIG_AUDIO_SNDIO' },
{ 'name': 'spice', 'if': 'CONFIG_SPICE' },
'wav' ] }
Just wanted to warn that we shouldn't assume empty enums can't
exist, because it would be quite easy to add 2 extra conditionals
to this audio example, and the enum wouldn't appear empty at a
glance, but none the less could be empty in some compile scenarios
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- [PATCH v3 0/3] qapi: Simplify enum generation, Philippe Mathieu-Daudé, 2023/03/15
- [PATCH v3 1/3] scripts/git.orderfile: Display QAPI script changes before schema ones, Philippe Mathieu-Daudé, 2023/03/15
- [PATCH v3 2/3] qapi: Do not generate empty enum, Philippe Mathieu-Daudé, 2023/03/15
- Re: [PATCH v3 2/3] qapi: Do not generate empty enum, Richard Henderson, 2023/03/15
- Re: [PATCH v3 2/3] qapi: Do not generate empty enum, Markus Armbruster, 2023/03/16
- Re: [PATCH v3 2/3] qapi: Do not generate empty enum, Markus Armbruster, 2023/03/16
- Re: [PATCH v3 2/3] qapi: Do not generate empty enum, Philippe Mathieu-Daudé, 2023/03/21
- Re: [PATCH v3 2/3] qapi: Do not generate empty enum, Daniel P . Berrangé, 2023/03/21
- Re: [PATCH v3 2/3] qapi: Do not generate empty enum, Eric Blake, 2023/03/21
- Re: [PATCH v3 2/3] qapi: Do not generate empty enum, Markus Armbruster, 2023/03/22
- Re: [PATCH v3 2/3] qapi: Do not generate empty enum, Philippe Mathieu-Daudé, 2023/03/21
- Re: [PATCH v3 2/3] qapi: Do not generate empty enum, Markus Armbruster, 2023/03/21
[PATCH v3 3/3] qapi: Generate enum count as definition, Philippe Mathieu-Daudé, 2023/03/15