Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 05/05/2014 07:30 PM, Fam Zheng wrote:
>
>>>     NAME: { 'type': TYPE, 'default': DEFAULT }
>>>
>>> where
>>>
>>>     NAME: { 'type': TYPE }
>>>
>>> can be abbreviated to
>>>
>>>     NAME: TYPE
>
>> 
>> In data definition, we allow inline sub-structure:
>> 
>> { 'type': 'VersionInfo',
>>   'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
>>            'package': 'str'} }
>> 
>> If we allow properties as a dict, we need to disambiguate properly from the
>> above case. Proposing:
>> 
>>     MAGIC: { 'name': NAME, 'type: TYPE, 'default': DEFAULT }
>
> Oh, good catch.

Indeed.

>                  The alternative is to drop all instances of inline
> sub-structs.  Searching for 'data.*{.*{', I found only VersionInfo and
> PciBridgeInfo; I then did a full manual search of qapi-schema.json and
> only found the additional instance of PciDeviceInfo where the sub-struct
> is not on the same line as the initial { of the 'data'.  Just getting
> rid of inlined sub-structs may be quite feasible.
>
> On a related vein, there are a number of types that aren't merely a
> string name.  For example:
>
> { 'command': 'migrate-set-capabilities',
>   'data': { 'capabilities': ['MigrationCapabilityStatus'] } }
>
> and similar, where we have an array type rather than a raw string type
> name.  But at least with that, NAME: { 'type': [ 'array' ] } is still a
> reasonable parse.
>
> The problem with MAGIC:{'name'...} is that you need to express more than
> one parameter, but as a dict, you can't reuse the same MAGIC more than
> once.  That is:
>
> 'data': { MAGIC : { 'name': 'qemu', 'type': { 'major'...} },
>           MAGIC : { 'name': 'package', 'type', 'str' } } }
>
> proves that you have to have two distinct MAGIC in the same 'data', so
> '' for both is out.
>
>> 
>> Where MAGIC should NOT be something that is a valid NAME from current schema.
>> Some ideas:
>> 
>>  - Special string, that has invalid chars as a identifier, like '*', '%', 
>> '&',
>>    etc, or simply an empty str ''.
>>    Of course we need to enforce the checking and distinguishing in
>>    scripts/qapi-types.py.
>> 
>>  - Non-string: current NAME must be a string, any type non-string could be
>>    distinguished from NAME, like number, bool, null, []. But its meaning 
>> could
>>    be confusing to reviewer.

JSON requires member names to be strings.

>>  - Special symbol: we can define a dedicate symbol for this, like the literal
>>    TYPE, in the schema. But this way we deviate more from JSON.
>
> Also, while we aren't strict JSON, it's nice that we're still fairly
> close to JSON5, and worth keeping that property.

Both ideas move us further away from JSON and JSON5.

JSON5 allows unquoted identifiers as member names, but that's just
shorthand for the same name in quotes.

>> Personally, I think empty str '' makes more sense for me. What do you think?
>> 
>
> At this point, I'm leaning towards dropping support for unnamed inlined
> sub-structs.

For what it's worth, qapi-code-gen.txt has only strings (i.e. type
names) in the value position of a member declaration, never an object
(i.e. anonymous complex type, what you called unnamed inlined
sub-struct).

Another idea (I'm not sure I like it, though): in the unsugared NAME:
OBJECT form, OBJECT has a mandatory member 'type'.  This can be used to
decide whether we have an unsugared member definition or sugared member
definition with an anonymous complex type.  Examples:

* This can't be an unsugared member definition, because it lacks 'type':

    'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'}

* This could be either, but we prefer unsugared then:

    'foo': { 'type': 'str' }

  If you want an anonymous complex type, you need to desugar, like this:

    'foo': { 'type': { 'type': 'str' } }

Might want to outlaw member name 'type' to remove the ambiguity.

Yet another idea: drop anonymous complex types in the sugared form only.
Then an object in the value position always means unsugared.  We change
the existing members of anonymous complex type from NAME: OBJECT to
NAME: { 'type': OBJECT }.

>> Anyway we only add things, so we will keep the '*name' suger.

The schema is still just an internal interface; we can change it as much
as we want.  With schema introspection, the desugared form of the schema
visible there will be ABI.  Which means we better use our chance to make
our schema language a bit more extensible while it lasts.