Message ID | 20210517163040.2308926-1-marcandre.lureau@redhat.com (mailing list archive) |
---|---|
Headers | show |
Series | qapi: untie 'if' conditions from C preprocessor | expand |
Since I won't be able to complete my review before my vaction, let me at least give you some high-level feedback. I admit I'm rather fond of the the radically simple, stupid way 'if' works. It's tied to C, which is just fine as long as all we need is C. Since we're going to need more than C, it's no longer fine. You replace the stupid C preprocessor expression string by a little language. The little language is less expressive than C preprocessor expressions. I agree with that choice. Simplicity is more important than expressiveness we may well never need. I'm a bit concerned about excessive OO boilerplate on the one hand, and keeping stuff factored cleanly on the other hand. Balancing act. We'll figure it out.
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 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): 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: 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 | 36 ++++-- 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 | 53 +++++--- 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.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/test-qapi.py | 2 +- tests/qapi-schema/union-branch-if-invalid.err | 2 +- 38 files changed, 493 insertions(+), 280 deletions(-)