[v2] qapi: Simplify enum generation

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

Posted by Philippe Mathieu-Daudé 2 years, 11 months ago

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

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

Posted by Stefan Berger 2 years, 11 months ago


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)

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

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

Posted by Philippe Mathieu-Daudé 2 years, 11 months ago

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!