Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221104160712.3005652-31-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qga/qapi-schema.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Michael Roth <michael.roth@amd.com>
Cc: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20221104160712.3005652-30-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/virtio.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221104160712.3005652-29-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/ui.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20221104160712.3005652-28-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/transaction.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
In qmp_transaction(), we can't just drop parameter @has_props, since
it's used to track whether @props needs to be freed. Replace it by a
local variable.
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20221104160712.3005652-27-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/tpm.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Message-Id: <20221104160712.3005652-26-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/stats.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Mark Kanda <mark.kanda@oracle.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Mark Kanda <mark.kanda@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221104160712.3005652-25-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/run-state.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Drop a superfluous conditional around
qapi_free_GuestPanicInformation() while there.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221104160712.3005652-24-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/rocker.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20221104160712.3005652-23-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/replay.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221104160712.3005652-22-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/qdev.json and
qapi/qom.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Eduardo Habkost <eduardo@habkost.net>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20221104160712.3005652-21-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/pci.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221104160712.3005652-20-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/net.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Jason Wang <jasowang@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20221104160712.3005652-19-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
[Fixes for MacOS squashed in]
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/misc.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20221104160712.3005652-18-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/migration.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Juan Quintela <quintela@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20221104160712.3005652-17-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/machine*.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20221104160712.3005652-16-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/job.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: John Snow <jsnow@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20221104160712.3005652-15-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/dump.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20221104160712.3005652-14-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/crypto.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221104160712.3005652-13-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/char.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221104160712.3005652-12-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/block*.json.
Said commit explains the transformation in more detail.
There is one instance of the invariant violation mentioned there:
qcow2_signal_corruption() passes false, "" when node_name is an empty
string. Take care to pass NULL then.
The previous two commits cleaned up two more.
Additionally, helper bdrv_latency_histogram_stats() loses its output
parameters and returns a value instead.
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20221104160712.3005652-11-armbru@redhat.com>
[Fixes for #ifndef LIBRBD_SUPPORTS_ENCRYPTION and MacOS squashed in]
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/audio.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Additionally, helper get_str() loses its @has_dst parameter.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20221104160712.3005652-8-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for qapi/acpi.py.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <ani@anisinha.ca>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20221104160712.3005652-7-armbru@redhat.com>
The has_FOO for pointer-valued FOO are redundant, except for arrays.
They are also a nuisance to work with. Recent commit "qapi: Start to
elide redundant has_FOO in generated C" provided the means to elide
them step by step. This is the step for
tests/qapi-schema/qapi-schema-test.json.
Said commit explains the transformation in more detail. The invariant
violations mentioned there do not occur here.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20221104160712.3005652-6-armbru@redhat.com>
In QAPI, absent optional members are distinct from any present value.
We thus represent an optional schema member FOO as two C members: a
FOO with the member's type, and a bool has_FOO. Likewise for function
arguments.
However, has_FOO is actually redundant for a pointer-valued FOO, which
can be null only when has_FOO is false, i.e. has_FOO == !!FOO. Except
for arrays, where we a null FOO can also be a present empty array.
The redundant has_FOO are a nuisance to work with. Improve the
generator to elide them. Uses of has_FOO need to be replaced as
follows.
Tests of has_FOO become the equivalent comparison of FOO with null.
For brevity, this is commonly done by implicit conversion to bool.
Assignments to has_FOO get dropped.
Likewise for arguments to has_FOO parameters.
Beware: code may violate the invariant has_FOO == !!FOO before the
transformation, and get away with it. The above transformation can
then break things. Two cases:
* Absent: if code ignores FOO entirely when !has_FOO (except for
freeing it if necessary), even non-null / uninitialized FOO works.
Such code is known to exist.
* Present: if code ignores FOO entirely when has_FOO, even null FOO
works. Such code should not exist.
In both cases, replacing tests of has_FOO by FOO reverts their sense.
We have to fix the value of FOO then.
To facilitate review of the necessary updates to handwritten code, add
means to opt out of this change, and opt out for all QAPI schema
modules where the change requires updates to handwritten code. The
next few commits will remove these opt-outs in reviewable chunks, then
drop the means to opt out.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20221104160712.3005652-5-armbru@redhat.com>
Detect array types as alternate branches, and turn the JSON list into
a QAPISchemaArrayType. Array types in an alternate are represented with
QTYPE_QLIST in the type field.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220321164243.200569-2-pbonzini@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
New option parameters unstable-input and unstable-output set policy
for unstable interfaces just like deprecated-input and
deprecated-output set policy for deprecated interfaces (see commit
6dd75472d5 "qemu-options: New -compat to set policy for deprecated
interfaces"). This is intended for testing users of the management
interfaces. It is experimental.
For now, this covers only syntactic aspects of QMP, i.e. stuff tagged
with feature 'unstable'. We may want to extend it to cover semantic
aspects, or the command line.
Note that there is no good way for management application to detect
presence of these new option parameters: they are not visible output
of query-qmp-schema or query-command-line-options. Tolerable, because
it's meant for testing. If running with -compat fails, skip the test.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: John Snow <jsnow@redhat.com>
Message-Id: <20211028102520.747396-10-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Doc comments fixed up]
New enum QapiSpecialFeature enumerates the special feature flags.
New helper gen_special_features() returns code to represent a
collection of special feature flags as a bitset.
The next few commits will put them to use.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-Id: <20211028102520.747396-5-armbru@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
This is quite similar to commit 84ab008687 "qapi: Add feature flags to
struct members", only for enums instead of structs.
Special feature flag 'deprecated' is silently ignored there. This is
okay only because it will be implemented shortly.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20211025042405.3762351-3-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Simple unions predate flat unions. Having both complicates the QAPI
schema language and the QAPI generator. We haven't been using simple
unions in new code for a long time, because they are less flexible and
somewhat awkward on the wire.
The previous commits eliminated simple union from the tree. Now drop
them from the QAPI schema language entirely, and update mentions of
"flat union" to just "union".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210917143134.412106-22-armbru@redhat.com>
None works fine, there is no need to replace it by {} in .__init__().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-3-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
QAPISchemaIfCond.cgen() is only ever used like
gen_if(ifcond.cgen())
and
gen_endif(ifcond.cgen())
Simplify to
ifcond.gen_if()
and
ifcond.gen_endif()
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-2-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[Import statements tidied up with isort]
Replace the simple list sugar form with a recursive structure that will
accept other operators in the following commits (all, any or not).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20210804083105.97531-7-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Accidental code motion undone. Degenerate :forms: comment dropped.
Helper _check_if() moved. Error messages tweaked. ui.json updated.
Accidental changes to qapi-schema-test.json dropped.]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Instead of building the condition documentation from a list of string,
use the result generated from QAPISchemaIfCond.docgen().
This changes the generated documentation from:
- COND1, COND2... (where COND1, COND2 are Literal nodes, and ',' is Text)
to:
- COND1 and COND2 (the whole string as a Literal node)
This will allow us to generate more complex conditions in the following
patches, such as "(COND1 and COND2) or COND3".
Adding back the differentiated formatting is left to the wish list.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20210804083105.97531-6-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[TODO comment added]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Instead of building prepocessor conditions from a list of string, use
the result generated from QAPISchemaIfCond.cgen() and hide the
implementation details.
Note: this patch introduces a minor regression, generating a redundant
pair of parenthesis. This is mostly fixed in a later patch in this
series ("qapi: replace if condition list with dict [..]")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20210804083105.97531-5-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Mechanical change, except for a new assertion in
QAPISchemaEntity.ifcond().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20210804083105.97531-3-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Rebased with obvious conflicts, commit message adjusted]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
QAPISchema._make_implicit_object_type() asserts that when an implicit
object type is used multiple times, @ifcond is the same for all uses.
It will be for legitimate uses, i.e. simple union branch wrapper
types. A comment explains this.
The assertion fails when a command or event is redefined with a
different condition. The redefinition is an error, but it's flagged
only later.
Fixing the assertion would complicate matters further. Not
worthwhile, drop it instead. We really need to get rid of simple
unions.
Tweak test case redefined-event to cover redefinition with a different
condition.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210806120510.2367124-1-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Fixes: f5d4361cda
Fixes: 52a474180a
Fixes: 46f49468c6
Remove the try/except block that handles file-opening errors in
QAPISchemaParser.__init__() and add one each to
QAPISchemaParser._include() and QAPISchema.__init__() respectively.
This simultaneously fixes the typing of info.fname (f5d4361cda), A
static typing violation in test-qapi (46f49468c6), and a regression of
an error message (52a474180a).
The short-ish version of what motivates this patch is:
- It's hard to write a good error message in the init method,
because we need to determine the context of our caller to do so.
It's easier to just let the caller write the message.
- We don't want to allow QAPISourceInfo(None, None, None) to exist. The
typing introduced by commit f5d4361cda types the 'fname' field as
(non-optional) str, which was premature until the removal of this
construct.
- Errors made using such an object are currently incorrect (since
52a474180a)
- It's not technically a semantic error if we cannot open the schema.
- There are various typing constraints that make mixing these two cases
undesirable for a single special case.
- test-qapi's code handling an fname of 'None' is now dead, drop it.
Additionally, Not all QAPIError objects have an 'info' field (since
46f49468), so deleting this stanza corrects a typing oversight in
test-qapi introduced by that commit.
Other considerations:
- open() is moved to a 'with' block to ensure file pointers are
cleaned up deterministically.
- Python 3.3 deprecated IOError and made it a synonym for OSError.
Avoid the misleading perception these exception handlers are
narrower than they really are.
The long version:
The error message here is incorrect (since commit 52a474180a):
> python3 qapi-gen.py 'fake.json'
qapi-gen.py: qapi-gen.py: can't read schema file 'fake.json': No such file or directory
In pursuing it, we find that QAPISourceInfo has a special accommodation
for when there's no filename. Meanwhile, the intent when QAPISourceInfo
was typed (f5d4361cda) was non-optional 'str'. This usage was
overlooked.
To remove this, I'd want to avoid having a "fake" QAPISourceInfo
object. I also don't want to explicitly begin accommodating
QAPISourceInfo itself being None, because we actually want to eventually
prove that this can never happen -- We don't want to confuse "The file
isn't open yet" with "This error stems from a definition that wasn't
defined in any file".
(An earlier series tried to create a dummy info object, but it was tough
to prove in review that it worked correctly without creating new
regressions. This patch avoids that distraction. We would like to first
prove that we never raise QAPISemError for any built-in object before we
add "special" info objects. We aren't ready to do that yet.)
So, which way out of the labyrinth?
Here's one way: Don't try to handle errors at a level with "mixed"
semantic contexts; i.e. don't mix inclusion errors (should report a
source line where the include was triggered) and command line errors
(where we specified a file we couldn't read).
Remove the error handling from the initializer of the parser. Pythonic!
Now it's the caller's job to figure out what to do about it. Handle the
error in QAPISchemaParser._include() instead, where we can write a
targeted error message where we are guaranteed to have an 'info' context
to report with.
The root level error can similarly move to QAPISchema.__init__(), where
we know we'll never have an info context to report with, so we use a
more abstract error type.
Now the error looks sensible again:
> python3 qapi-gen.py 'fake.json'
qapi-gen.py: can't read schema file 'fake.json': No such file or directory
With these error cases separated, QAPISourceInfo can be solidified as
never having placeholder arguments that violate our desired types. Clean
up test-qapi along similar lines.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-2-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
It's already treated as optional, with one direct caller and some
subclass callers passing 'None'. Make it officially optional, which
requires moving the position of the argument to come after all required
parameters.
QAPISemError becomes functionally identical to QAPISourceError. Keep the
name to preserve its semantic meaning and avoid code churn, but remove
the now-useless __init__ wrapper.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210421192233.3542904-4-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Rename QAPIError to QAPISourceError, and then create a new QAPIError
class that serves as the basis for all of our other custom exceptions,
without specifying any class properties.
This leaves QAPIError as a package-wide error class that's suitable for
any current or future errors.
(Right now, we don't have any errors that DON'T also want to specify a
Source location, but this MAY change. In these cases, a common abstract
ancestor would be desired.)
Add docstrings to explain the intended function of each error class.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210421192233.3542904-2-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Rename pragma returns-whitelist to command-returns-exceptions, and
name-case-whitelist to member-name-case-exceptions.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-20-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
NB: The type aliases (SchemaInfo et al) declare intent for some of the
"dictly-typed" objects we pass around in introspect.py. They do not
enforce the shape of those objects, and cannot, until Python 3.7 or
later. (And even then, it may not be "worth it".)
Annotations are also added to the QAPISchemaEntity __init__ method in
schema.py to allow mypy to statically prove the type of typ.name,
needed to prove the return type of
QAPISchemaGenIntrospectVisitor._use_type().
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210216021809.134886-15-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Note on QAPISchemaEntity.__init__() squashed into commit message,
Comment wrapped to conform to PEP 8]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Use a constant to make it obvious we're referring to a very specific thing.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210201193747.2169670-11-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Use './builtin' as the built-in module name instead of
None. Clarify the typing that this is now always a string.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210201193747.2169670-9-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Define what a module is and define what kind of a module it is once and
for all, in one place.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210201193747.2169670-6-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Fix a minor typing issue, and then establish a mypy type-checking
baseline.
Like pylint, this should be run from the folder above:
> mypy --config-file=qapi/mypy.ini qapi/
This is designed and tested for mypy 0.770 or greater.
Signed-off-by: John Snow <jsnow@redhat.com>
Tested-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20201009161558.107041-19-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
At this point, that just means using a consistent strategy for constant names.
constants get UPPER_CASE and names not used externally get a leading underscore.
As a preference, while renaming constants to be UPPERCASE, move them to
the head of the file. Generally, it's nice to be able to audit the code
that runs on import in one central place.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20201009161558.107041-13-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Petty style guide fixes and line length enforcement. Not a big win, not
a big loss, but flake8 passes 100% on the qapi module, which gives us an
easy baseline to enforce hereafter.
A note on the flake8 exception: flake8 will warn on *any* bare except,
but pylint's is context-aware and will suppress the warning if you
re-raise the exception.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20201009161558.107041-9-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
While we're mucking around with imports, we might as well formalize the
style we use. Let's use isort to do it for us.
lines_after_imports=2: Use two lines after imports, to match PEP8's
desire to have "two lines before and after" class definitions, which are
likely to start immediately after imports.
force_sort_within_sections: Intermingles "from x" and "import x" style
statements, such that sorting is always performed strictly on the module
name itself.
force_grid_wrap=4: Four or more imports from a single module will force
the one-per-line style that's more git-friendly. This will generally
happen for 'typing' imports.
multi_line_output=3: Uses the one-per-line indented style for long
imports.
include_trailing_comma: Adds a comma to the last import in a group,
which makes git conflicts nicer to deal with, generally.
line_length: 72 is chosen to match PEP8's "docstrings and comments" line
length limit. If you have a single line import that exceeds 72
characters, your names are too long!
Suggested-by: Cleber Rosa <crosa@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201009161558.107041-8-jsnow@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
