Re: [PATCH v3 2/3] qapi: Do not generate empty enum

[Philippe Mathieu-Daudé]

On 16/3/23 15:57, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:


> But all of this falls apart with conditional members!
>
> Example 1 (taken from qapi/block-core.json):
>
>      { 'enum': 'BlockdevAioOptions',
>        'data': [ 'threads', 'native',
>                  { 'name': 'io_uring', 'if': 'CONFIG_LINUX_IO_URING' } ] }
>
> Generates now:
>
>      typedef enum BlockdevAioOptions {
>          BLOCKDEV_AIO_OPTIONS_THREADS,
>          BLOCKDEV_AIO_OPTIONS_NATIVE,
>      #if defined(CONFIG_LINUX_IO_URING)
>          BLOCKDEV_AIO_OPTIONS_IO_URING,
>      #endif /* defined(CONFIG_LINUX_IO_URING) */
>          BLOCKDEV_AIO_OPTIONS__MAX,
>      } BlockdevAioOptions;
>
> BLOCKDEV_AIO_OPTIONS__MAX is 3 if defined(CONFIG_LINUX_IO_URING), else
> 2.
>
> After the next patch:
>
>      typedef enum BlockdevAioOptions {
>          BLOCKDEV_AIO_OPTIONS_THREADS,
>          BLOCKDEV_AIO_OPTIONS_NATIVE,
>      #if defined(CONFIG_LINUX_IO_URING)
>          BLOCKDEV_AIO_OPTIONS_IO_URING,
>      #endif /* defined(CONFIG_LINUX_IO_URING) */
>      #define BLOCKDEV_AIO_OPTIONS__MAX 3
>      } BlockdevAioOptions;
>
> Now it's always 3.
>
> Example 2 (same with members reordered):
>
>      { 'enum': 'BlockdevAioOptions',
>        'data': [ { 'name': 'io_uring', 'if': 'CONFIG_LINUX_IO_URING' },
>                  'threads', 'native' ] }
>
> Same problem for __MAX, additional problem for __DUMMY:
>
>      typedef enum BlockdevAioOptions {
>          BLOCKDEV_AIO_OPTIONS__DUMMY = 0,
>      #if defined(CONFIG_LINUX_IO_URING)
>          BLOCKDEV_AIO_OPTIONS_IO_URING = 0,
>      #endif /* defined(CONFIG_LINUX_IO_URING) */
>          BLOCKDEV_AIO_OPTIONS_THREADS,
>          BLOCKDEV_AIO_OPTIONS_NATIVE,
>      #define BLOCKDEV_AIO_OPTIONS__MAX 3
>      } BlockdevAioOptions;
>
> If CONFIG_LINUX_IO_URING is off, the enum starts at 1 instead of 0.
>
> Arrays indexed by the enum start with a hole.  Code using them is
> probably not prepared for holes.

Can we meet half-way only generating the MAX definitions for
unconditional enums, keeping the conditional ones as is?

-- >8 --
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
@@ -88,16 +88,18 @@ def gen_enum(name: str,
              members: List[QAPISchemaEnumMember],
              prefix: Optional[str] = None) -> str:
     assert members
-    # append automatically generated _MAX value
-    enum_members = members + [QAPISchemaEnumMember('_MAX', None)]
-
     ret = mcgen('''

 typedef enum %(c_name)s {
 ''',
                 c_name=c_name(name))

-    for memb in enum_members:
+    has_cond = any(memb.ifcond.is_present() for memb in members)
+    if has_cond:
+        # append automatically generated _MAX value
+        members += [QAPISchemaEnumMember('_MAX', None)]
+
+    for memb in members:
         ret += memb.ifcond.gen_if()
         ret += mcgen('''
     %(c_enum)s,
@@ -105,6 +107,13 @@ def gen_enum(name: str,
                      c_enum=c_enum_const(name, memb.name, prefix))
         ret += memb.ifcond.gen_endif()

+    if not has_cond:
+        ret += mcgen('''
+#define %(c_name)s %(c_length)s
+''',
+                     c_name=c_enum_const(name, '_MAX', prefix),
+                     c_length=len(members))
+
     ret += mcgen('''
 } %(c_name)s;
 ''',
---