Eric Blake <ebl...@redhat.com> writes:
> On 03/13/2017 01:18 AM, Markus Armbruster wrote:
>> qapi.py has a hardcoded white-list of type names that may violate the
>> rule on use of upper and lower case. Add a new pragma directive
>> 'name-case-whitelist', and use it to replace the hard-coded
>> white-list.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> docs/qapi-code-gen.txt | 6 ++++++
>> qapi-schema.json | 11 ++++++++++-
>> scripts/qapi.py | 22 ++++++++++------------
>> tests/qapi-schema/enum-member-case.err | 2 +-
>> tests/qapi-schema/enum-member-case.json | 1 +
>> 5 files changed, 28 insertions(+), 14 deletions(-)
>>
>
>> +++ b/qapi-schema.json
>> @@ -61,7 +61,16 @@
>> 'query-migrate-cache-size',
>> 'query-tpm-models',
>> 'query-tpm-types',
>> - 'ringbuf-read' ] } }
>> + 'ringbuf-read' ],
>> + 'name-case-whitelist': [
>> + 'ACPISlotType', # DIMM, visible through
>> query-acpi-ospm-status
>> + 'CpuInfoMIPS', # PC, visible through query-cpu
>> + 'CpuInfoTricore', # PC, visible through query-cpu
>> + 'QapiErrorClass', # all members, visible through errors
>> + 'UuidInfo', # UUID, visible through query-uuid
>> + 'X86CPURegister32', # all members, visible indirectly through
>> qom-get
>> + 'q_obj_CpuInfo-base' # CPU, visible through query-cpu
>> + ] } }
>
> Interesting - here you bunch up 2 of the 3 pragmas into one dict, while
> still leaving the third related to documentation in its own dict.
>
> That 'q_obj_CpuInfo-base' is ugly, and I had a patch around previously
> that used a saner name rather than making callers reverse-engineer the
> implicit naming rules. Related to my work on anonymous bases to flat
> unions, so I'll get to rebase that work and post it on top of yours.
> But not a show-stopper for this patch, where it is just moving the location.
... to a more visible place :)
Regarding flat unions: what I really want is unifying struct and both
kinds of unions into a single object type. Something like
{ 'object': STRING, # type name
'*base': STRING-OR-DICT, # base type
'*data': DICT, # common members
'*tag': STRING, # tag name, must be in base or common
'*variant': DICT } # map tag value -> variant members
Introspection already works that way.
Note no sugar for "simple" unions. They become syntactically
non-simple. Feature.
Flat unions become syntactically non-horrible.
>> @@ -311,6 +302,13 @@ class QAPISchemaParser(object):
>> "Pragma returns-whitelist must be"
>> " a list of strings")
>> returns_whitelist = value
>> + elif name == 'name-case-whitelist':
>> + if (not isinstance(value, list)
>> + or any([not isinstance(elt, str) for elt in value])):
>> + raise QAPISemError(info,
>> + "Pragma name-case-whitelist must be"
>> + " a list of strings")
>> + name_case_whitelist = value
>
> Same comments as before - new error message with no testsuite coverage,
> and no checking for duplicate assignment where last one silently wins;
> but where I'm okay with deferring if you don't want to delay 2.9 for a
> v2 respin.
>
> So if you use it unchanged,
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
