1. Patchew
  2. QEMU
  3. View series

[PATCH v3 0/9] qapi: untie 'if' conditions from C preprocessor

marcandre.lureau@redhat.com posted 9 patches 3 years ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210429134032.1125111-1-marcandre.lureau@redhat.com
There is a newer version of this series
docs/devel/qapi-code-gen.txt                  |  33 +++---
docs/sphinx/qapidoc.py                        |   6 +-
qapi/block-core.json                          |  16 +--
qapi/block-export.json                        |   6 +-
qapi/char.json                                |   8 +-
qapi/machine-target.json                      |  28 +++--
qapi/migration.json                           |  10 +-
qapi/misc-target.json                         |  37 +++---
qapi/qom.json                                 |  10 +-
qapi/sockets.json                             |   4 +-
qapi/ui.json                                  |  48 ++++----
qga/qapi-schema.json                          |   8 +-
tests/unit/test-qmp-cmds.c                    |   1 +
scripts/qapi/commands.py                      |   4 +-
scripts/qapi/common.py                        | 106 +++++++++++++++---
scripts/qapi/events.py                        |   5 +-
scripts/qapi/expr.py                          |  62 +++++++---
scripts/qapi/gen.py                           |  16 ++-
scripts/qapi/introspect.py                    |  33 +++---
scripts/qapi/schema.py                        |  99 ++++++++++++----
scripts/qapi/types.py                         |  43 ++++---
scripts/qapi/visit.py                         |  25 ++---
.../alternate-branch-if-invalid.err           |   2 +-
tests/qapi-schema/bad-if-empty.err            |   2 +-
tests/qapi-schema/bad-if-list.err             |   2 +-
tests/qapi-schema/bad-if.err                  |   3 +-
tests/qapi-schema/bad-if.json                 |   2 +-
tests/qapi-schema/doc-good.json               |   6 +-
tests/qapi-schema/doc-good.out                |  12 +-
tests/qapi-schema/doc-good.txt                |   6 +-
tests/qapi-schema/enum-if-invalid.err         |   3 +-
tests/qapi-schema/features-if-invalid.err     |   2 +-
tests/qapi-schema/features-missing-name.json  |   2 +-
tests/qapi-schema/qapi-schema-test.json       |  58 +++++-----
tests/qapi-schema/qapi-schema-test.out        |  67 ++++++-----
.../qapi-schema/struct-member-if-invalid.err  |   2 +-
tests/qapi-schema/union-branch-if-invalid.err |   2 +-
37 files changed, 482 insertions(+), 297 deletions(-)
[PATCH v3 0/9] qapi: untie 'if' conditions from C preprocessor
Posted by marcandre.lureau@redhat.com 3 years ago 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

This series makes the 'if' conditions less liberal, by formalizing a simple
expression tree based on bare boolean logic of configure option identifiers.

(this allows to express conditions in Rust in my QAPI-Rust PoC series)

This is based on John Snow QAPI pt4:
https://patchew.org/QEMU/20210421192233.3542904-1-jsnow@redhat.com/

Based-on: <20210421192233.3542904-2-jsnow@redhat.com>

thanks

v3:
 - rebasing on queued pt4 (after waiting for it to land)
 - improve documentation generation, to be more human-friendly
 - drop typing annotations from schema.py (not yet queued)
 - commit message tweaks

v2:
 - fix the normalization step to handle recursive expr
 - replace IfCond by QAPISchemaIf (JohnS)
 - commit message and documentation tweaks
 - mypy/flake8/isort

Marc-André Lureau (9):
  qapi: replace List[str] by QAPISchemaIfCond
  qapi: move gen_if/gen_endif to QAPISchemaIfCond
  qapi: start building an 'if' predicate tree
  qapi: introduce IfPredicateList and IfAny
  qapi: add IfNot
  qapi: normalize 'if' condition to IfPredicate tree
  qapi: convert 'if' C-expressions to the new syntax tree
  qapi: make 'if' condition strings simple identifiers
  docs: update the documentation about schema configuration

 docs/devel/qapi-code-gen.txt                  |  33 +++---
 docs/sphinx/qapidoc.py                        |   6 +-
 qapi/block-core.json                          |  16 +--
 qapi/block-export.json                        |   6 +-
 qapi/char.json                                |   8 +-
 qapi/machine-target.json                      |  28 +++--
 qapi/migration.json                           |  10 +-
 qapi/misc-target.json                         |  37 +++---
 qapi/qom.json                                 |  10 +-
 qapi/sockets.json                             |   4 +-
 qapi/ui.json                                  |  48 ++++----
 qga/qapi-schema.json                          |   8 +-
 tests/unit/test-qmp-cmds.c                    |   1 +
 scripts/qapi/commands.py                      |   4 +-
 scripts/qapi/common.py                        | 106 +++++++++++++++---
 scripts/qapi/events.py                        |   5 +-
 scripts/qapi/expr.py                          |  62 +++++++---
 scripts/qapi/gen.py                           |  16 ++-
 scripts/qapi/introspect.py                    |  33 +++---
 scripts/qapi/schema.py                        |  99 ++++++++++++----
 scripts/qapi/types.py                         |  43 ++++---
 scripts/qapi/visit.py                         |  25 ++---
 .../alternate-branch-if-invalid.err           |   2 +-
 tests/qapi-schema/bad-if-empty.err            |   2 +-
 tests/qapi-schema/bad-if-list.err             |   2 +-
 tests/qapi-schema/bad-if.err                  |   3 +-
 tests/qapi-schema/bad-if.json                 |   2 +-
 tests/qapi-schema/doc-good.json               |   6 +-
 tests/qapi-schema/doc-good.out                |  12 +-
 tests/qapi-schema/doc-good.txt                |   6 +-
 tests/qapi-schema/enum-if-invalid.err         |   3 +-
 tests/qapi-schema/features-if-invalid.err     |   2 +-
 tests/qapi-schema/features-missing-name.json  |   2 +-
 tests/qapi-schema/qapi-schema-test.json       |  58 +++++-----
 tests/qapi-schema/qapi-schema-test.out        |  67 ++++++-----
 .../qapi-schema/struct-member-if-invalid.err  |   2 +-
 tests/qapi-schema/union-branch-if-invalid.err |   2 +-
 37 files changed, 482 insertions(+), 297 deletions(-)

-- 
2.29.0
Re: [PATCH v3 0/9] qapi: untie 'if' conditions from C preprocessor
Posted by Stefan Hajnoczi 2 years, 11 months ago 
On Thu, Apr 29, 2021 at 05:40:23PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Hi,
> 
> This series makes the 'if' conditions less liberal, by formalizing a simple
> expression tree based on bare boolean logic of configure option identifiers.
> 
> (this allows to express conditions in Rust in my QAPI-Rust PoC series)
> 
> This is based on John Snow QAPI pt4:
> https://patchew.org/QEMU/20210421192233.3542904-1-jsnow@redhat.com/
> 
> Based-on: <20210421192233.3542904-2-jsnow@redhat.com>
> 
> thanks
> 
> v3:
>  - rebasing on queued pt4 (after waiting for it to land)
>  - improve documentation generation, to be more human-friendly
>  - drop typing annotations from schema.py (not yet queued)
>  - commit message tweaks
> 
> v2:
>  - fix the normalization step to handle recursive expr
>  - replace IfCond by QAPISchemaIf (JohnS)
>  - commit message and documentation tweaks
>  - mypy/flake8/isort
> 
> Marc-André Lureau (9):
>   qapi: replace List[str] by QAPISchemaIfCond
>   qapi: move gen_if/gen_endif to QAPISchemaIfCond
>   qapi: start building an 'if' predicate tree
>   qapi: introduce IfPredicateList and IfAny
>   qapi: add IfNot
>   qapi: normalize 'if' condition to IfPredicate tree
>   qapi: convert 'if' C-expressions to the new syntax tree
>   qapi: make 'if' condition strings simple identifiers
>   docs: update the documentation about schema configuration
> 
>  docs/devel/qapi-code-gen.txt                  |  33 +++---
>  docs/sphinx/qapidoc.py                        |   6 +-
>  qapi/block-core.json                          |  16 +--
>  qapi/block-export.json                        |   6 +-
>  qapi/char.json                                |   8 +-
>  qapi/machine-target.json                      |  28 +++--
>  qapi/migration.json                           |  10 +-
>  qapi/misc-target.json                         |  37 +++---
>  qapi/qom.json                                 |  10 +-
>  qapi/sockets.json                             |   4 +-
>  qapi/ui.json                                  |  48 ++++----
>  qga/qapi-schema.json                          |   8 +-
>  tests/unit/test-qmp-cmds.c                    |   1 +
>  scripts/qapi/commands.py                      |   4 +-
>  scripts/qapi/common.py                        | 106 +++++++++++++++---
>  scripts/qapi/events.py                        |   5 +-
>  scripts/qapi/expr.py                          |  62 +++++++---
>  scripts/qapi/gen.py                           |  16 ++-
>  scripts/qapi/introspect.py                    |  33 +++---
>  scripts/qapi/schema.py                        |  99 ++++++++++++----
>  scripts/qapi/types.py                         |  43 ++++---
>  scripts/qapi/visit.py                         |  25 ++---
>  .../alternate-branch-if-invalid.err           |   2 +-
>  tests/qapi-schema/bad-if-empty.err            |   2 +-
>  tests/qapi-schema/bad-if-list.err             |   2 +-
>  tests/qapi-schema/bad-if.err                  |   3 +-
>  tests/qapi-schema/bad-if.json                 |   2 +-
>  tests/qapi-schema/doc-good.json               |   6 +-
>  tests/qapi-schema/doc-good.out                |  12 +-
>  tests/qapi-schema/doc-good.txt                |   6 +-
>  tests/qapi-schema/enum-if-invalid.err         |   3 +-
>  tests/qapi-schema/features-if-invalid.err     |   2 +-
>  tests/qapi-schema/features-missing-name.json  |   2 +-
>  tests/qapi-schema/qapi-schema-test.json       |  58 +++++-----
>  tests/qapi-schema/qapi-schema-test.out        |  67 ++++++-----
>  .../qapi-schema/struct-member-if-invalid.err  |   2 +-
>  tests/qapi-schema/union-branch-if-invalid.err |   2 +-
>  37 files changed, 482 insertions(+), 297 deletions(-)
> 
> -- 
> 2.29.0
> 
> 
> 

Please double-check that the build and tests pass after each commit (for
bisectability).

I'm not familiar with the details of the QAPI code generator but in
overall this looks like a nice step:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Re: [PATCH v3 0/9] qapi: untie 'if' conditions from C preprocessor
Posted by Marc-André Lureau 2 years, 11 months ago 
Hi Markus

On Tue, May 11, 2021 at 8:53 PM Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Thu, Apr 29, 2021 at 05:40:23PM +0400, marcandre.lureau@redhat.com
> wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Hi,
> >
> > This series makes the 'if' conditions less liberal, by formalizing a
> simple
> > expression tree based on bare boolean logic of configure option
> identifiers.
> >
> > (this allows to express conditions in Rust in my QAPI-Rust PoC series)
> >
> > This is based on John Snow QAPI pt4:
> > https://patchew.org/QEMU/20210421192233.3542904-1-jsnow@redhat.com/
> >
> > Based-on: <20210421192233.3542904-2-jsnow@redhat.com>
> >
>

The patch series applies cleanly on top of master now. I checked no
regression between each commit, including python style checks.

If you are overloaded, can I make a pull request for it?

thanks

> thanks
> >
> > v3:
> >  - rebasing on queued pt4 (after waiting for it to land)
> >  - improve documentation generation, to be more human-friendly
> >  - drop typing annotations from schema.py (not yet queued)
> >  - commit message tweaks
> >
> > v2:
> >  - fix the normalization step to handle recursive expr
> >  - replace IfCond by QAPISchemaIf (JohnS)
> >  - commit message and documentation tweaks
> >  - mypy/flake8/isort
> >
> > Marc-André Lureau (9):
> >   qapi: replace List[str] by QAPISchemaIfCond
> >   qapi: move gen_if/gen_endif to QAPISchemaIfCond
> >   qapi: start building an 'if' predicate tree
> >   qapi: introduce IfPredicateList and IfAny
> >   qapi: add IfNot
> >   qapi: normalize 'if' condition to IfPredicate tree
> >   qapi: convert 'if' C-expressions to the new syntax tree
> >   qapi: make 'if' condition strings simple identifiers
> >   docs: update the documentation about schema configuration
> >
> >  docs/devel/qapi-code-gen.txt                  |  33 +++---
> >  docs/sphinx/qapidoc.py                        |   6 +-
> >  qapi/block-core.json                          |  16 +--
> >  qapi/block-export.json                        |   6 +-
> >  qapi/char.json                                |   8 +-
> >  qapi/machine-target.json                      |  28 +++--
> >  qapi/migration.json                           |  10 +-
> >  qapi/misc-target.json                         |  37 +++---
> >  qapi/qom.json                                 |  10 +-
> >  qapi/sockets.json                             |   4 +-
> >  qapi/ui.json                                  |  48 ++++----
> >  qga/qapi-schema.json                          |   8 +-
> >  tests/unit/test-qmp-cmds.c                    |   1 +
> >  scripts/qapi/commands.py                      |   4 +-
> >  scripts/qapi/common.py                        | 106 +++++++++++++++---
> >  scripts/qapi/events.py                        |   5 +-
> >  scripts/qapi/expr.py                          |  62 +++++++---
> >  scripts/qapi/gen.py                           |  16 ++-
> >  scripts/qapi/introspect.py                    |  33 +++---
> >  scripts/qapi/schema.py                        |  99 ++++++++++++----
> >  scripts/qapi/types.py                         |  43 ++++---
> >  scripts/qapi/visit.py                         |  25 ++---
> >  .../alternate-branch-if-invalid.err           |   2 +-
> >  tests/qapi-schema/bad-if-empty.err            |   2 +-
> >  tests/qapi-schema/bad-if-list.err             |   2 +-
> >  tests/qapi-schema/bad-if.err                  |   3 +-
> >  tests/qapi-schema/bad-if.json                 |   2 +-
> >  tests/qapi-schema/doc-good.json               |   6 +-
> >  tests/qapi-schema/doc-good.out                |  12 +-
> >  tests/qapi-schema/doc-good.txt                |   6 +-
> >  tests/qapi-schema/enum-if-invalid.err         |   3 +-
> >  tests/qapi-schema/features-if-invalid.err     |   2 +-
> >  tests/qapi-schema/features-missing-name.json  |   2 +-
> >  tests/qapi-schema/qapi-schema-test.json       |  58 +++++-----
> >  tests/qapi-schema/qapi-schema-test.out        |  67 ++++++-----
> >  .../qapi-schema/struct-member-if-invalid.err  |   2 +-
> >  tests/qapi-schema/union-branch-if-invalid.err |   2 +-
> >  37 files changed, 482 insertions(+), 297 deletions(-)
> >
> > --
> > 2.29.0
> >
> >
> >
>
> Please double-check that the build and tests pass after each commit (for
> bisectability).
>
> I'm not familiar with the details of the QAPI code generator but in
> overall this looks like a nice step:
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>


-- 
Marc-André Lureau
Re: [PATCH v3 0/9] qapi: untie 'if' conditions from C preprocessor
Posted by Markus Armbruster 2 years, 11 months ago 
Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi Markus
>
> On Tue, May 11, 2021 at 8:53 PM Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
>> On Thu, Apr 29, 2021 at 05:40:23PM +0400, marcandre.lureau@redhat.com
>> wrote:
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > Hi,
>> >
>> > This series makes the 'if' conditions less liberal, by formalizing a
>> simple
>> > expression tree based on bare boolean logic of configure option
>> identifiers.
>> >
>> > (this allows to express conditions in Rust in my QAPI-Rust PoC series)
>> >
>> > This is based on John Snow QAPI pt4:
>> > https://patchew.org/QEMU/20210421192233.3542904-1-jsnow@redhat.com/
>> >
>> > Based-on: <20210421192233.3542904-2-jsnow@redhat.com>
>> >
>>
>
> The patch series applies cleanly on top of master now. I checked no
> regression between each commit, including python style checks.

Appears to conflict with John's "[PATCH v2 00/21] qapi: static typing
conversion, pt5a".  I didn't examine the conflicts.

Since I reviewed John's v1 recently, and git-range-diff to v2 looks
fairly innocent at a glance, I'd prefer not to rock that boat.  Let's
discuss what to do as soon as I reviewed John's v2.

> If you are overloaded, can I make a pull request for it?

Not yet, please.
Re: [PATCH v3 0/9] qapi: untie 'if' conditions from C preprocessor
Posted by John Snow 2 years, 11 months ago 
On 5/12/21 1:43 PM, Markus Armbruster wrote:
> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> 
>> Hi Markus
>>
>> On Tue, May 11, 2021 at 8:53 PM Stefan Hajnoczi <stefanha@gmail.com> wrote:
>>
>>> On Thu, Apr 29, 2021 at 05:40:23PM +0400, marcandre.lureau@redhat.com
>>> wrote:
>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>
>>>> Hi,
>>>>
>>>> This series makes the 'if' conditions less liberal, by formalizing a
>>> simple
>>>> expression tree based on bare boolean logic of configure option
>>> identifiers.
>>>>
>>>> (this allows to express conditions in Rust in my QAPI-Rust PoC series)
>>>>
>>>> This is based on John Snow QAPI pt4:
>>>> https://patchew.org/QEMU/20210421192233.3542904-1-jsnow@redhat.com/
>>>>
>>>> Based-on: <20210421192233.3542904-2-jsnow@redhat.com>
>>>>
>>>
>>
>> The patch series applies cleanly on top of master now. I checked no
>> regression between each commit, including python style checks.
> 
> Appears to conflict with John's "[PATCH v2 00/21] qapi: static typing
> conversion, pt5a".  I didn't examine the conflicts.
> 
> Since I reviewed John's v1 recently, and git-range-diff to v2 looks
> fairly innocent at a glance, I'd prefer not to rock that boat.  Let's
> discuss what to do as soon as I reviewed John's v2.
> 

It should hopefully be very minimal. I advised Marc-Andre to rebase on 
master as this series did not appear to touch the parser. Conflict 
should be minimal-ish.

(Maybe it's clashing in Schema just a little bit? I do touch schema.py 
very gently...)

>> If you are overloaded, can I make a pull request for it?
> 
> Not yet, please.
> 

As soon as I am done sending a new version of my Python packaging and CI 
series I will run this through the wringer and give my reviews.

--js

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.