Re: [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup()

[Philippe Mathieu-Daudé]

On 27/2/23 14:10, Markus Armbruster wrote:
> 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;
>            | ^

While I could find that in the C++ standard, I couldn't in the C one.

I wonder if in that case QAPI should generate foo__MAX = 0. No code /
structure should use an enum type which doesn't contain any member...