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

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

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;
  ''',
---