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

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On Tue, Mar 21, 2023 at 03:19:28PM +0000, Daniel P. Berrangé wrote:
>> On Tue, Mar 21, 2023 at 03:31:56PM +0100, Philippe Mathieu-Daudé wrote:
>> > On 16/3/23 15:57, Markus Armbruster wrote:
>> > > Daniel P. Berrangé <berrange@redhat.com> writes:
>> > > 
>> > > > On Thu, Mar 16, 2023 at 01:31:04PM +0100, Markus Armbruster wrote:
>> > > > > Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>> > > > > 
>> > > > > > Per the C++ standard, empty enum are ill-formed. Do not generate
>> > > 
>> > > The C standard.  The C++ standard doesn't apply here :)
>> > 
>> > I can't find how empty enums are considered by the C standard...

C99 §6.7.2.2 Enumeration specifiers

               enum-specifier:
                       enum identifier-opt { enumerator-list }
                       enum identifier-opt { enumerator-list , }
                       enum identifier

               enumerator-list:
                       enumerator
                       enumerator-list , enumerator

               enumerator:
                       enumeration-constant
                       enumeration-constant = constant-expr

Empty enum is a syntax error.

>> The C standard doesn't really matter either.
>> 
>> What we actually care about is whether GCC and CLang consider the
>> empty enums to be permissible or not. or to put it another way...
>> if it compiles, ship it :-)
>
> But it doesn't compile:
>
> $ cat foo.c
> typedef enum Blah {
> } Blah;
> int main(void) {
>   Blah b = 0;
> }
> $ gcc -o foo -Wall foo.c
> foo.c:2:1: error: empty enum is invalid
>     2 | } Blah;
>       | ^

Gcc opts for an error message more useful than "identifier expected".

> foo.c: In function ‘main’:
> foo.c:4:5: error: unknown type name ‘Blah’; use ‘enum’ keyword to refer to the type
>     4 |     Blah b = 0;
>       |     ^~~~
>       |     enum 
> foo.c:4:10: warning: unused variable ‘b’ [-Wunused-variable]
>     4 |     Blah b = 0;
>       |          ^
>
> So we _do_ need to avoid creating an enum with all members optional in
> the configuration where all options are disabled, if we want that
> configuration to compile.  Or require that all QAPI enums have at
> least one non-optional member.

There is just one way to avoid creating such an enum: make sure the QAPI
enum has members in all configurations we care for.

The issue at hand is whether catching empty enums in qapi-gen already is
practical.  Desirable, because qapi-gen errors are friendlier than C
compiler errors in generated code.

Note "practical": qapi-gen makes an effort to catch errors before the C
compiler catches them.  But catching all of them is impractical.

Having qapi-gen catch empty enums sure is practical for truly empty
enums.  But I doubt an enum without any members is a mistake people make
much.

It isn't for enums with only conditional members: the configuration that
makes them all vanish may or may not actually matter, or even exist, and
qapi-gen can't tell.  The C compiler can tell, but only for the
configuration being compiled.

Requiring at least one non-optional member is a restriction: enums with
only conditional members are now rejected even when the configuration
that makes them all vanish does not actually matter.

Is shielding the user from C compiler errors about empty enums worth the
restriction?