A definition's conditional should imply the conditionals of types it
uses. If it doesn't, some configurations won't compile.
Example (from tests/qapi-schema/qapi-schema-test.json):
{ 'union': 'TestIfUnion', 'data':
{ 'foo': 'TestStruct',
'bar': { 'type': 'str', 'if': 'TEST_IF_UNION_BAR'} },
'if': { 'all': ['TEST_IF_UNION', 'TEST_IF_STRUCT'] } }
{ 'command': 'test-if-union-cmd',
'data': { 'union-cmd-arg': 'TestIfUnion' },
'if': 'TEST_IF_UNION' }
generates
#if (defined(TEST_IF_UNION)) && (defined(TEST_IF_STRUCT))
typedef struct TestIfUnion TestIfUnion;
#endif /* (defined(TEST_IF_UNION)) && (defined(TEST_IF_STRUCT)) */
and
#if defined(TEST_IF_UNION)
void qmp_test_if_union_cmd(TestIfUnion *union_cmd_arg, Error **errp);
void qmp_marshal_test_if_union_cmd(QDict *args, QObject **ret, Error **errp);
#endif /* defined(TEST_IF_UNION) */
which doesn't compile when !defined(TEST_IF_STRUCT).
Messed up in f8c4fdd6ae "tests/qapi: Cover commands with 'if' and
union / alternate 'data'", v4.0.0. Harmless, as we don't actually use
this configuration. Correct it anyway, along with another instance.
This loses coverage for 'not'. The next commit will bring it back.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210831123809.1107782-4-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Change the 'if' condition strings to be C-agnostic. It will accept
'[A-Z][A-Z0-9_]*' identifiers. This allows to express configuration
conditions in other languages (Rust or Python for ex) or other more
suitable forms.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: John Snow <jsnow@redhat.com>
Message-Id: <20210804083105.97531-11-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Rebased with semantic conflict in redefined-event.json]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
For the sake of completeness, introduce the 'not' condition.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20210804083105.97531-10-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Long line broken in tests/qapi-schema/qapi-schema-test.json]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
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>
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>
New test case enum-dict-no-name.json crashes:
$ python3 scripts/qapi-gen.py tests/qapi-schema/enum-dict-no-name.json
Traceback (most recent call last):
[...]
File "/work/armbru/qemu/scripts/qapi/expr.py", line 458, in check_enum
member_name = member['name']
KeyError: 'name'
Root cause: we try to retrieve member 'name' before we check for
missing members. With that fixed, we get the expected error "'data'
member misses key 'name'".
Fixes: 0825f62c84
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210616072121.626431-1-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Commit a14f0bf165 ("docs: convert build system documentation to rST")
converted docs/devel/build-system.txt to docs/devel/build-system.rst.
We still have several references to the old file, so let's fix them
with the following command:
sed -i s/build-system.txt/build-system.rst/ \
$(git grep -l docs/devel/build-system.txt)
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20210517151702.109066-4-sgarzare@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
When the token can be None (EOF), we can't use 'x in "abc"' style
membership tests to group types of tokens together, because 'None in
"abc"' is a TypeError.
Easy enough to fix. (Use a tuple: It's neither a static typing error nor
a runtime error to check for None in Tuple[str, ...])
Add tests to prevent a regression. (Note: they cannot be added prior to
this fix, as the unhandled stack trace will not match test output in the
CI system.)
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-11-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Instead of using get_expr nested=False, allow get_expr to always return
any expression. In exchange, add a new error message to the top-level
parser that explains the semantic error: Top-level expressions must
always be JSON objects.
This helps mypy understand the rest of this function which assumes that
get_expr did indeed return a dict.
The exception type changes from QAPIParseError to QAPISemError as a
result, and the error message in two tests now changes.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-7-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
This tests the error-return pathway introduced in the previous commit.
(Thanks to Paolo for the help with the Meson magic.)
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-3-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@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>
Prior to this commit, specifying a non-object value here causes the QAPI
parser to crash in expr.py with a stack trace with (likely) an
AttributeError when we attempt to call that value's items() method.
This member needs to be an object (Dict), and not anything else. Add a
check for this with a nicer error message, and formalize that check with
new test cases that exercise that error.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210421182032.3521476-8-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Union branch names should use '-', not '_'. Enforce this. The only
offenders are in tests/. Fix them.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-29-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message typo fixed]
Enum members should use '-', not '_'. Enforce this. Fix the fixable
offenders (all in tests/), and add the remainder to pragma
member-name-exceptions.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-28-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Struct members, including command arguments, event data, and union
inline base members, should use '-', not '_'. Enforce this. Fix the
fixable offenders (all in tests/), and add the remainder to pragma
member-name-exceptions.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-27-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Test args-name-clash covers command parameter name clash. This
effectively covers struct member name clash as well. The next commit
will make parameter name clash impossible. Convert args-name-clash
from testing command to testing a struct, and rename it to
struct-member-name-clash.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-26-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message typo fixed]
Command names should be lower-case. Enforce this. Fix the fixable
offenders (all in tests/), and add the remainder to pragma
command-name-exceptions.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-25-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Command names and member names within a type should be all lower case
with words separated by a hyphen. We also accept underscore. Rework
check_name_lower() to optionally reject underscores, but don't use
that option, yet.
Update expected test output for the changed error message.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-23-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@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>
This test covers returning "bad" types. Pragma returns-whitelist is
just one aspect. Naming it returns-whitelist is suboptimal. Rename
to returns-bad-type.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-19-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Rename pragma-doc-required-crap to pragma-not-bool,
pragma-returns-whitelist-crap to pragma-value-not-list, and
pragma-name-case-whitelist-crap to pragma-value-not-list-of-str.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-18-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The previous commit changed this test to clash with a predefined enum
type, not a built-in type. Adjust its name.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-16-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Type names should be CamelCase. Enforce this. The only offenders are
in tests/. Fix them. Add test type-case to cover the new error.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-15-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Regexp simplified, new test made more robust]
Event names should be ALL_CAPS with words separated by underscore.
Enforce this. The only offenders are in tests/. Fix them. Existing
test event-case covers the new error.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-14-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
check_type() fails to reject optional members with reserved names,
because it neglects to strip off the leading '*'. Fix that.
The stripping in check_name_str() is now useless. Drop.
Also drop the "no leading '*'" assertion, because valid_name.match()
ensures it can't fail.
Fixes: 9fb081e0b9
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-8-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Member name 'u' and names starting with 'has-' or 'has_' are reserved
for the generator. check_type() enforces this, covered by tests
reserved-member-u and reserved-member-has.
These tests neglect to cover optional members, where the name starts
with '*'. Tweak reserved-member-u to fix that. Test
reserved-member-has still covers non-optional members.
This demonstrates the reserved member name check is broken for
optional members. The next commit will fix it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-7-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
[Commit message improved slightly]
Simple unions don't need more features, they need to die.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-6-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Commit 0426d53c65 "qapi: Simplify visiting of alternate types"
eliminated the implicit alternate enum, but neglected to update a
comment about it in a test. Do that now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-5-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
A few old comments talk about "desired future use of defaults" and
"anonymous inline branch types". Kind of misleading since commit
87adbbffd4 "qapi: add a dictionary form for TYPE" added longhand
member definitions. Talk about that instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-4-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
flat-union-inline.json covers longhand branch definition with an
invalid type value. It's redundant: longhand branch definition is
covered by flat-union-inline-invalid-dict.json, and invalid type value
is covered by nested-struct-data.json. Drop the test.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-3-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
This policy suppresses deprecated bits in output, and thus permits
"testing the future". Implement it for QMP event data: suppress
deprecated members.
No QMP event data is deprecated right now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210318155519.1224118-6-armbru@redhat.com>
This policy suppresses deprecated bits in output, and thus permits
"testing the future". Implement it for QMP command results. Example:
when QEMU is run with -compat deprecated-output=hide, then
{"execute": "query-cpus-fast"}
yields
{"return": [{"thread-id": 9805, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "qom-path": "/machine/unattached/device[0]", "cpu-index": 0, "target": "x86_64"}]}
instead of
{"return": [{"arch": "x86", "thread-id": 22436, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "qom-path": "/machine/unattached/device[0]", "cpu-index": 0, "target": "x86_64"}]}
Note the suppression of deprecated member "arch".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210318155519.1224118-4-armbru@redhat.com>
Commit 9d55380b5a "qapi: Remove null from schema language" (v4.2.0)
neglected to update two error messages. Do that now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210224101442.1837475-1-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@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>
This patch adds a new 'coroutine' flag to QMP command definitions that
tells the QMP dispatcher that the command handler is safe to be run in a
coroutine.
The documentation of the new flag pretends that this flag is already
used as intended, which it isn't yet after this patch. We'll implement
this in another patch in this series.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20201005155855.256490-9-kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We no longer use the generated texinfo format documentation,
so delete the code that generates it, and the test case for
the generation.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200925162316.21205-17-peter.maydell@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Add a test of the rST output from the QAPI doc-comment generator,
similar to what we currently have that tests the Texinfo output.
This is a bit more awkward with Sphinx, because the generated output
is not 100% under our control the way the QAPI-to-Texinfo generator
was. We can't observe the data we generate, only the Sphinx
output. Two issues.
One, the output can vary with the Sphinx version. In practice Sphinx's
plaintext output generation has been identical between at least Sphinx
1.6 and 3.0, so we use that. (The HTML output has had changes across
versions). We use an exact-match comparison check, with the
understanding that perhaps changes in a future Sphinx version might
require us to implement something more clever to cope with variation
in the output.
Two, the test can only protect us from changes in the data we generate
that are visible in plain text.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200925162316.21205-16-peter.maydell@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message improved]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
doc-good.json currently uses the old *strong* and _emphasis_ markup.
As part of the conversion to rST this needs to switch to **strong**
and *emphasis*, because rST uses underscores as part of its markup
of hyperlinks and will otherwise warn about the syntax error.
In commit a660eed482 we fixed up the in-tree uses of the
old markup:
1) _this_ was replaced with *this*
2) the only in-tree use of *this* was left alone (turning
a 'strong' into an 'emphasis')
(and so currently in-tree nothing is using either new-style
**strong** or old-style _emphasis_).
Update doc-good.json in a similar way:
1) replace _this_ with *this*
2) remove the usage of old-style *this*
(This slightly reduces the coverage for the old Texinfo generator,
which is about to go away, but is fine for the new rST generator
because that does not need to handle strong/emphasis itself because
it is simply passing the entire text as raw rST to Sphinx.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200925162316.21205-13-peter.maydell@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Make the handling of indentation in doc comments more sophisticated,
so that when we see a section like:
Notes: some text
some more text
indented line 3
we save it for the doc-comment processing code as:
some text
some more text
indented line 3
and when we see a section with the heading on its own line:
Notes:
some text
some more text
indented text
we also accept that and save it in the same form.
If we detect that the comment document text is not indented as much
as we expect it to be, we throw a parse error. (We don't complain
about over-indented sections, because for rST this can be legitimate
markup.)
The golden reference for the doc comment text is updated to remove
the two 'wrong' indents; these now form a test case that we correctly
stripped leading whitespace from an indented multi-line argument
definition.
We update the documentation in docs/devel/qapi-code-gen.txt to
describe the new indentation rules.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200925162316.21205-6-peter.maydell@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Whitespace between sentences tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
As we accumulate lines from doc comments when parsing the JSON, the
QAPIDoc class generally strips leading and trailing whitespace using
line.strip() when it calls _append_freeform(). This is fine for
Texinfo, but for rST leading whitespace is significant. We'd like to
move to having the text in doc comments be rST format rather than a
custom syntax, so move the removal of leading whitespace from the
QAPIDoc class to the texinfo-specific processing code in
texi_format() in qapi/doc.py.
(Trailing whitespace will always be stripped by the rstrip() in
Section::append regardless.)
In a followup commit we will make the whitespace in the lines of doc
comment sections more consistently follow the input source.
There is no change to the generated .texi files before and after this
commit.
Because the qapi-schema test checks the exact values of the
documentation comments against a reference, we need to update that
reference to match the new whitespace. In the first four places this
is now correctly checking that we did put in the amount of whitespace
to pass a rST-formatted list to the backend; in the last two places
the extra whitespace is 'wrong' and will go away again in the
following commit.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200925162316.21205-5-peter.maydell@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
doc-good.json tests doc comment parser corner cases. We're about to
largely replace it by a Sphinx extension, which will have different
corner cases. Tweak the test so it passes both with the old parser
and the Sphinx extension, by making it match the more restrictive
rST syntax:
* in a single list the bullet types must all match
* lists must have leading and following blank lines
* the rules on when and where indentation matters differ
* the '|' example syntax is going to go away entirely, so stop
testing it
This will avoid the tests spuriously breaking when we tighten up the
parser code in the following commits.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200925162316.21205-4-peter.maydell@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Ideally we would use the '--strip-trailing-cr' option, but not
being POSIX is a portability problem (i.e. BSDs and Solaris
based OSes). Instead use the '-b' option which, although doing
slightly more, produce the expected result on Windows."
Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20200915121318.247-11-luoyonggang@gmail.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Our current QAPI doc-comment markup allows section headers (introduced
with a leading '=' or '==') anywhere in a free-form documentation
comment. This works for Texinfo because the generator simply prints a
Texinfo section command at that point in the output stream. For rST
generation, since we're assembling a tree of docutils nodes, this is
awkward because a new section implies starting a new section node at
the top level of the tree and generating text into there.
Make section headers start a new free-form documentation block, so the
future rST document generator doesn't have to look at every line in
free-form blocks and handle headings in odd places.
This change makes no difference to the generated Texinfo.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200320091805.5585-3-armbru@redhat.com>
Section markup in definition documentation makes no sense and can
produce invalid Texinfo. Reject.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200320091805.5585-2-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The various schemas included in QEMU use a JSON-based format which
is, however, strictly speaking not valid JSON.
As a consequence, when vim tries to apply syntax highlight rules
for JSON (as guessed from the file name), the result is an unreadable
mess which mostly consist of red markers pointing out supposed errors
in, well, pretty much everything.
Using Python syntax highlighting produces much better results, and
in fact these files already start with specially-formatted comments
that instruct Emacs to process them as if they were Python files.
This commit adds the equivalent special comments for vim.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Message-Id: <20200729185024.121766-1-abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Unlike regular feature flags, the new special feature flag
"deprecated" is recognized by the QAPI generator. For now, it's only
permitted with commands, events, and struct members. It will be put
to use shortly.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200317115459.31821-26-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Doc typo fixed]
In v4.1.0, we added feature flags just to struct types (commit
6a8c0b5102^..f3ed93d545), to satisfy an immediate need (commit
c9d4070991 "file-posix: Add dynamic-auto-read-only QAPI feature"). In
v4.2.0, we added them to commands (commit 23394b4c39 "qapi: Add
feature flags to commands") to satisfy another immediate need (commit
d76744e65e "qapi: Allow introspecting fix for savevm's cooperation
with blockdev").
Add them to the remaining definitions: enumeration types, union types,
alternate types, and events.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200317115459.31821-13-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200304155932.20452-3-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
This is only needed for Python 2, which we do not support anymore.
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200204160604.19883-1-pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Use the program search path to find the Python 3 interpreter.
Patch created mechanically by running:
$ sed -i "s,^#\!/usr/bin/\(env\ \)\?python$,#\!/usr/bin/env python3," \
$(git grep -l 'if __name__.*__main__')
Reported-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20200130163232.10446-5-philmd@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
When a sub-module doesn't contain any definitions, we don't generate
code for it, but we do generate the #include.
We generate code only for modules that get visited.
QAPISchema.visit() visits only modules that have definitions. It can
visit modules multiple times.
Clean this up as follows. Collect entities in their QAPISchemaModule.
Have QAPISchema.visit() call QAPISchemaModule.visit() for each module.
Have QAPISchemaModule.visit() call .visit_module() for itself, and
QAPISchemaEntity.visit() for each of its entities. This way, we visit
each module exactly once.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191120182551.23795-6-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Commit f3ed93d545 "qapi: Allow documentation for features" neglected
to check documentation against the schema. Fix that: check them the
same way we check arguments.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-20-armbru@redhat.com>
Improve error messages from
the following documented members are not in the declaration: a
the following documented members are not in the declaration: aa, bb
to the more concise
documented member 'a' does not exist
documented members 'aa', 'bb' do not exist
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-19-armbru@redhat.com>
When a command's 'data' is an object, its doc comment describes the
arguments defined there. When 'data' names a type, the doc comment
does not describe arguments. Instead, the doc generator inserts a
pointer to the named type.
An event's doc comment works the same.
We don't actually check doc comments for commands and events.
Instead, QAPISchema._def_command() forwards the doc comment to the
implicit argument type, where it gets checked. Works because the
check only cares for the implicit argument type's members.
Not only is this needlessly hard to understand, it actually falls
apart in two cases:
* When 'data' is empty, there is nothing to forward to, and the doc
comment remains unchecked. Demonstrated by test doc-bad-event-arg.
* When 'data' names a type, we can't forward, as the type has its own
doc comment. The command or event's doc comment remains unchecked.
Demonstrated by test doc-bad-boxed-command-arg.
The forwarding goes back to commit 069fb5b250 "qapi: Prepare for
requiring more complete documentation", put to use in commit
816a57cd6e "qapi: Fix detection of bogus member documentation". That
fix was incomplete.
To fix this, make QAPISchemaCommand and QAPISchemaEvent check doc
comments, and drop the forwarding of doc comments to implicit argument
types.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-12-armbru@redhat.com>
Enumeration type documentation comments are not checked, as
demonstrated by test doc-bad-enum-member. This is because we neglect
to call self.doc.check() for enumeration types. Messed up in
816a57cd6e "qapi: Fix detection of bogus member documentation". Fix
it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-10-armbru@redhat.com>
Generate a reference "Arguments: the members of ...", just like we do
for commands since commit c2dd311cb7 "qapi2texi: Implement boxed
argument documentation".
No change to generated QMP documentation; we don't yet use boxed
events outside tests/.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-7-armbru@redhat.com>
Commit 8aa3a33e44 "tests/qapi-schema: Test for good feature lists in
structs" made test-qapi.py show features, but neglected to show their
documentation. Fix that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-5-armbru@redhat.com>
Add negative tests doc-bad-boxed-command-arg and doc-bad-event-arg to
cover boxed and no arguments. They demonstrate insufficient doc
comment checking.
Update positive test doc-good to cover boxed event arguments. It
demonstrates the generated doc comment misses arguments.
These bugs will be fixed later in this series.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-3-armbru@redhat.com>
Add negative tests doc-bad-enum-member and doc-bad-feature to cover
documentation for nonexistent enum members and features, and test
doc-undoc-feature to cover features lacking documentation. None of
them works. To be fixed later in this series.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-2-armbru@redhat.com>
Commit 8aa3a33e44 "tests/qapi-schema: Test for good feature lists in
structs" neglected to cover documentation comments, and the previous
commit followed its example. Make up for them.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191018081454.21369-5-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191018081454.21369-4-armbru@redhat.com>
Similarly to features for struct types introduce the feature flags also
for commands. This will allow notifying management layers of fixes and
compatible changes in the behaviour of a command which may not be
detectable any other way.
The changes were heavily inspired by commit 6a8c0b5102.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191018081454.21369-3-armbru@redhat.com>
Command and event details are indented three spaces, everything else
four. Messed up in commit 156402e504. Use four spaces consistently.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Message-Id: <20191018081454.21369-2-armbru@redhat.com>
The QAPI code generator clocks in at some 3100 SLOC in 8 source files.
Almost 60% of the code is in qapi/common.py. Split it into more
focused modules:
* Move QAPISchemaPragma and QAPISourceInfo to qapi/source.py.
* Move QAPIError and its sub-classes to qapi/error.py.
* Move QAPISchemaParser and QAPIDoc to parser.py. Use the opportunity
to put QAPISchemaParser first.
* Move check_expr() & friends to qapi/expr.py. Use the opportunity to
put the code into a more sensible order.
* Move QAPISchema & friends to qapi/schema.py
* Move QAPIGen and its sub-classes, ifcontext,
QAPISchemaModularCVisitor, and QAPISchemaModularCVisitor to qapi/gen.py
* Delete camel_case(), it's unused since commit e98859a9b9 "qapi:
Clean up after recent conversions to QAPISchemaVisitor"
A number of helper functions remain in qapi/common.py. I considered
moving the code generator helpers to qapi/gen.py, but decided not to.
Perhaps we should rewrite them as methods of QAPIGen some day.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20191018074345.24034-7-armbru@redhat.com>
[Add "# -*- coding: utf-8 -*-" lines]
"make check-qapi-schema" takes around 10s user + system time for me.
With -j, it takes a bit over 3s real time. We have worse tests. It's
still annoying when you work on the QAPI generator.
Some 1.4s user + system time is consumed by make figuring out what to
do, measured by making a target that does nothing. There's nothing I
can do about that right now. But let's see what we can do about the
other 8s.
Almost 7s are spent running test-qapi.py for every test case, the rest
normalizing and diffing test-qapi.py output. We have 190 test cases.
If I downgrade to python2, it's 4.5s, but python2 is a goner.
Hacking up test-qapi.py to exit(0) without doing anything makes it
only marginally faster. The problem is Python startup overhead.
Our configure puts -B into $(PYTHON). Running without -B is faster:
4.4s.
We could improve the Makefile to run test cases only when the test
case or the generator changed. But I'm after improvement in the case
where the generator changed.
test-qapi.py is designed to be the simplest possible building block
for a shell script to do the complete job (it's actually a Makefile,
not a shell script; no real difference). Python is just not meant for
that. It's for bigger blocks.
Move the post-processing and diffing into test-qapi.py, and make it
capable of testing multiple schema files. Set executable bits while
there.
Running it once per test case now takes slightly longer than 8s. But
running it once for all of them takes under 0.2s.
Messing with the Makefile to run it only on the tests that need
retesting is clearly not worth the bother.
Expected error output changes because the new normalization strips off
$(SRCDIR)/tests/qapi-schema/ instead of just $(SRCDIR)/.
The .exit files go away, because there is no exit status to test
anymore.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20191018074345.24034-5-armbru@redhat.com>
qapi-gen.py crashes when it can't open the main schema file, and when
it can't read from any schema file. Lazy.
Change QAPISchema.__init__() to take a file name instead of a file
object. Move the open code from _include() to __init__(), so it's
used for the main schema file, too.
Move the read into the try for good measure, and rephrase the error
message.
Reporting open or read failure for the main schema file needs a
QAPISourceInfo representing "no source". Make QAPISourceInfo cope
with fname=None.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-27-armbru@redhat.com>
Point to the previous definition, unless it's a built-in.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-26-armbru@redhat.com>
Have check_exprs() check this later, so the error message gains an "in
definition line". Tweak the error message.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-25-armbru@redhat.com>
check_if()'s errors don't point to the offending part of the
expression. For instance:
tests/qapi-schema/alternate-branch-if-invalid.json:2: 'if' condition ' ' makes no sense
Other check_FOO() do, with the help of a @source argument. Make
check_if() do that, too. The example above improves to:
tests/qapi-schema/alternate-branch-if-invalid.json:2: 'if' condition ' ' of 'data' member 'branch' makes no sense
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190927134639.4284-23-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Many error messages refer to the offending definition even though
they're preceded by an "in definition" line. Rephrase them.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190927134639.4284-22-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Have check_exprs() call check_keys() later, so its error messages gain
an "in definition" line.
Both check_keys() and check_name_is_str() check the definition's name
is a string. Since check_keys() now runs after check_name_is_str()
rather than before, its check is dead. Bury it. Checking values in
check_keys() is unclean anyway.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-21-armbru@redhat.com>
Split check_flags() off check_keys() and have check_exprs() call it
later, so its error messages gain an "in definition" line. Tweak the
error messages.
Checking values in a function named check_keys() is unclean anyway.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-20-armbru@redhat.com>
Move check_if() from check_keys() to check_exprs() and call it later,
so its error messages gain an "in definition" line.
Checking values in a function named check_keys() is unclean anyway.
The original sin was commit 0545f6b887 "qapi: Better error messages
for bad expressions", which checks the value of key 'name'. More
sinning in commit 2cbf09925a "qapi: More rigorous checking for type
safety bypass", commit c818408e44 "qapi: Implement boxed types for
commands/events", and commit 967c885108 "qapi: add 'if' to top-level
expressions". This commit does penance for the latter. The next
commits will do penance for the others.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-19-armbru@redhat.com>
QAPISchemaCommand.check() and QAPISchemaEvent().check() check 'data'
is present when 'boxed': true. That's context-free. Move to
check_command() and check_event().
Tweak the error message while there.
check_exprs() & friends now check exactly what qapi-code-gen.txt calls
the second layer of syntax.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-18-armbru@redhat.com>
When we introduced the QAPISchema intermediate representation (commit
ac88219a6c), we took a shortcut: we left check_exprs() & friends
alone instead of moving semantic checks into the
QAPISchemaFOO.check(). The .check() assert check_exprs() did its job.
Time to finish the conversion job. Move exactly the context-sensitive
checks to the .check(). They replace assertions there. Context-free
checks stay put.
Fixes the misleading optional tag error demonstrated by test
flat-union-optional-discriminator.
A few other error message improve.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-17-armbru@redhat.com>
The checks for reserved names are spread far and wide. Move one from
add_name() to new check_defn_name_str(). This is a first step towards
collecting them all in dedicated name checking functions next to
check_name().
While there, drop the quotes around the meta-type in
check_name_str()'s error messages: "'command' uses ... name 'NAME'"
becomes "command uses ... name 'NAME'".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-13-armbru@redhat.com>
The special "does not allow optional name" error is well meant, but
confusing in practice. Drop it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-12-armbru@redhat.com>
Split check_name() into check_name_is_str() and check_name_str(), keep
check_name() as a wrapper.
Move add_name()'s call into its caller check_exprs(), and inline.
This permits delaying check_name_str() there, so its error message
gains an "in definition" line.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-10-armbru@redhat.com>
We report name clashes like this:
struct-base-clash.json: In struct 'Sub':
struct-base-clash.json:5: 'name' (member of Sub) collides with 'name' (member of Base)
The "(member of Sub)" is redundant with "In struct 'Sub'". Comes from
QAPISchemaMember.describe(). Pass info to it, so it can detect the
redundancy and avoid it. Result:
struct-base-clash.json: In struct 'Sub':
struct-base-clash.json:5: member 'name' collides with member 'name' of type 'Base'
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-8-armbru@redhat.com>
Starting error messages with a capital letter complicates things when
text can get interpolated both at the beginning and in the middle of
an error message. The next patch will do that. Switch to lower case
to keep it simpler.
For what it's worth, the GNU Coding Standards advise the message
"should not begin with a capital letter when it follows a program name
and/or file name, because that isn’t the beginning of a sentence. (The
sentence conceptually starts at the beginning of the line.)"
While there, avoid breaking lines containing multiple arguments in the
middle of an argument.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-7-armbru@redhat.com>
QAPISchemaMember.check_clash() checks for member names that map to the
same c_name(). Takes care of rejecting duplicate names.
It also checks a naming rule: no uppercase in member names. That's a
rather odd place to do it. Enforcing naming rules is
check_name_str()'s job.
qapi-code-gen.txt specifies the name case rule applies to the name as
it appears in the schema. check_clash() checks c_name(name) instead.
No difference, as c_name() leaves alone case, but unclean.
Move the name case check into check_name_str(), less the c_name().
New argument @permit_upper suppresses it. Pass permit_upper=True for
definitions (which are not members), and when the member's owner is
whitelisted with pragma name-case-whitelist.
Bonus: name-case-whitelist now applies to a union's inline base, too.
Update qapi/qapi-schema.json pragma to whitelist union CpuInfo instead
of CpuInfo's implicit base type's name q_obj_CpuInfo-base.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-6-armbru@redhat.com>
We take pains to include the offending expression in error messages,
e.g.
tests/qapi-schema/alternate-any.json:2: alternate 'Alt' member 'one' cannot use type 'any'
But not always:
tests/qapi-schema/enum-if-invalid.json:2: 'if' condition must be a string or a list of strings
Instead of improving them one by one, report the offending expression
whenever it is known, like this:
tests/qapi-schema/enum-if-invalid.json: In enum 'TestIfEnum':
tests/qapi-schema/enum-if-invalid.json:2: 'if' condition must be a string or a list of strings
Error messages that mention the offending expression become a bit
redundant, e.g.
tests/qapi-schema/alternate-any.json: In alternate 'Alt':
tests/qapi-schema/alternate-any.json:2: alternate 'Alt' member 'one' cannot use type 'any'
I'll take care of that later in this series.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-5-armbru@redhat.com>
Commit 87adbbffd4..3e270dcacc "qapi: Add 'if' to (implicit
struct|union|alternate) members" (v4.0.0) neglected test coverage, and
promptly failed to check the conditions. Review fail.
Recent commit "tests/qapi-schema: Demonstrate insufficient 'if'
checking" added test coverage, demonstrating the bug. Fix it by add
the missing check_if().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-13-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
"'if': 'COND'" generates "#if COND". We reject empty COND because it
won't compile. Blank COND won't compile any better, so reject that,
too.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-12-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
check_union() checks the discriminator exists in base and makes sense.
Two error messages mention the base. These are broken for anonymous
bases, as demonstrated by tests flat-union-invalid-discriminator and
flat-union-invalid-if-discriminator.err. The third one doesn't
bother.
First broken when commit ac4338f8eb "qapi: Allow anonymous base for
flat union" (v2.6.0) neglected to adjust the "not a member of base"
error message. Commit ccadd6bcba "qapi: Add 'if' to implicit struct
members" (v4.0.0) then cloned the flawed error message.
Dumb them down not to mention the base.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-11-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
We represent the parse tree as OrderedDict. We fetch optional dict
members with .get(). So far, so good.
We represent null literals as None. .get() returns None both for
"absent" and for "present, value is the null literal". Uh-oh.
Test features-if-invalid exposes this bug: "'if': null" is
misinterpreted as absent "if".
We added null to the schema language to "allow [...] an explicit
default value" (commit e53188ada5 "qapi: Allow true, false and null in
schema json", v2.4.0). Hasn't happened; null is still unused except
as generic invalid value in tests/.
To fix, we'd have to replace .get() by something more careful, or
represent null differently. Feasible, but we got more and bigger fish
to fry right now. Remove the null literal from the schema language.
Replace null in tests by another invalid value.
Test features-if-invalid now behaves as it should.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-10-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Show text up to next structural character, whitespace, or quote
character instead of just the first character.
Forgotten quotes now get reported like "Stray 'command'" instead of
"Stray 'c'".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-9-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Consistently enclose error messages in double quotes. Use single
quotes within, except for one case of "'".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-8-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The error message for forgotten quotes around a name shows just the
name's first character, which isn't as nice as it could be. Same for
attempting to use a number.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-7-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Cover invalid 'if' in struct members, features, union and alternate
branches. Four out of four are broken. Mark FIXME.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-6-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Comment typo fixed]
When the union definition's base is an object, some error messages
show it as an OrderedDict. Oops. Mark FIXME.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-5-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Test flat-union-optional-discriminator declares its union tag as
'*switch': 'Enum', and points to it with 'discriminator': '*switch'.
This gets rejected as "discriminator of flat union 'MyUnion' uses
invalid name '*switch'". Correct; member 'discriminator' doesn't
accept a '*' prefix.
However, this merely tests name validity checking, which we already
cover elsewhere. More interesting is testing the valid name 'switch'.
This reports "discriminator 'switch' is not a member of base struct
'Base'", which is misleading.
Copy the existing 'discriminator': '*switch' test to
flat-union-discriminator-bad-name, and rewrite its comment. Change
flat-union-optional-discriminator to test 'discriminator': 'switch',
and mark it FIXME.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-4-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
