[v3] qapi: add #if pre-processor conditions to generated code (part 3)

[Qemu-devel] [PATCH v3 15/17] Revert "qapi-events: add 'if' condition to implicit event enum"

Posted by Markus Armbruster 6 years, 9 months ago

This reverts commit 7bd263490590ee6fcf34ecb6203437e22f6e5a9c.

The commit applied the events' conditions to the members of enum
QAPIEvent.  Awkward, because it renders QAPIEvent unusable in
target-independent code as soon as we make an event target-dependent.
Reverting this has the following effects:

* ui/vnc.c can remain target independent.

* monitor_qapi_event_conf[] doesn't have to muck around with #ifdef.

* query-events again doesn't reflect conditionals.  I'm going to
  deprecate it in favor of query-qmp-schema.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/events.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 28bbc3745d..2067660be4 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -193,7 +193,9 @@ void %(event_emit)s(%(event_enum)s event, QDict *qdict);
             self._genc.add(gen_event_send(name, arg_type, boxed,
                                           self._event_enum_name,
                                           self._event_emit_name))
-        self._event_enum_members.append(QAPISchemaMember(name, ifcond))
+        # Note: we generate the enum member regardless of @ifcond, to
+        # keep the enumeration usable in target-independent code.
+        self._event_enum_members.append(QAPISchemaMember(name))
 
 
 def gen_events(schema, output_dir, prefix):
-- 
2.17.2

Re: [Qemu-devel] [PATCH v3 15/17] Revert "qapi-events: add 'if' condition to implicit event enum"

Posted by Marc-André Lureau 6 years, 9 months ago

Hi

On Wed, Feb 6, 2019 at 7:17 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> This reverts commit 7bd263490590ee6fcf34ecb6203437e22f6e5a9c.
>
> The commit applied the events' conditions to the members of enum
> QAPIEvent.  Awkward, because it renders QAPIEvent unusable in
> target-independent code as soon as we make an event target-dependent.

Well, my solution didn't have this problem iirc. You could mention
that by having target-specific event enums, we could solve this.
It might be worth a FIXME too.

> Reverting this has the following effects:
>
> * ui/vnc.c can remain target independent.
>
> * monitor_qapi_event_conf[] doesn't have to muck around with #ifdef.
>
> * query-events again doesn't reflect conditionals.  I'm going to
>   deprecate it in favor of query-qmp-schema.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

But that's ok for now:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  scripts/qapi/events.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
> index 28bbc3745d..2067660be4 100644
> --- a/scripts/qapi/events.py
> +++ b/scripts/qapi/events.py
> @@ -193,7 +193,9 @@ void %(event_emit)s(%(event_enum)s event, QDict *qdict);
>              self._genc.add(gen_event_send(name, arg_type, boxed,
>                                            self._event_enum_name,
>                                            self._event_emit_name))
> -        self._event_enum_members.append(QAPISchemaMember(name, ifcond))
> +        # Note: we generate the enum member regardless of @ifcond, to
> +        # keep the enumeration usable in target-independent code.
> +        self._event_enum_members.append(QAPISchemaMember(name))
>
>
>  def gen_events(schema, output_dir, prefix):
> --
> 2.17.2
>

Re: [Qemu-devel] [PATCH v3 15/17] Revert "qapi-events: add 'if' condition to implicit event enum"

Posted by Markus Armbruster 6 years, 9 months ago

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi
>
> On Wed, Feb 6, 2019 at 7:17 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> This reverts commit 7bd263490590ee6fcf34ecb6203437e22f6e5a9c.
>>
>> The commit applied the events' conditions to the members of enum
>> QAPIEvent.  Awkward, because it renders QAPIEvent unusable in
>> target-independent code as soon as we make an event target-dependent.
>
> Well, my solution didn't have this problem iirc. You could mention
> that by having target-specific event enums, we could solve this.

What about appending:

    Another option would be to split target-dependent parts off enum
    QAPIEvent into a target-dependent enum.  Doesn't seem worthwhile right
    now.

> It might be worth a FIXME too.

Well, the only thing that's broken is query-events, and I propose to
delete it instead of fixing it.

>> Reverting this has the following effects:
>>
>> * ui/vnc.c can remain target independent.
>>
>> * monitor_qapi_event_conf[] doesn't have to muck around with #ifdef.
>>
>> * query-events again doesn't reflect conditionals.  I'm going to
>>   deprecate it in favor of query-qmp-schema.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> But that's ok for now:
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks!