Re: [Qemu-devel] [PATCH v6 21/27] qapi: add #if conditions to generated code members

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Marc-André Lureau <address@hidden> writes:
>
>> Hi
>> On Thu, Dec 6, 2018 at 9:42 PM Markus Armbruster <address@hidden> wrote:
>>>
>>> Marc-André Lureau <address@hidden> writes:
>>>
>>> > Wrap generated enum/struct members and code with #if/#endif, using the
>>>
>>> enum and struct members
>>
>> ok
>>
>>>
>>> > .ifcond members added in the previous patches.
>>> >
>>> > Some types generate both enum and struct members for example, so a
>>> > step-by-step is unnecessarily complicated to deal with (it would
>>> > easily generate invalid intermediary code).
>>>
>>> Can you give an example of a schema definition that would lead to
>>> complications?
>>>
>>
>> Honestly, I don't remember well (it's been a while I wrote that code).
>
> I know...
>
>> It must be related to implicit enums, such as union kind... If there
>> is no strong need to split this patch, I would rather not do that
>> extra work.
>
> I'm not looking for reasons to split this patch, I'm looking for
> stronger reasons to keep it just like it is :)
>
> Your hunch that complications would arise for simple unions plausible:
> there the same conditional needs to be applied both to the C enum's
> member and the C union member.
>
> For the generated C code to compile, each union tag enum member
> conditional must imply the corresponding variant conditional.
>
> For flat unions, the two are separate.  The QAPI generator makes no
> effort to check the enum member's if condition implies the union
> variant's if condition; if you mess them up in the schema, you get to
> deal with the C compilation errors.
>
> For simple unions, the two are one.
>
> If we separate the generator updates for enums and for union members,
> and do enum members first, then unions with conditional tag members
> can't compile.  Corrollary: simple unions with conditional variants
> can't compile.
>
> What if we do union members first?
>
> Again, I'm not asking for patch splitting here, I'm just trying to
> arrive at a clearer understanding to avoid making insufficiently
> supported claims in the commit message.  The combined patch looks small
> and clean enough to keep it combined.
>
> [...]

What about this commit message:

    qapi: Add #if conditions to generated code members

    Wrap generated enum and struct members and their supporting code with
    #if/#endif, using the .ifcond members added in the previous patches.

    We do enum and struct in a single patch because union tag enum and the
    associated variants tie them together, and dealing with that to split
    the patch doesn't seem worthwhile.