summaryrefslogtreecommitdiff
path: root/scripts
AgeCommit message (Collapse)Author
2019-10-22qapi: Add feature flags to commandsPeter Krempa
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 6a8c0b51025. 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>
2019-10-22qapi: Clear scripts/qapi/doc.py executable bits againMarkus Armbruster
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>
2019-10-22qapi: Split up scripts/qapi/common.pyMarkus Armbruster
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]
2019-10-22qapi: Move gen_enum(), gen_enum_lookup() back to qapi/types.pyMarkus Armbruster
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>
2019-10-22qapi: Eliminate accidental global frontend stateMarkus Armbruster
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>
2019-10-22qapi: Store pragma state in QAPISourceInfo, not global stateMarkus Armbruster
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>
2019-10-22qapi: Don't suppress doc generation without pragma doc-requiredMarkus Armbruster
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>
2019-10-15trace: avoid "is" with a literal Python 3.8 warningsStefan Hajnoczi
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>
2019-10-07make-release: pull in edk2 submodules so we can build it from tarballsMichael Roth
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>
2019-10-04vmxcap: correct the name of the variablesPaolo Bonzini
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>
2019-09-28qapi: Improve source file read error handlingMarkus Armbruster
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>
2019-09-28qapi: Improve reporting of redefinitionMarkus Armbruster
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>
2019-09-28qapi: Improve reporting of missing documentation commentMarkus Armbruster
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>
2019-09-28qapi: Eliminate check_keys(), rename check_known_keys()Markus Armbruster
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>
2019-09-28qapi: Improve reporting of invalid 'if' furtherMarkus Armbruster
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>
2019-09-28qapi: Avoid redundant definition references in error messagesMarkus Armbruster
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>
2019-09-28qapi: Improve reporting of missing / unknown definition keysMarkus Armbruster
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>
2019-09-28qapi: Improve reporting of invalid flagsMarkus Armbruster
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>
2019-09-28qapi: Improve reporting of invalid 'if' errorsMarkus Armbruster
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>
2019-09-28qapi: Move context-free checking to the proper placeMarkus Armbruster
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>
2019-09-28qapi: Move context-sensitive checking to the proper placeMarkus Armbruster
When we introduced the QAPISchema intermediate representation (commit ac88219a6c7), 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>
2019-09-28qapi: Inline check_name() into check_union()Markus Armbruster
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>
2019-09-28qapi: Plumb info to the QAPISchemaMemberMarkus Armbruster
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>
2019-09-28qapi: Make check_type()'s array case a bit more obviousMarkus Armbruster
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>
2019-09-28qapi: Move check for reserved names out of add_name()Markus Armbruster
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>
2019-09-28qapi: Report invalid '*' prefix like any other invalid nameMarkus Armbruster
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>
2019-09-28qapi: Use check_name_str() where it sufficesMarkus Armbruster
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>
2019-09-28qapi: Improve reporting of invalid name errorsMarkus Armbruster
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>
2019-09-28qapi: Reorder check_FOO() parameters for consistencyMarkus Armbruster
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>
2019-09-28qapi: Improve reporting of member name clashesMarkus Armbruster
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>
2019-09-28qapi: Change frontend error messages to start with lower caseMarkus Armbruster
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>
2019-09-28qapi: Clean up member name case checkingMarkus Armbruster
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>
2019-09-28qapi: Prefix frontend errors with an "in definition" lineMarkus Armbruster
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>
2019-09-28qapi: New QAPISourceInfo, replacing dictMarkus Armbruster
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>
2019-09-28qapi: Rename .owner to .defined_inMarkus Armbruster
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>
2019-09-28qapi: Tighten QAPISchemaFOO.check() assertionsMarkus Armbruster
When we introduced the QAPISchema intermediate representation (commit ac88219a6c7), 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>
2019-09-24qapi: Assert .visit() and .check_clash() run only after .check()Markus Armbruster
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>
2019-09-24qapi: Fix excessive QAPISchemaEntity.check() recursionMarkus Armbruster
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]
2019-09-24qapi: Fix to .check() empty structs just onceMarkus Armbruster
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>
2019-09-24qapi: Delete useless check_exprs() code for simple union kindMarkus Armbruster
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>
2019-09-24qapi: Clean up around check_known_keys()Markus Armbruster
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>
2019-09-24qapi: Simplify check_keys()Markus Armbruster
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>
2019-09-24qapi: Normalize 'if' in check_exprs(), like other sugarMarkus Armbruster
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>
2019-09-24qapi: Fix missing 'if' checks in struct, union, alternate 'data'Markus Armbruster
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>
2019-09-24qapi: Reject blank 'if' conditions in addition to empty onesMarkus Armbruster
"'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>
2019-09-24qapi: Fix broken discriminator error messagesMarkus Armbruster
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>
2019-09-24qapi: Remove null from schema languageMarkus Armbruster
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>
2019-09-24qapi: Improve reporting of lexical errorsMarkus Armbruster
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>
2019-09-24qapi: Use quotes more consistently in frontend error messagesMarkus Armbruster
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>
2019-09-24qapi: Tweak code to match docs/devel/qapi-code-gen.txtMarkus Armbruster
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>