Re: [Qemu-devel] [PATCH v2 15/54] qapi: add 'if' to top-level expressions

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> Hi
>
> On Mon, Sep 4, 2017 at 3:27 PM, Markus Armbruster <address@hidden> wrote:
>> Marc-André Lureau <address@hidden> writes:
>>
>>> Accept 'if' key in top-level elements, accepted as string or list of
>>> string type. The following patches will modify the test visitor to
>>> check the value is correctly saved, and generate #if/#endif code (as a
>>> single #if/endif line or a series for a list).
>>>
>>> Example of 'if' key:
>>> { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },                       
>>>                                                                             
>>>                                                             'if': 
>>> 'defined(TEST_IF_STRUCT)' }
>>
>> Lost line break?
>
> yes
>
>>
>>> A following patch for qapi-code-gen.txt will provide more complete
>>> documentation for 'if' usage.
>>>
>>> Signed-off-by: Marc-André Lureau <address@hidden>
[...]
>>> diff --git a/tests/qapi-schema/qapi-schema-test.out 
>>> b/tests/qapi-schema/qapi-schema-test.out
>>> index 3b1e9082d3..7fbaea19bc 100644
>>> --- a/tests/qapi-schema/qapi-schema-test.out
>>> +++ b/tests/qapi-schema/qapi-schema-test.out
>>> @@ -52,6 +52,22 @@ enum QEnumTwo ['value1', 'value2']
>>>      prefix QENUM_TWO
>>>  enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
>>>      prefix QTYPE
>>> +alternate TestIfAlternate
>>> +    tag type
>>> +    case foo: int
>>> +    case bar: TestStruct
>>> +command TestIfCmd q_obj_TestIfCmd-arg -> None
>>> +   gen=True success_response=True boxed=False
>>> +enum TestIfEnum ['foo', 'bar']
>>> +event TestIfEvent q_obj_TestIfEvent-arg
>>> +   boxed=False
>>> +object TestIfStruct
>>> +    member foo: int optional=False
>>> +object TestIfUnion
>>> +    member type: TestIfUnionKind optional=False
>>> +    tag type
>>> +    case foo: q_obj_TestStruct-wrapper
>>> +enum TestIfUnionKind ['foo']
>>>  object TestStruct
>>>      member integer: int optional=False
>>>      member boolean: bool optional=False
>>> @@ -172,6 +188,12 @@ object q_obj_EVENT_D-arg
>>>      member b: str optional=False
>>>      member c: str optional=True
>>>      member enum3: EnumOne optional=True
>>> +object q_obj_TestIfCmd-arg
>>> +    member foo: TestIfStruct optional=False
>>> +object q_obj_TestIfEvent-arg
>>> +    member foo: TestIfStruct optional=False
>>> +object q_obj_TestStruct-wrapper
>>> +    member data: TestStruct optional=False
>>>  object q_obj_UserDefFlatUnion2-base
>>>      member integer: int optional=True
>>>      member string: str optional=False
>>
>> The conditionals aren't visible in qapi-schema-test.out.  They should
>> be.
>>
>
> That's a follow-up patch "qapi: add 'ifcond' to visitor methods"

Which I've since found %-}

Similarly, the generated code is for now *unconditional*.  Later patches
generate the conditionals.

Let's point this out in the commit message.

Remember, your poor, ignorant reviewer squints at your patches through a
toiler paper tube, and may need your help to see the bigger picture.

>> *Much* easier to review than its predecessor PATCH 07/26.  Appreciated!