Re: [Qemu-devel] [PATCH v3 20/50] qapi-event: add 'if' condition to generated enum

[Marc-André Lureau]

Hi

On Fri, Dec 8, 2017 at 3:07 PM, Markus Armbruster <address@hidden> wrote:
> Markus Armbruster <address@hidden> writes:
>
>> Marc-André Lureau <address@hidden> writes:
>>
>>> Add condition to QAPIEvent enum members based on the event 'if'.
>>>
>>> Signed-off-by: Marc-André Lureau <address@hidden>
>>> ---
>>>  scripts/qapi-event.py | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
>>> index 38f4264817..60c6f7030d 100644
>>> --- a/scripts/qapi-event.py
>>> +++ b/scripts/qapi-event.py
>>> @@ -168,7 +168,7 @@ class QAPISchemaGenEventVisitor(QAPISchemaVisitor):
>>>      def visit_event(self, name, info, ifcond, arg_type, boxed):
>>>          self.decl += gen_event_send_decl(name, arg_type, boxed)
>>>          self.defn += gen_event_send(name, arg_type, boxed)
>>> -        self._event_names.append(QAPISchemaMember(name))
>>> +        self._event_names.append(QAPISchemaMember(name, ifcond))
>>>
>>>
>>>  (input_file, output_dir, do_c, do_h, prefix, dummy) = parse_command_line()
>>
>> No test coverage?

There is no coverage of this change in qapi-schema-test.out since the
event_names enum is an implicit type created by qapi-event.py.

We verify the generated event.h still compiles ;)

Seriously, if necessary we could add a specific test somehow, checking
the generated code further.

>
> Wait!  This patch has no effect, because the it merely puts the ifcond
> argument into QAPISchemaMember.ifcond.  Only later patches put
> QAPISchemaMember.ifcond to use.  Correct?

In general, the patches were splitted this way:
1. accept 'if' in json & verify with visitors
2. add the #if wrapping in the generated code
3. add negative / error tests

In v4, I merged most tests with 1.

> Aside: the ifcond_decorator could already be doing something with the
> argument, but I'll be hanged if I remember how that magic works.
>

It's wrapping visitor methods with a ifcond argument in the previous
patches, not used later.

thanks

-- 
Marc-André Lureau