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>
Commit fbf09a2fa4 "qapi: add 'ifcond' to visitor methods" brought back
the executable bits. Fix that. Drop the #! line for good measure.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20191018074345.24034-8-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]
The next commit will split up qapi/common.py. gen_enum() needs
QAPISchemaEnumMember, and that's in the way. Move it to qapi/types.py
along with its buddy gen_enum_lookup().
Permit me a short a digression on history: how did gen_enum() end up
in qapi/common.py? Commit 21cd70dfc1 "qapi script: add event support"
duplicated qapi-types.py's gen_enum() and gen_enum_lookup() in
qapi-event.py. Simply importing them would have been cleaner, but
wasn't possible as qapi-types.py was a program, not a module. Commit
efd2eaa6c2 "qapi: De-duplicate enum code generation" de-duplicated by
moving them to qapi.py, which was a module.
Since then, program qapi-types.py has morphed into module types.py.
It's where gen_enum() and gen_enum_lookup() started, and where they
belong.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20191018074345.24034-6-armbru@redhat.com>
The frontend can't be run more than once due to its global state.
A future commit will want to do that.
The only global frontend state remaining is accidental:
QAPISchemaParser.__init__()'s parameter previously_included=[].
Python evaluates the default once, at definition time. Any
modifications to it are visible in subsequent calls. Well-known
Python trap. Change the default to None and replace it by the real
default in the function body. Use the opportunity to convert
previously_included to a set.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20191018074345.24034-4-armbru@redhat.com>
The frontend can't be run more than once due to its global state.
A future commit will want to do that.
Recent commit "qapi: Move context-sensitive checking to the proper
place" got rid of many global variables already, but pragma state is
still stored in global variables (that's why a pragma directive's
scope is the complete schema).
Move the pragma state to QAPISourceInfo.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20191018074345.24034-3-armbru@redhat.com>
Commit bc52d03ff5 "qapi: Make doc comments optional where we don't
need them" made scripts/qapi2texi.py fail[*] unless the schema had
pragma 'doc-required': true. The stated reason was inability to cope
with incomplete documentation.
When commit fb0bc835e5 "qapi-gen: New common driver for code and doc
generators" folded scripts/qapi2texi.py into scripts/qapi-gen.py, it
turned the failure into silent suppression.
The doc generator can cope with incomplete documentation now. I don't
know since when, or what the problem was, or even whether it ever
existed.
Drop the silent suppression.
[*] The fail part was broken, fixed in commit e8ba07ea9a.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20191018074345.24034-2-armbru@redhat.com>
The following statement produces a SyntaxWarning with Python 3.8:
if len(format) is 0:
scripts/tracetool/__init__.py:459: SyntaxWarning: "is" with a literal. Did you mean "=="?
Use the conventional len(x) == 0 syntax instead.
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20191010122154.10553-1-stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
The `make efi` target added by 536d2173 is built from the roms/edk2
submodule, which in turn relies on additional submodules nested under
roms/edk2.
The make-release script currently only pulls in top-level submodules,
so these nested submodules are missing in the resulting tarball.
We could try to address this situation more generally by recursively
pulling in all submodules, but this doesn't necessarily ensure the
end-result will build properly (this case also required other changes).
Additionally, due to the nature of submodules, we may not always have
control over how these sorts of things are dealt with, so for now we
continue to handle it on a case-by-case in the make-release script.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Bruce Rogers <brogers@suse.com>
Cc: qemu-stable@nongnu.org # v4.1.0
Reported-by: Bruce Rogers <brogers@suse.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Message-Id: <20190912231202.12327-2-mdroth@linux.vnet.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
The low bits are 1 if the control must be one, the high bits
are 1 if the control can be one. Correct the variable names
as they are very confusing.
Signed-off-by: Paolo Bonzini <pbonzini@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_keys() has become a trivial wrapper for check_known_keys().
Eliminate it.
This makes its name available. Rename check_known_keys().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-24-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>
check_name() consists of check_name_is_str() and check_name_str().
check_union() relies on the latter to catch optional discriminators.
The next commit will replace that by a more straightforward check.
Inlining check_name() into check_union() now should make that easier
to review.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-16-armbru@redhat.com>
Future commits will need info in the .check() methods of
QAPISchemaMember and its descendants. Get it there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-15-armbru@redhat.com>
check_type() checks the array's contents, then peels off the array and
falls through to the "not array" code without resetting allow_array
and allow_dict to False. Works because the peeled value is a string,
and allow_array and allow_dict aren't used then. Tidy up anyway:
recurse instead, defaulting allow_array and allow_dict to False.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-14-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>
Replace check_name() by check_name_str() where the name is known to be
a string.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-11-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>
Most check_FOO() take the thing being checked as first argument.
check_name(), check_type(), check_known_keys() don't. Clean that up.
While there, drop a "Todo" comment that should have been dropped in
commit 87adbbffd4 "qapi: add a dictionary form for TYPE".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-9-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>
We track source locations with a dict of the form
{'file': FNAME, 'line': LINENO, 'parent': PARENT}
where PARENT is None for the main file, and the include directive's
source location for included files.
This is serviceable enough, but the next commit will add information,
and that's going to come out cleaner if we turn this into a class. So
do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-4-armbru@redhat.com>
QAPISchemaMember.owner is the name of the defining entity. That's a
confusing name when an object type inherits members from a base type.
Rename it to .defined_in. Rename .set_owner() and ._pretty_owner() to
match.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-3-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(). check_exprs() still checks and reports errors,
and the .check() assert check_exprs() did the job. There are a few
gaps, though.
QAPISchemaArrayType.check() neglects to assert the element type is not
an array. Add the assertion.
QAPISchemaObjectTypeVariants.check() neglects to assert the tag member
is not optional. Add the assertion.
It neglects to assert the tag member is not conditional. Add the
assertion.
It neglects to assert we actually have variants. Add the assertion.
It asserts the variants are object types, but neglects to assert they
don't have variants. Tighten the assertion.
QAPISchemaObjectTypeVariants.check_clash() has the same issue.
However, it can run only after .check(). Delete the assertion instead
of tightening it.
QAPISchemaAlternateType.check() neglects to assert the branch types
don't conflict. Fixing that isn't trivial, so add just a TODO comment
for now. It'll be resolved later in this series.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-2-armbru@redhat.com>
Easy since the previous commit provides .checked.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-20-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Entity checking goes back to commit ac88219a6c "qapi: New QAPISchema
intermediate representation", v2.5.0. It's designed to work as
follows: QAPISchema.check() calls .check() for all the schema's
entities. An entity's .check() recurses into another entity's
.check() only if the C struct generated for the former contains the C
struct generated for the latter (pointers don't count). This is used
to detect "object contains itself".
There are two instances of this:
* An object's C struct contains its base's C struct
QAPISchemaObjectType.check() calls self.base.check()
* An object's C struct contains its variants' C structs
QAPISchemaObjectTypeVariants().check calls v.type.check(). Since
commit b807a1e1e3 "qapi: Check for QAPI collisions involving variant
members", v2.6.0.
Thus, only object types can participate in recursion.
QAPISchemaObjectType.check() is made for that: it checks @self when
called the first time, recursing into base and variants, it reports an
"contains itself" error when this recursion reaches an object being
checked, and does nothing it reaches an object that has been checked
already.
The other .check() may safely assume they get called exactly once.
Sadly, this design has since eroded:
* QAPISchemaCommand.check() and QAPISchemaEvent.check() call
.args_type.check(). Since commit c818408e44 "qapi: Implement boxed
types for commands/events", v2.7.0. Harmless, since args_type can
only be an object type.
* QAPISchemaEntity.check() calls ._ifcond.check() when inheriting the
condition from another type. Since commit 4fca21c1b0 qapi: leave
the ifcond attribute undefined until check(), v3.0.0. This makes
simple union wrapper types recurse into the wrapped type (nothing
else uses this condition inheritance). The .check() of types used
as simple union branch type get called multiple times.
* QAPISchemaObjectType.check() calls its super type's .check()
*before* the conditional handling multiple calls. Also since commit
4fca21c1b0. QAPISchemaObjectType.check()'s guard against multiple
checking doesn't protect QAPISchemaEntity.check().
* QAPISchemaArrayType.check() calls .element_type.check(). Also since
commit 4fca21c1b0. The .check() of types used as array element
types get called multiple times.
Commit 56a4689582 "qapi: Fix array first used in a different module"
(v4.0.0) added more code relying on this .element_type.check().
The absence of explosions suggests the .check() involved happen to be
effectively idempotent.
Fix the unwanted recursion anyway:
* QAPISchemaCommand.check() and QAPISchemaEvent.check() calling
.args_type.check() is unnecessary. Delete the calls.
* Fix QAPISchemaObjectType.check() to call its super type's .check()
after the conditional handling multiple calls.
* A QAPISchemaEntity's .ifcond becomes valid at .check(). This is due
to arrays and simple unions.
Most types get ifcond and info passed to their constructor.
Array types don't: they get it from their element type, which
becomes known only in .element_type.check().
The implicit wrapper object types for simple union branches don't:
they get it from the wrapped type, which might be an array.
Ditch the idea to set .ifcond in .check(). Instead, turn it into a
property and compute it on demand. Safe because it's only used
after the schema has been checked.
Most types simply return the ifcond passed to their constructor.
Array types forward to their .element_type instead, and the wrapper
types forward to the wrapped type.
* A QAPISchemaEntity's .module becomes valid at .check(). This is
because computing it needs info and schema.fname, and because array
types get it from their element type instead.
Make it a property just like .ifcond.
Additionally, have QAPISchemaEntity.check() assert it gets called at
most once, so the design invariant will stick this time. Neglecting
that was entirely my fault.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-19-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message tidied up]
QAPISchemaObjectType.check() does nothing for types that have been
checked already. Except the "has been checked" predicate is broken
for empty types: self.members is [] then, which isn't true. The bug
is harmless, but fix it anyway: use self.member is not None instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-18-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Commit bceae7697f "qapi script: support enum type as discriminator in
union" made check_exprs() add the implicit enum types of simple unions
to global @enum_types. I'm not sure it was needed even then. It's
certainly not needed now. Delete it.
discriminator_find_enum_define() and add_name() parameter @implicit
are now dead. Bury them.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-17-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
All callers pass a dict argument to @keys, except check_keys() passes
a dict's .keys(). Drop .keys() there, and rename parameter @keys to
@value.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-16-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
check_keys() parameter expr_elem expects a dictionary with keys 'expr'
and 'info'. Passing the two values separately is simpler, so do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-15-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
We normalize shorthand to longhand forms in check_expr(): enumeration
values with normalize_enum(), feature values with
normalize_features(), struct members, union branches and alternate
branches with normalize_members(). If conditions are an exception: we
normalize them in QAPISchemaEntity.check() and
QAPISchemaMember.__init(), with listify_cond(). The idea goes back to
commit 2cbc94376e "qapi: pass 'if' condition into QAPISchemaEntity
objects", v3.0.0.
Normalize in check_expr() instead, with new helper normalize_if().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-14-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@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 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>
