All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: qemu-devel@nongnu.org,
	 Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>,
	Paolo Bonzini <pbonzini@redhat.com>,
	 Michael Roth <michael.roth@amd.com>,
	 "Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Stefan Berger <stefanb@linux.vnet.ibm.com>,
	 Gerd Hoffmann <kraxel@redhat.com>,
	 Juan Quintela <quintela@redhat.com>
Subject: Re: [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup()
Date: Mon, 27 Feb 2023 14:10:47 +0100	[thread overview]
Message-ID: <87ilfnqmnc.fsf@pond.sub.org> (raw)
In-Reply-To: <20230224155451.20211-3-philmd@linaro.org> ("Philippe Mathieu-Daudé"'s message of "Fri, 24 Feb 2023 16:54:51 +0100")

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.



  parent reply	other threads:[~2023-02-27 13:11 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-24 15:54 [PATCH 0/2] qapi: Simplify enum generation Philippe Mathieu-Daudé
2023-02-24 15:54 ` [PATCH 1/2] qapi: Do not generate default switch case in gen_visit_object_members() Philippe Mathieu-Daudé
2023-02-27 12:40   ` Juan Quintela
2023-02-27 13:14   ` Markus Armbruster
2023-02-24 15:54 ` [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup() Philippe Mathieu-Daudé
2023-02-24 21:16   ` Richard Henderson
2023-02-27 12:41   ` Juan Quintela
2023-02-27 13:10   ` Markus Armbruster [this message]
2023-03-15 10:32     ` Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ilfnqmnc.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=pavel.dovgaluk@ispras.ru \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanb@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.