qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_look


From: Markus Armbruster
Subject: Re: [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup()
Date: Mon, 27 Feb 2023 14:10:47 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> QAPI's gen_enum() generates QAPI enum values and the number
> of this values (as foo__MAX).
> The number of entries in an enum type is not part of the
> enumerated values, but we generate it as such. See for
> example:
>
>   typedef enum OnOffAuto {
>       ON_OFF_AUTO_AUTO,
>       ON_OFF_AUTO_ON,
>       ON_OFF_AUTO_OFF,
>       ON_OFF_AUTO__MAX,        <---------
>   } OnOffAuto;
>
> Instead of declaring the enum count as the last enumerated
> value, #define it, so it is not part of the enum. The previous
> example becomes:
>
>   typedef enum OnOffAuto {
>       ON_OFF_AUTO_AUTO,
>       ON_OFF_AUTO_ON,
>       ON_OFF_AUTO_OFF,
>   #define ON_OFF_AUTO__MAX 3   <---------
>   } OnOffAuto;

I'm in favour.  In fact, I've wanted to do this for a while, just never
got around to it.

> Since Clang enables the -Wswitch warning by default [*], remove all
> pointless foo__MAX cases in switch statement, in order to avoid:
>
>  audio/audio.c:2231:10: error: case value not in enumerated type 
> 'AudioFormat' (aka 'enum AudioFormat') [-Wswitch]
>     case AUDIO_FORMAT__MAX:
>          ^
>  ui/input.c:233:14: error: case value not in enumerated type 'KeyValueKind' 
> (aka 'enum KeyValueKind') [-Wswitch]
>         case KEY_VALUE_KIND__MAX:
>              ^
>  ...
>
> [*] https://clang.llvm.org/docs/DiagnosticsReference.html#wswitch
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Falls apart when the enum is empty:

    gcc -m64 -mcx16 -Iqga/qemu-ga.p -Iqga -I../qga -I. -Iqapi -Itrace -Iui 
-Iui/shader -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include 
-I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch 
-std=gnu11 -O0 -g -isystem /work/armbru/qemu/linux-headers -isystem 
linux-headers -iquote . -iquote /work/armbru/qemu -iquote 
/work/armbru/qemu/include -iquote /work/armbru/qemu/tcg/i386 -pthread 
-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing 
-fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes 
-Wstrict-prototypes -Wredundant-decls -Wold-style-declaration 
-Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels 
-Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute 
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi 
-fstack-protector-strong -fPIE -MD -MQ 
qga/qemu-ga.p/meson-generated_.._qga-qapi-emit-events.c.o -MF 
qga/qemu-ga.p/meson-generated_.._qga-qapi-emit-events.c.o.d -o 
qga/qemu-ga.p/meson-generated_.._qga-qapi-emit-events.c.o -c 
qga/qga-qapi-emit-events.c
    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;
          | ^

qga/qapi-schema.json does not define any events, so its (implicitly
defined) event enumeration comes out empty.

We could detect this, and either emit a dummy event, or suppress code
generation for events entirely.

Explicitly defined enumerations may also be empty.  qapi-code-gen.rst
says:

    Nothing prevents an empty enumeration, although it is probably not
    useful.

We could forbid empty enumerations.

Speaking of qapi-code-gen.rst: it also needs an update.  Search for
__MAX.




reply via email to

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