diff mbox series

[v3,20/50] qapi-event: add 'if' condition to generated enum

Message ID 20170911110623.24981-21-marcandre.lureau@redhat.com
State New
Headers show
Series Hi, | expand

Commit Message

Marc-André Lureau Sept. 11, 2017, 11:05 a.m. UTC
Add condition to QAPIEvent enum members based on the event 'if'.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi-event.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Markus Armbruster Dec. 8, 2017, 1:58 p.m. UTC | #1
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Add condition to QAPIEvent enum members based on the event 'if'.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  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?
Markus Armbruster Dec. 8, 2017, 2:07 p.m. UTC | #2
Markus Armbruster <armbru@redhat.com> writes:

> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
>> Add condition to QAPIEvent enum members based on the event 'if'.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  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?

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?

Aside: the ifcond_decorator could already be doing something with the
argument, but I'll be hanged if I remember how that magic works.
Marc-André Lureau Jan. 11, 2018, 9:31 p.m. UTC | #3
Hi

On Fri, Dec 8, 2017 at 3:07 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Markus Armbruster <armbru@redhat.com> writes:
>
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>
>>> Add condition to QAPIEvent enum members based on the event 'if'.
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>  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
diff mbox series

Patch

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