mbox series

[v5,0/9] qapi: untie 'if' conditions from C preprocessor

Message ID 20210608120723.2268181-1-marcandre.lureau@redhat.com
Headers show
Series qapi: untie 'if' conditions from C preprocessor | expand

Message

Marc-André Lureau June 8, 2021, 12:07 p.m. UTC 
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)

thanks

v5:
 - drop the [ COND, ... ] sugar form
 - move documentation update as first patch
 - documentation and commit message tweaks

v4:
 - keep gen_if/gen_endif in common.py, reducing C codegen in schema.py
 - raise NotImplemented instead of False for unhandled __eq__
 - change check_if() to keep the json/raw form, add _make_if() to build a
   QAPISchemaIfCond
 - improve __repr__ usage
 - drop ABC usage
 - tweaks here and there
 - add various commit tags

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):
  docs: update the documentation about schema configuration
  qapi: replace List[str] by QAPISchemaIfCond
  qapi: make gen_if/gen_endif take a simple string
  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/devel/qapi-code-gen.txt                  |  30 +++--
 docs/sphinx/qapidoc.py                        |   6 +-
 qapi/block-core.json                          |  16 +--
 qapi/block-export.json                        |   6 +-
 qapi/char.json                                |  12 +-
 qapi/machine-target.json                      |  28 +++--
 qapi/migration.json                           |  10 +-
 qapi/misc-target.json                         |  40 +++---
 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                        | 116 ++++++++++++++++--
 scripts/qapi/events.py                        |   5 +-
 scripts/qapi/expr.py                          |  51 +++++---
 scripts/qapi/gen.py                           |  14 +--
 scripts/qapi/introspect.py                    |  26 ++--
 scripts/qapi/schema.py                        | 112 +++++++++++++----
 scripts/qapi/types.py                         |  33 ++---
 scripts/qapi/visit.py                         |  23 ++--
 .../alternate-branch-if-invalid.err           |   2 +-
 tests/qapi-schema/bad-if-empty-list.json      |   2 +-
 tests/qapi-schema/bad-if-empty.err            |   2 +-
 tests/qapi-schema/bad-if-list.err             |   2 +-
 tests/qapi-schema/bad-if-list.json            |   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       |  60 +++++----
 tests/qapi-schema/qapi-schema-test.out        |  67 +++++-----
 .../qapi-schema/struct-member-if-invalid.err  |   2 +-
 tests/qapi-schema/test-qapi.py                |   2 +-
 tests/qapi-schema/union-branch-if-invalid.err |   2 +-
 .../qapi-schema/union-branch-if-invalid.json  |   2 +-
 41 files changed, 498 insertions(+), 286 deletions(-)

Comments

Markus Armbruster June 15, 2021, 11:56 a.m. UTC | #1 
Done, except for the tests in PATCH 7.

I agree with the QAPI schema language change.

Having this many classes just for conditionals feels tiresome.  I'm
tempted to try axing all but one just to see how it comes out.  This is
not a demand.

Let's discuss my review comments, and then figure out what still needs
to be done to get merged.