aboutsummaryrefslogtreecommitdiff
path: root/scripts/qapi/common.py
AgeCommit message (Collapse)AuthorFilesLines
2021-05-20qapi: add must_match helperJohn Snow1-1/+7
Mypy cannot generally understand that these regex functions cannot possibly fail. Add a "must_match" helper that makes this clear for mypy. Signed-off-by: John Snow <jsnow@redhat.com> Message-Id: <20210519183951.3946870-10-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-03-23qapi: Permit flat union members for any tag valueMarkus Armbruster1-4/+4
Flat union branch names match the tag enum's member names. Omitted branches default to "no members for this tag value". Branch names starting with a digit get rejected like "'data' member '0' has an invalid name". However, omitting the branch works. This is because flat union tag values get checked twice: as enum member name, and as union branch name. The former accepts leading digits, the latter doesn't. Branches whose names start with a digit therefore cannot have members. Feels wrong. Get rid of the restriction by skipping the latter check. This can expose c_name() to input it can't handle: a name starting with a digit. Improve it to return a valid C identifier for any input. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-9-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [Commit message rewritten]
2020-10-10qapi/common.py: move build_params into gen.pyJohn Snow1-23/+0
Including it in common.py creates a circular import dependency; schema relies on common, but common.build_params requires a type annotation from schema. To type this properly, it needs to be moved outside the cycle. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-18-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: Convert comments into docstrings, and elaborateJohn Snow1-14/+40
As docstrings, they'll show up in documentation and IDE help. The docstring style being targeted is the Sphinx documentation style. Sphinx uses an extension of ReST with "domains". We use the (implicit) Python domain, which supports a number of custom "info fields". Those info fields are documented here: https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#info-field-lists Primarily, we use `:param X: descr`, `:return[s]: descr`, and `:raise[s] Z: when`. Everything else is the Sphinx dialect of ReST. (No, nothing checks or enforces this style that I am aware of. Sphinx either chokes or succeeds, but does not enforce a standard of what is otherwise inside the docstring. Pycharm does highlight when your param fields are not aligned with the actual fields present. It does not highlight missing return or exception statements. There is no existing style guide I am aware of that covers a standard for a minimally acceptable docstring. I am debating writing one.) Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-17-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: add type hint annotationsJohn Snow1-11/+16
Annotations do not change runtime behavior. This commit *only* adds annotations. Note that build_params() cannot be fully annotated due to import dependency issues. The commit after next will take care of it. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-16-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: Replace one-letter 'c' variableJohn Snow1-4/+4
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20201009161558.107041-14-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: delint with pylintJohn Snow1-10/+8
At this point, that just means using a consistent strategy for constant names. constants get UPPER_CASE and names not used externally get a leading underscore. As a preference, while renaming constants to be UPPERCASE, move them to the head of the file. Generally, it's nice to be able to audit the code that runs on import in one central place. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-13-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: Add indent managerJohn Snow1-16/+33
Code style tools really dislike the use of global keywords, because it generally involves re-binding the name at runtime which can have strange effects depending on when and how that global name is referenced in other modules. Make a little indent level manager instead. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-12-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: Remove python compatibility workaroundJohn Snow1-4/+1
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-11-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-03-05qapi: Drop conditionals for Python 2Markus Armbruster1-5/+1
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>
2019-10-22qapi: Split up scripts/qapi/common.pyMarkus Armbruster1-2321/+0
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 Armbruster1-59/+0
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 Armbruster1-2/+3
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 Armbruster1-17/+19
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-09-28qapi: Improve source file read error handlingMarkus Armbruster1-20/+26
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 Armbruster1-0/+5
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 Armbruster1-10/+8
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 Armbruster1-19/+21
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 Armbruster1-11/+16
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 Armbruster1-80/+49
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 Armbruster1-21/+19
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 Armbruster1-10/+12
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 Armbruster1-2/+2
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 Armbruster1-8/+8
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 Armbruster1-231/+193
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 Armbruster1-2/+4
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 Armbruster1-34/+42
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 Armbruster1-1/+2
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 Armbruster1-6/+10
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 Armbruster1-4/+2
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 Armbruster1-5/+4
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 Armbruster1-4/+16
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 Armbruster1-41/+39
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 Armbruster1-13/+23
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 Armbruster1-90/+102
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 Armbruster1-11/+14
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 Armbruster1-1/+14
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 Armbruster1-28/+41
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 Armbruster1-30/+31
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 Armbruster1-2/+7
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 Armbruster1-1/+10
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 Armbruster1-22/+52
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 Armbruster1-1/+1
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 Armbruster1-37/+2
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 Armbruster1-4/+4
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 Armbruster1-11/+8
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 Armbruster1-11/+15
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 Armbruster1-0/+3
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 Armbruster1-2/+3
"'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 Armbruster1-5/+4
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>
generated by cgit v1.1 at 2025-01-06 15:05:35 +0000