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>
Tests duplicate-key and double-data test the same thing. The former
predates the latter, and it has a better name. Delete the latter, and
tweak the former's comment.
Tests include-format-err and include-extra-junk test the same thing.
The former predates the latter, but the latter has a better name and a
comment. Delete the former.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-3-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The previous commit made qapi-code-gen.txt define "(top-level)
expression" as either "directive" or "definition". The code still
uses "expression" when it really means "definition". Tidy up.
The previous commit made qapi-code-gen.txt use "object" rather than
"dictionary". The code still uses "dictionary". Tidy up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-17-armbru@redhat.com>
For consistency with docs/devel/qapi-code-gen.txt.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-12-armbru@redhat.com>
Absent flat union branches default to the empty struct (since commit
800877bb16 "qapi: allow empty branches in flat unions"). But an
attempt to omit all of them is rejected with "Union 'FOO' has no
branches". Harmless oddity, but it's easy to avoid, so do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-11-armbru@redhat.com>
[Commit message typo fixed]
A union or alternate without branches makes no sense and doesn't work:
it can't be instantiated. A union or alternate with just one branch
works, but is degenerate. We accept the former, but reject the
latter. Weird. docs/devel/qapi-code-gen.txt doesn't mention the
difference. It claims an alternate definition is "is similar to a
simple union type".
Permit degenerate alternates to make them consistent with unions.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-10-armbru@redhat.com>
We reject empty types with 'boxed': true. We don't really need that
to work, but making it work is actually simpler than rejecting it, so
do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-9-armbru@redhat.com>
Since the previous commit restricted strings to printable ASCII,
\uXXXX's only use is obfuscation. Drop it.
This leaves \\, \/, \', and \". Since QAPI schema strings are all
names, and names are restricted to ASCII letters, digits, hyphen, and
underscore, none of them is useful.
The latter three have no test coverage. Drop them.
Keep \\ to avoid (more) gratuitous incompatibility with JSON.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-8-armbru@redhat.com>
RFC 8259 on string contents:
All Unicode characters may be placed within the quotation marks,
except for the characters that MUST be escaped: quotation mark,
reverse solidus, and the control characters (U+0000 through
U+001F).
The QAPI schema parser accepts both less and more than JSON: it
accepts only ASCII with \u (less), and accepts control characters
other than LF (new line) unescaped. How it treats unescaped non-ASCII
input differs between Python 2 and Python 3.
Make it accept strictly less: require printable ASCII. Drop support
for \b, \f, \n, \r, \t.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-7-armbru@redhat.com>
Invalid name 'not\\possible' is reported as 'not\possible'. Control
characters (quoted or not) are even more confusing. Mark FIXME.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-6-armbru@redhat.com>
Commands and events can define their argument type inline (default) or
by referring to another type ('boxed': true, since commit c818408e44
"qapi: Implement boxed types for commands/events", v2.7.0). The
unboxed inline definition is an (anonymous) struct type. The boxed
type may be a struct, union, or alternate type.
The latter is problematic: docs/interop/qemu-spec.txt requires the
value of the 'data' key to be a json-object, but any non-degenerate
alternate type has at least one branch that isn't.
Fortunately, we haven't made use of alternates in this context outside
tests/. Drop support for them.
QAPISchemaAlternateType.is_empty() is now unused. Drop it, too.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-4-armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20190606153803.5278-4-armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20190606153803.5278-3-armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Sometimes, the behaviour of QEMU changes without a change in the QMP
syntax (usually by allowing values or operations that previously
resulted in an error). QMP clients may still need to know whether
they can rely on the changed behavior.
Let's add feature flags to the QAPI schema language, so that we can make
such changes visible with schema introspection.
An example for a schema definition using feature flags looks like this:
{ 'struct': 'TestType',
'data': { 'number': 'int' },
'features': [ 'allow-negative-numbers' ] }
Introspection information then looks like this:
{ "name": "TestType", "meta-type": "object",
"members": [
{ "name": "number", "type": "int" } ],
"features": [ "allow-negative-numbers" ] }
This patch implements feature flags only for struct types. We'll
implement them more widely as needed.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20190606153803.5278-2-armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We generally put implicitly defined types in whatever module triggered
their definition. This is wrong for array types, as the included test
case demonstrates. Let's have a closer look at it.
Type 'Status' is defined sub-sub-module.json. Array type ['Status']
occurs in main module qapi-schema-test.json and in
include/sub-module.json. The main module's use is first, so the array
type gets put into the main module.
The generated C headers define StatusList in qapi-types.h. But
include/qapi-types-sub-module.h uses it without including
qapi-types.h. Oops.
To fix that, put the array type into its element type's module.
Now StatusList gets generated into qapi-types-sub-module.h, which all
its users include.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190301154051.23317-8-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The forward reference from the main module to the sub-module works
fine, except for an issue visible in qapi-schema-test.out: the array
type wrapped around the forward reference ends up in the main module,
not the sub-module. The next commit will explain why that's bad, and
fix it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190301154051.23317-7-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The lists in UserDefNativeListUnion aren't "native", they're lists of
built-in types. The next commit will add a list of a user-defined
type. Drop "Native", and adjust the tests using the type.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190301154051.23317-6-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The #include directives to pull in sub-modules use file names relative
to the main module. Works only when all modules are in the same
directory, or the main module's output directory is in the compiler's
include path. Use relative file names instead.
The dummy variable we generate to avoid empty .o files has an invalid
name for sub-modules in other directories. Fix that.
Both messed up in commit 252dc3105f "qapi: Generate separate .h, .c
for each module". Escaped testing because tests/qapi-schema-test.json
doesn't cover sub-modules in other directories, only
tests/qapi-schema/include-relpath.json does, and we generate and
compile C code only for the former, not the latter. Fold the latter
into the former. This would have caught the mistakes fixed in this
commit.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190301154051.23317-5-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Commit 967c885108 neglected to cover arrays of conditional types. Do
that now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190301154051.23317-3-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The next few commits mess with array types, and having the changes
exposed in output of test-qapi.py will be useful.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190301154051.23317-2-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Rationale added to commit message]
We neglect to call .visit_module() for the special module we use for
built-ins. Harmless, but clean it up anyway. The
tests/qapi-schema/*.out now show the built-in module as 'module None'.
Subclasses of QAPISchemaModularCVisitor need to ._add_module() this
special module to enable code generation for built-ins. When this
hasn't been done, QAPISchemaModularCVisitor.visit_module() does
nothing for the special module. That looks like built-ins could
accidentally be generated into the wrong module when a subclass
neglects to call ._add_module(). Can't happen, because built-ins are
all visited before any other module. But that's non-obvious. Switch
off code generation explicitly.
Rename QAPISchemaModularCVisitor._begin_module() to
._begin_user_module().
New QAPISchemaModularCVisitor._is_builtin_module(), for clarity.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20190214152251.2073-4-armbru@redhat.com>
Use a common function to generate the "If:..." line.
While at it, get rid of the existing \n\n (no idea why it was
there). Use a line-break in member description, this seems to look
slightly better in the plaintext version.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20181213123724.4866-19-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The generated code is for now *unconditional*. Later patches generate
the conditionals.
Note that union discriminators may not have 'if' conditionals.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20181213123724.4866-14-marcandre.lureau@redhat.com>
Message-Id: <20181213123724.4866-15-marcandre.lureau@redhat.com>
[Patches squashed, commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Wherever a struct/union/alternate/command/event member with NAME: TYPE
form is accepted, desugar it to a NAME: { 'type': TYPE } form.
This will allow to add new member details, such as 'if' in the
following patch to introduce conditionals, or 'default' for default
values etc.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20181213123724.4866-13-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
QAPISchemaMember gains .ifcond for enum members: inherited classes,
such as QAPISchemaObjectTypeMember, will thus have an ifcond member
after this (those different types will also use the .ifcond to store
the condition and generate conditional code in the following patches).
The generated code remains unconditional for now. Later patches
generate the conditionals.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20181213123724.4866-10-marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Desugar the enum NAME form to { 'name': NAME }. This will allow to add
new enum members, such as 'if' in the following patch.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20181213123724.4866-7-marcandre.lureau@redhat.com>
Message-Id: <20181213123724.4866-8-marcandre.lureau@redhat.com>
Message-Id: <20181213123724.4866-9-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Harmless accidental move backed out, long line wrapped, patches
squashed]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Report the set of missing or unknown keys. And give a hint about the
accepted keys.
The error message for multiple meta type members (visible in
tests/qapi-schema/double-type.err) is not improved.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20181213123724.4866-6-marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit 93bda4dd46 changed the internal representation of enum type
members from str to QAPISchemaMember, but we still print only a
string. Has been good enough, as the name is the member's only
attribute of interest, but that's about to change. To prepare, print
them more like object type members.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20181213123724.4866-4-marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
This will allow to add and access more properties associated with enum
values/members, like the associated 'if' condition. We may want to
have a specialized type QAPISchemaEnumMember, for now this will do.
Modify gen_enum() and gen_enum_lookup() for the same reason.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20181213123724.4866-3-marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Verify the usage of this schema feature and the API behaviour. This
should be the only case where qmp_dispatch() returns NULL.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Fix the following issues:
common.py:873:13: E129 visually indented line with same indent as next logical line
common.py:1766:5: E741 ambiguous variable name 'l'
common.py:1784:1: E305 expected 2 blank lines after class or function definition, found 1
common.py:1833:1: E305 expected 2 blank lines after class or function definition, found 1
common.py:1843:1: E305 expected 2 blank lines after class or function definition, found 1
visit.py:181:18: E127 continuation line over-indented for visual indent
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20180621083551.775-1-armbru@redhat.com>
[Fixup squashed in:]
Message-ID: <871sd0nzw9.fsf@dusky.pond.sub.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The documentation is generated only once, and doesn't know C
pre-conditions. Add 'If:' sections for top-level entities.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180703155648.11933-13-marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Modify the test visitor to check correct passing of values.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180703155648.11933-5-marcandre.lureau@redhat.com>
[Accidental change to roms/seabios dropped]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Accept 'if' key in top-level elements, accepted as string or list of
string type. The following patches will modify the test visitor to
check the value is correctly saved, and generate #if/#endif code (as a
single #if/endif line or a series for a list).
Example of 'if' key:
{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
'if': 'defined(TEST_IF_STRUCT)' }
The generated code is for now *unconditional*. Later patches generate
the conditionals.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180703155648.11933-2-marcandre.lureau@redhat.com>
[Commit message and Documentation improved]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Affects documentation and a few error messages.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-2-armbru@redhat.com>
It often happens that just a few discriminator values imply extra data in
a flat union. Existing checks did not make possible to leave other values
uncovered. Such cases had to be worked around by either stating a dummy
(empty) type or introducing another (subset) discriminator enumeration.
Both options create redundant entities in qapi files for little profit.
With this patch it is not necessary anymore to add designated union
fields for every possible value of a discriminator enumeration.
Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Message-Id: <1529311206-76847-2-git-send-email-anton.nefedov@virtuozzo.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
use new allow-preconfig parameter in tests and make sure that
the QAPISchema can parse allow-preconfig correctly
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1526058959-41425-1-git-send-email-imammedo@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
New option will be used to allow commands, which are prepared/need
to run, during preconfig state. Other commands that should be able
to run in preconfig state, should be amended to not expect machine
in initialized state or deal with it.
For compatibility reasons, commands that don't use new flag
'allow-preconfig' explicitly are not permitted to run in
preconfig state but allowed in all other states like they used
to be.
Within this patch allow following commands in preconfig state:
qmp_capabilities
query-qmp-schema
query-commands
query-command-line-options
query-status
exit-preconfig
to allow qmp connection, basic introspection and moving to the next
state.
PS:
set-numa-node and query-hotpluggable-cpus will be enabled later in
a separate patches.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <1526057503-39287-1-git-send-email-imammedo@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[ehabkost: Changed "since 2.13" to "since 3.0"]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
It simply tests the new OOB capability, and make sure the QAPISchema can
parse it correctly.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-7-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
The allow_oob parameter was passed in but not used in tests. Now
reflect that in the tests, so we need to touch up other command testers
with that new change.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-6-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Here "oob" stands for "Out-Of-Band". When "allow-oob" is set, it means
the command allows out-of-band execution.
The "oob" idea is proposed by Markus Armbruster in following thread:
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg02057.html
This new "allow-oob" boolean will be exposed by "query-qmp-schema" as
well for command entries, so that QMP clients can know which commands
can be used in out-of-band calls. For example the command "migrate"
originally looks like:
{"name": "migrate", "ret-type": "17", "meta-type": "command",
"arg-type": "86"}
And it'll be changed into:
{"name": "migrate", "ret-type": "17", "allow-oob": false,
"meta-type": "command", "arg-type": "86"}
This patch only provides the QMP interface level changes. It does not
contain the real out-of-band execution implementation yet.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-18-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: rebase on introspection done by qlit]
Signed-off-by: Eric Blake <eblake@redhat.com>
The include directive permits modular QAPI schemata, but the generated
code is monolithic all the same. To permit generating modular code,
the front end needs to pass more information on inclusions to the back
ends. The commit before last added the necessary information to the
parse tree. This commit adds it to the intermediate representation
and its QAPISchemaVisitor. A later commit will use this to to
generate modular code.
New entity QAPISchemaInclude represents inclusions. Call new visitor
method visit_include() for it, so visitors can see the sub-modules a
module includes.
Note that unlike other entities, QAPISchemaInclude has no name, and is
therefore not added to entity_dict.
New QAPISchemaEntity attribute @module names the entity's source file.
Call new visitor method visit_module() when it changes during a visit,
so visitors can keep track of the module being visited.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180211093607.27351-18-armbru@redhat.com>
[eblake: avoid accidental deletion of self._predefining]
Signed-off-by: Eric Blake <eblake@redhat.com>
The generators' conversion to visitors (merge commit 9e72681d16)
changed the processing order of entities from source order to
alphabetical order. The next commit needs source order, so change it
back.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180211093607.27351-17-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Error messages print absolute file names of included files even if the
user gave a relative one on the command line:
$ PYTHONPATH=scripts python -B tests/qapi-schema/test-qapi.py tests/qapi-schema/include-cycle.json
In file included from tests/qapi-schema/include-cycle.json:1:
In file included from /work/armbru/qemu/tests/qapi-schema/include-cycle-b.json:1:
/work/armbru/qemu/tests/qapi-schema/include-cycle-c.json:1: Inclusion loop for include-cycle.json
Improve this to
In file included from tests/qapi-schema/include-cycle.json:1:
In file included from tests/qapi-schema/include-cycle-b.json:1:
tests/qapi-schema/include-cycle-c.json:1: Inclusion loop for include-cycle.json
The error message when an include file can't be opened prints the
include directive's file name, which is relative to the including
file. Change this to print the file name relative to the working
directory. Visible in tests/qapi-schema/include-no-file.err.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180211093607.27351-12-armbru@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Whenever qapi-schema.json changes, we run six programs eleven times to
update eleven files. Similar for qga/qapi-schema.json. This is
silly. Replace the six programs by a single program that spits out
all eleven files.
The programs become modules in new Python package qapi, along with the
helper library. This requires moving them to scripts/qapi/. While
moving them, consistently drop executable mode bits.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180211093607.27351-9-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
[eblake: move change to one-line 'blurb' earlier in series, mention mode
bit change as intentional, update qapi-code-gen.txt to match actual
generated events.c file]
Signed-off-by: Eric Blake <eblake@redhat.com>
Every generator has separate boilerplate for .h and .c, and their
differences are boring. All of them repeat the license note.
Reduce the repetition as follows. Move common text like the license
note to common open_output(), next to the existing common text there.
For each generator, replace the two separate descriptions by a single
one.
While there, emit an "automatically generated" note into generated
documentation, too.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180211093607.27351-3-armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
The iteritems()/itervalues() methods are gone in py3, but the
items()/values() methods are still around. The latter are less
efficient than the former in py2, but this has unmeasurably
small impact on QEMU build time, so taking portability over
efficiency is a net win.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <20180116134217.8725-3-berrange@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Python 3 no longer supports the bare "print" statement, it must be
called as a normal function with round brackets. It is possible to
opt-in to this new syntax with Python 2.6 onwards by importing the
"print_function" from the "__future__" module, making it easy to
support Python 2 and 3 in parallel.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <20180116134217.8725-2-berrange@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Use a string instead of a list of strings. While there, generate
fewer superfluous blank lines.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20171002141341.24616-10-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Use a string instead of a list of strings.
This makes qapi2texi.py generate additional blank lines. They're
harmless, and the next commit will get rid of them again.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20171002141341.24616-9-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
We have two representations of sections without a name: the main
section uses name=None, the others name=''. Standardize on name=None.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20171002141341.24616-8-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
A negative test case crept into doc-good.json: invalid use of section
markup we currently fail to reject. Move this into its own
doc-bad-section.json.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20171002141341.24616-6-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The conflict check added by commit c0644771 ("qapi: Reject
alternates that can't work with keyval_parse()") doesn't work
with the following declaration:
{ 'alternate': 'Alt',
'data': { 'one': 'bool',
'two': 'str' } }
It crashes with:
Traceback (most recent call last):
File "./scripts/qapi-types.py", line 295, in <module>
schema = QAPISchema(input_file)
File "/home/ehabkost/rh/proj/virt/qemu/scripts/qapi.py", line 1468, in __init__
self.exprs = check_exprs(parser.exprs)
File "/home/ehabkost/rh/proj/virt/qemu/scripts/qapi.py", line 958, in check_exprs
check_alternate(expr, info)
File "/home/ehabkost/rh/proj/virt/qemu/scripts/qapi.py", line 830, in check_alternate
% (name, key, types_seen[qtype]))
KeyError: 'QTYPE_QSTRING'
This happens because the previously-seen conflicting member
('one') can't be found at types_seen[qtype], but at
types_seen['QTYPE_BOOL'].
Fix the bug by moving the error check to the same loop that adds
new items to types_seen, raising an exception if types_seen[qt]
is already set.
Add two additional test cases that can detect the bug.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <20170717180926.14924-1-ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
I expect the 'null' type to be useful mostly for members of alternate
types.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We would like to use a same QObject type to represent numbers, whether
they are int, uint, or floats. Getters will allow some compatibility
between the various types if the number fits other representations.
Add a few more tests while at it.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20170607163635.17635-7-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[parse_stats_intervals() simplified a bit, comment in
test_visitor_in_int_overflow() tidied up, suppress bogus warnings]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Alternates with both a 'number' and an 'int' branch will become
invalid when the next patch merges of QFloat and QInt into QNum.
More sophisticated alternate code could keep them valid, but since
we have no users outside tests, simply drop the tests.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20170607163635.17635-4-marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Alternates are sum types like unions, but use the JSON type on the
wire / QType in QObject instead of an explicit tag. That's why we
require alternate members to have distinct QTypes.
The recently introduced keyval_parse() (commit d454dbe) can only
produce string scalars. The qobject_input_visitor_new_keyval() input
visitor mostly hides the difference, so code using a QObject input
visitor doesn't have to care whether its input was parsed from JSON or
KEY=VALUE,... The difference leaks for alternates, as noted in commit
0ee9ae7: a non-string, non-enum scalar alternate value can't currently
be expressed.
In part, this is just our insufficiently sophisticated implementation.
Consider alternate type 'GuestFileWhence'. It has an integer member
and a 'QGASeek' member. The latter is an enumeration with values
'set', 'cur', 'end'. The meaning of b=set, b=cur, b=end, b=0, b=1 and
so forth is perfectly obvious. However, our current implementation
falls apart at run time for b=0, b=1, and so forth. Fixable, but not
today; add a test case and a TODO comment.
Now consider an alternate type with a string and an integer member.
What's the meaning of a=42? Is it the string "42" or the integer 42?
Whichever meaning you pick makes the other inexpressible. This isn't
just an implementation problem, it's fundamental. Our current
implementation will pick string.
So far, we haven't needed such alternates. To make sure we stop and
think before we add one that cannot sanely work with keyval_parse(),
let's require alternate members to have sufficiently distinct
representation in KEY=VALUE,... syntax:
* A string member clashes with any other scalar member
* An enumeration member clashes with bool members when it has value
'on' or 'off'.
* An enumeration member clashes with numeric members when it has a
value that starts with '-', '+', or a decimal digit. This is a
rather lazy approximation of the actual number syntax accepted by
the visitor.
Note that enumeration values starting with '-' and '+' are rejected
elsewhere already, but better safe than sorry.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1495471335-23707-5-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The next commit is going to make alternate members of type 'str'
conflict with other scalar types. Would break a few test cases that
don't actually require 'str'. Flip them from 'str' to 'bool' or
'EnumOne'.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1495471335-23707-4-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
We have a number of negative tests, but we don't have systematic
positive coverage. Fix that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1490015515-25851-6-git-send-email-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
test-qapi.py used to print the internal representation of doc comments
(commit 3313b61). This went away when we dropped the doc comments in
positive tests (commit 87c16dc). Bring it back, because I'm going to
add real positive doc comment tests.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1490015515-25851-5-git-send-email-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
When choking on a token where an expression is expected, we report
'Expected "{", "[" or string'. Close, but no cigar. Fix it to
Expected '"{", "[", string, boolean or "null"'.
Missed in commit e53188a.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-48-git-send-email-armbru@redhat.com>
check_definition_doc() checks for member documentation without a
matching member. It laboriously second-guesses what members
QAPISchema._def_exprs() will create. That's a stupid game.
Move the check into QAPISchema.check(), where the members are known.
Delegate the actual checking to new QAPIDoc.check().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-38-git-send-email-armbru@redhat.com>
New test doc-bad-union-member.json shows we can fail to reject
documentation for nonexistent members.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-37-git-send-email-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-36-git-send-email-armbru@redhat.com>
Results in a more precise error location, but the real reason is
emptying out check_docs() step by step.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-35-git-send-email-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-34-git-send-email-armbru@redhat.com>
Move the check whether the doc matches the expression name from
check_definition_doc() to check_exprs(). This changes the error
location from the comment to the expression. Makes sense as the
message talks about the expression: "Definition of '%s' follows
documentation for '%s'". It's also a step towards getting rid of
check_docs().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-33-git-send-email-armbru@redhat.com>
This fixes the errors uncovered by the previous commit.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-32-git-send-email-armbru@redhat.com>
New tests doc-before-include.json and doc-before-pragma.json show we
fail to reject a misplaced expression comment.
New test doc-no-symbol.json shows a bad error message.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1489582656-31133-31-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
We traditionally mark optional members #optional in the doc comment.
Before commit 3313b61, this was entirely manual.
Commit 3313b61 added some automation because its qapi2texi.py relied
on #optional to determine whether a member is optional. This is no
longer the case since the previous commit: the only thing qapi2texi.py
still does with #optional is stripping it out. We still reject bogus
qapi-schema.json and six places for qga/qapi-schema.json.
Thus, you can't actually rely on #optional to see whether something is
optional. Yet we still make people add it manually. That's just
busy-work.
Drop the code to check, fix up and strip out #optional, along with all
instances of #optional. To keep it out, add code to reject it, to be
dropped again once the dust settles.
No change to generated documentation.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-18-git-send-email-armbru@redhat.com>
Common Python pitfall: 'assert base_members' fires on [] in addition
to None. Correct to 'assert base_members is not None'.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-10-git-send-email-armbru@redhat.com>
The new test case shows off qapi.py choking on an empty union base.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-9-git-send-email-armbru@redhat.com>
qapi.py has a hardcoded white-list of type names that may violate the
rule on use of upper and lower case. Add a new pragma directive
'name-case-whitelist', and use it to replace the hard-coded
white-list.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1489582656-31133-7-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
qapi.py has a hardcoded white-list of command names that may violate
the rules on permitted return types. Add a new pragma directive
'returns-whitelist', and use it to replace the hard-coded white-list.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1489582656-31133-6-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
This reverts commit 3313b61's changes to tests/qapi-schema/, except
for tests/qapi-schema/doc-*.
We could keep some of these doc comments to serve as positive test
cases. However, they don't actually add to what we get from doc
comment use in actual schemas, as we we don't test output matches
expectations, and don't systematically cover doc comment features.
Proper positive test coverage would be nice.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-4-git-send-email-armbru@redhat.com>
Since we added the documentation generator in commit 3313b61, doc
comments are mandatory. That's a very good idea for a schema that
needs to be documented, but has proven to be annoying for testing.
Make doc comments optional again, but add a new directive
{ 'pragma': { 'doc-required': true } }
to let a QAPI schema require them.
Add test cases for the new pragma directive. While there, plug a
minor hole in includ directive test coverage.
Require documentation in the schemas we actually want documented:
qapi-schema.json and qga/qapi-schema.json.
We could probably make qapi2texi.py cope with incomplete
documentation, but for now, simply make it refuse to run unless the
schema has 'doc-required': true.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1489582656-31133-3-git-send-email-armbru@redhat.com>
[qapi-code-gen.txt wording tweaked]
Reviewed-by: Eric Blake <eblake@redhat.com>
As the name suggests, the qapi2texi script converts JSON QAPI
description into a texi file suitable for different target
formats (info/man/txt/pdf/html...).
It parses the following kind of blocks:
Free-form:
##
# = Section
# == Subsection
#
# Some text foo with *emphasis*
# 1. with a list
# 2. like that
#
# And some code:
# | $ echo foo
# | -> do this
# | <- get that
#
##
Symbol description:
##
# @symbol:
#
# Symbol body ditto ergo sum. Foo bar
# baz ding.
#
# @param1: the frob to frobnicate
# @param2: #optional how hard to frobnicate
#
# Returns: the frobnicated frob.
# If frob isn't frobnicatable, GenericError.
#
# Since: version
# Notes: notes, comments can have
# - itemized list
# - like this
#
# Example:
#
# -> { "execute": "quit" }
# <- { "return": {} }
#
##
That's roughly following the following EBNF grammar:
api_comment = "##\n" comment "##\n"
comment = freeform_comment | symbol_comment
freeform_comment = { "# " text "\n" | "#\n" }
symbol_comment = "# @" name ":\n" { member | tag_section | freeform_comment }
member = "# @" name ':' [ text ] "\n" freeform_comment
tag_section = "# " ( "Returns:", "Since:", "Note:", "Notes:", "Example:", "Examples:" ) [ text ] "\n" freeform_comment
text = free text with markup
Note that the grammar is ambiguous: a line "# @foo:\n" can be parsed
both as freeform_comment and as symbol_comment. The actual parser
recognizes symbol_comment.
See docs/qapi-code-gen.txt for more details.
Deficiencies and limitations:
- the generated QMP documentation includes internal types
- union type support is lacking
- type information is lacking in generated documentation
- doc comment error message positions are imprecise, they point
to the beginning of the comment.
- a few minor issues, all marked TODO/FIXME in the code
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20170113144135.5150-16-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[test-qapi.py tweaked to avoid trailing empty lines in .out]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Turn on the ability to pass command and event arguments in
a single boxed parameter, which must name a non-empty type
(although the type can be a struct with all optional members).
For structs, it makes it possible to pass a single qapi type
instead of a breakout of all struct members (useful if the
arguments are already in a struct or if the number of members
is large); for other complex types, it is now possible to use
a union or alternate as the data for a command or event.
The empty type may be technically feasible if needed down the
road, but it's easier to forbid it now and relax things to allow
it later, than it is to allow it now and have to special case
how the generated 'q_empty' type is handled (see commit 7ce106a9
for reasons why nothing is generated for the empty type). An
alternate type is never considered empty, but now that a boxed
type can be either an object or an alternate, we have to provide
a trivial QAPISchemaAlternateType.is_empty(). The new call to
arg_type.is_empty() during QAPISchemaCommand.check() requires
that we first check the type in question; but there is no chance
of introducing a cycle since objects do not refer back to commands.
We still have a split in syntax checking between ad-hoc parsing
up front (merely validates that 'boxed' has a sane value) and
during .check() methods (if 'boxed' is set, then 'data' must name
a non-empty user-defined type).
Generated code is unchanged, as long as no client uses the
new feature.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1468468228-27827-10-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Test files renamed to *-boxed-*]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The next patch will add support for passing a qapi union type
as the 'data' of a command. But to do that, the user function
for implementing the command, as called by the generated
marshal command, must take the corresponding C struct as a
single boxed pointer, rather than a breakdown into one
parameter per member. Even without a union, being able to use
a C struct rather than a list of parameters can make it much
easier to handle coding with QAPI.
This patch adds the internal plumbing of a 'boxed' flag
associated with each command and event. In several cases,
this means adding indentation, with one new dead branch and
the remaining branch being the original code more deeply
nested; this was done so that the new implementation in the
next patch is easier to review without also being mixed with
indentation changes.
For this patch, no behavior or generated output changes, other
than the testsuite outputting the value of the new flag
(always False for now).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1468468228-27827-9-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Identifier box renamed to boxed in two places]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Clean up the only remaining external use of the tag_name field of
QAPISchemaObjectTypeVariants, by explicitly listing the generated
'type' tag for all variants in the testsuite (you can still tell
simple unions by the -wrapper types). Then we can mark the
tag_name field as private by adding a leading underscore to prevent
any further use.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1468468228-27827-5-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We were previously enforcing that all flat union branches were
found in the corresponding enum, but not that all enum values
were covered by branches. The resulting generated code would
abort() if the user passes the uncovered enum value.
We don't automatically treat non-present branches in a flat
union as empty types, for symmetry with simple unions (there,
the enum type is generated from the list of all branches, so
there is no way to omit a branch but still have it be part of
the union).
A later patch will add shorthand so that branches that are empty
in flat unions can be declared as 'branch':{} instead of
'branch':'Empty', to avoid the need for an otherwise useless
explicit empty type. [Such shorthand for simple unions is a bit
harder to justify, since we would still have to generate a
wrapper type that parses 'data':{}, rather than truly being an
empty branch with no additional siblings to the 'type' member.]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1468468228-27827-3-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Rather than requiring all flat unions to explicitly create
a separate base struct, we can allow the qapi schema to specify
the common members via an inline dictionary. This is similar to
how commands can specify an inline anonymous type for its 'data'.
We already have several struct types that only exist to serve as
a single flat union's base; the next commit will clean them up.
In particular, this patch's change to the BlockdevOptions example
in qapi-code-gen.txt will actually be done in the real QAPI schema.
Now that anonymous bases are legal, we need to rework the
flat-union-bad-base negative test (as previously written, it
forms what is now valid QAPI; tweak it to now provide coverage
of a new error message path), and add a positive test in
qapi-schema-test to use an anonymous base (making the integer
argument optional, for even more coverage).
Note that this patch only allows anonymous bases for flat unions;
simple unions are already enough syntactic sugar that we do not
want to burden them further. Meanwhile, while it would be easy
to also allow an anonymous base for structs, that would be quite
redundant, as the members can be put right into the struct
instead.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-15-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The original choice of ':obj-' as the prefix for implicit types
made it obvious that we weren't going to clash with any user-defined
names, which cannot contain ':'. But now we want to create structs
for implicit types, to get rid of special cases in the generators,
and our use of ':' in implicit names needs a tweak to produce valid
C code.
We could transliterate ':' to '_', except that C99 mandates that
"identifiers that begin with an underscore are always reserved for
use as identifiers with file scope in both the ordinary and tag name
spaces". So it's time to change our naming convention: we can
instead use the 'q_' prefix that we reserved for ourselves back in
commit 9fb081e0. Technically, since we aren't planning on exposing
the empty type in generated code, we could keep the name ':empty',
but renaming it to 'q_empty' makes the check for startswith('q_')
cover all implicit types, whether or not code is generated for them.
As long as we don't declare 'empty' or 'obj' ticklish, it shouldn't
clash with c_name() prepending 'q_' to the user's ticklish names.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-5-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The generator special-cased
{ 'command':'foo', 'data': {} }
to avoid emitting a visitor variable, but failed to see that
{ 'struct':'NamedEmptyType, 'data': {} }
{ 'command':'foo', 'data':'NamedEmptyType' }
needs the same treatment. There, the generator happily generates a
visitor to get no arguments, and a visitor to destroy no arguments;
and the compiler isn't happy with that, as demonstrated by the updated
qapi-schema-test.json:
tests/test-qmp-marshal.c: In function ‘qmp_marshal_user_def_cmd0’:
tests/test-qmp-marshal.c:264:14: error: variable ‘v’ set but not used [-Werror=unused-but-set-variable]
Visitor *v;
^
No change to generated code except for the testsuite addition.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We started moving away from the use of the 'void *data' member
in the C union corresponding to a QAPI union back in commit
544a373; recent commits have gotten rid of other uses. Now
that it is completely unused, we can remove the member itself
as well as the FIXME comment. Update the testsuite to drop the
negative test union-clash-data.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1457021813-10704-11-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
C types and JSON objects don't have fields, but members. We
shouldn't gratuitously invent terminology. This patch is a
strict renaming of generator code internals (including testsuite
comments), before later patches rename C interfaces.
No change to generated code with this patch.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1457021813-10704-2-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Upcoming patches will adjust how we visit an object branch of an
alternate; but we were completely lacking testsuite coverage.
Rectify this, so that the future patches will be able to highlight
the changes and still prove that we avoided regressions.
In particular, the use of a flat union UserDefFlatUnion rather
than a simple struct UserDefA as the branch will give us coverage
of an object with variants. And visiting an alternate as both
the top level and as a nested member gives confidence in correct
memory allocation handling, especially if the test is run under
valgrind.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-5-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The whole point of an alternate is to allow some type-safety while
still accepting more than one JSON type. Meanwhile, the 'any'
type exists to bypass type-safety altogether. The two are
incompatible: you can't accept every type, and still tell which
branch of the alternate to use for the parse; fix this to give a
sane error instead of a Python stack trace.
Note that other types that can't be alternate members are caught
earlier, by check_type().
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-4-git-send-email-eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Empty unions serve no purpose, and while we compile with gcc
which permits them, strict C99 forbids them. We happen to inject
a dummy 'void *data' member into the C unions that represent QAPI
unions and alternates, but we want to get rid of that member (it
pollutes the namespace for no good reason), which would leave us
with an empty union if the user didn't provide any branches. While
empty structs make sense in QAPI, empty unions don't add any
expressiveness to the QMP language. So prohibit them at parse
time. Update the documentation and testsuite to match.
Note that the documentation already mentioned that alternates
should have "two or more JSON data types"; so this also fixes
the code to enforce that. However, we have existing uses of a
union type with only one branch, so the 2-or-more strictness
is intentionally limited to alternates.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
When we added support for a user-specified prefix for an enum
type (commit 351d36e), we forgot to teach the qapi-visit code
to honor that prefix in the case of using a prefixed enum as
the discriminator for a flat union. While there is still some
on-list debate on whether we want to keep prefixes, we should
at least make it work as long as it is still part of the code
base.
Reported-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455665965-27638-1-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
It should be fairly obvious that qapi base classes need to
form an acyclic graph, since QMP cannot specify the same
key more than once, while base classes are included as flat
members alongside other members added by the child. But the
old check_member_clash() parser function was not prepared to
check for this, and entered an infinite recursion (at least
until Python gives up, complaining about nesting too deep).
Now that check_member_clash() has been recently removed,
attempts at self-inheritance trigger an assertion failure
introduced by commit ac88219a. The obvious fix is to turn
the assertion into a conditional.
This patch includes both the tests (base-cycle-direct and
base-cycle-indirect) and the fix, since the .err file output
for the unfixed case is not useful (particularly when it was
warning about unbounded recursion, as that limit may be
platform-specific).
We don't need to worry about cycles in flat unions (neither
the base type nor the type of a variant can be a union) nor
in alternates (alternate branches cannot themselves be an
alternate). But if we later allow a union type as a variant,
we will still be okay, as QAPISchemaObjectTypeVariants.check()
triggers the same QAPISchemaObjectType.check() that will
detect any loops.
Likewise, we need not worry about the case of diamond
inheritance where the same class is used for a flat union base
class and one of its variants; either both uses will introduce
a collision in trying to insert the same member name twice, or
the shared type is empty and changes nothing.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-16-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
