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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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