[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.