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

[Philippe Mathieu-Daudé]

On 15/3/23 16:19, Stefan Berger wrote:
> On 3/15/23 07:13, Philippe Mathieu-Daudé wrote:
> > Per the C++ standard, empty enum are ill-formed. Do not generate
> > them in order to avoid:
> >
> >    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;
> >          | ^
> >
> > Reported-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   docs/devel/qapi-code-gen.rst | 6 +++---
> >   scripts/qapi/schema.py       | 5 ++++-
> >   2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
> > index 23e7f2fb1c..d684c7c24d 100644
> > --- a/docs/devel/qapi-code-gen.rst
> > +++ b/docs/devel/qapi-code-gen.rst
> > @@ -206,6 +206,9 @@ Syntax::
> >
> >   Member 'enum' names the enum type.
> >
> > +Empty enumeration (no member) does not generate anything (not even
> > +constant PREFIX__MAX).
> > +
> >   Each member of the 'data' array defines a value of the enumeration
> >   type.  The form STRING is shorthand for :code:`{ 'name': STRING }`.  
> > The
> >   'name' values must be be distinct.
> > @@ -214,9 +217,6 @@ Example::
> >
> >    { 'enum': 'MyEnum', 'data': [ 'value1', 'value2', 'value3' ] }
> >
> > -Nothing prevents an empty enumeration, although it is probably not
> > -useful.
> > -
> >   On the wire, an enumeration type's value is represented by its
> >   (string) name.  In C, it's represented by an enumeration constant.
> >   These are of the form PREFIX_NAME, where PREFIX is derived from the
> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> > index 207e4d71f3..28045dbe93 100644
> > --- a/scripts/qapi/schema.py
> > +++ b/scripts/qapi/schema.py
> > @@ -309,6 +309,7 @@ class QAPISchemaEnumType(QAPISchemaType):
> >
> >       def __init__(self, name, info, doc, ifcond, features, members, 
> > prefix):
> >           super().__init__(name, info, doc, ifcond, features)
> > +        assert members
>
> not: assert isinstance(members, list)

This doesn't work as [] is a list instance (we want to check the
members[] array contains elements). More verbose could be:

            assert len(members) > 0

> >           for m in members:
> >               assert isinstance(m, QAPISchemaEnumMember)
> >               m.set_defined_in(name)
> > @@ -1047,8 +1048,10 @@ def _make_implicit_object_type(self, name, 
> > info, ifcond, role, members):
> >           return name
> >
> >       def _def_enum_type(self, expr: QAPIExpression):
> > -        name = expr['enum']
> >           data = expr['data']
> > +        if not data:
> > +            return
> > +        name = expr['enum']
> >           prefix = expr.get('prefix')
> >           ifcond = QAPISchemaIfCond(expr.get('if'))
> >           info = expr.info
>
>
> Acked-by: Stefan Berger <stefanb@linux.ibm.com>

Thanks!