From 7599697c66d22ff4c859ba6ccea30e6a9aae6b9b Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Mar 2016 16:48:29 -0600 Subject: qapi: Adjust names of implicit types The original choice of ':obj-' as the prefix for implicit types made it obvious that we weren't going to clash with any user-defined names, which cannot contain ':'. But now we want to create structs for implicit types, to get rid of special cases in the generators, and our use of ':' in implicit names needs a tweak to produce valid C code. We could transliterate ':' to '_', except that C99 mandates that "identifiers that begin with an underscore are always reserved for use as identifiers with file scope in both the ordinary and tag name spaces". So it's time to change our naming convention: we can instead use the 'q_' prefix that we reserved for ourselves back in commit 9fb081e0. Technically, since we aren't planning on exposing the empty type in generated code, we could keep the name ':empty', but renaming it to 'q_empty' makes the check for startswith('q_') cover all implicit types, whether or not code is generated for them. As long as we don't declare 'empty' or 'obj' ticklish, it shouldn't clash with c_name() prepending 'q_' to the user's ticklish names. Signed-off-by: Eric Blake Message-Id: <1458254921-17042-5-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- docs/qapi-code-gen.txt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'docs') diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index e0b2ef11f6..c648f76952 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -575,9 +575,9 @@ names an object type without members. Example: the SchemaInfo for command query-qmp-schema { "name": "query-qmp-schema", "meta-type": "command", - "arg-type": ":empty", "ret-type": "SchemaInfoList" } + "arg-type": "q_empty", "ret-type": "SchemaInfoList" } - Type ":empty" is an object type without members, and type + Type "q_empty" is an automatic object type without members, and type "SchemaInfoList" is the array of SchemaInfo type. The SchemaInfo for an event has meta-type "event", and variant member @@ -594,9 +594,9 @@ QAPI schema implicitly defines an object type. Example: the SchemaInfo for EVENT_C from section Events { "name": "EVENT_C", "meta-type": "event", - "arg-type": ":obj-EVENT_C-arg" } + "arg-type": "q_obj-EVENT_C-arg" } - Type ":obj-EVENT_C-arg" is an implicitly defined object type with + Type "q_obj-EVENT_C-arg" is an implicitly defined object type with the two members from the event's definition. The SchemaInfo for struct and union types has meta-type "object". @@ -660,11 +660,11 @@ Union types { "name": "type", "type": "BlockdevOptionsKind" } ], "tag": "type", "variants": [ - { "case": "file", "type": ":obj-FileOptions-wrapper" }, - { "case": "qcow2", "type": ":obj-Qcow2Options-wrapper" } ] } + { "case": "file", "type": "q_obj-FileOptions-wrapper" }, + { "case": "qcow2", "type": "q_obj-Qcow2Options-wrapper" } ] } Enumeration type "BlockdevOptionsKind" and the object types - ":obj-FileOptions-wrapper", ":obj-Qcow2Options-wrapper" are + "q_obj-FileOptions-wrapper", "q_obj-Qcow2Options-wrapper" are implicitly defined. The SchemaInfo for an alternate type has meta-type "alternate", and -- cgit v1.2.3-55-g7522 From bd59adce692cbb70e5639c0f5611b45a0d107167 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Mar 2016 16:48:38 -0600 Subject: qapi: Make BlockdevOptions doc example closer to reality Although we don't want to repeat the entire BlockdevOptions QMP command in the example, it helps if we aren't needlessly diverging (the initial example was written before we had committed the actual QMP interface). Use names that match what is found in qapi/block-core.json, such as '*read-only' rather than 'readonly', or 'BlockdevRef' rather than 'BlockRef'. For the simple union example, invent BlockdevOptionsSimple so that later text is unambiguous which of the two union forms is meant (telling the user to refer back to two 'BlockdevOptions' wasn't nice, and QMP has only the flat union form). Also, mention that the discriminator of a flat union is non-optional. Signed-off-by: Eric Blake Message-Id: <1458254921-17042-14-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- docs/qapi-code-gen.txt | 74 +++++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 37 deletions(-) (limited to 'docs') diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index c648f76952..12af1b8cef 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -297,22 +297,22 @@ be empty. A simple union type defines a mapping from automatic discriminator values to data types like in this example: - { 'struct': 'FileOptions', 'data': { 'filename': 'str' } } - { 'struct': 'Qcow2Options', - 'data': { 'backing-file': 'str', 'lazy-refcounts': 'bool' } } + { 'struct': 'BlockdevOptionsFile', 'data': { 'filename': 'str' } } + { 'struct': 'BlockdevOptionsQcow2', + 'data': { 'backing': 'str', '*lazy-refcounts': 'bool' } } - { 'union': 'BlockdevOptions', - 'data': { 'file': 'FileOptions', - 'qcow2': 'Qcow2Options' } } + { 'union': 'BlockdevOptionsSimple', + 'data': { 'file': 'BlockdevOptionsFile', + 'qcow2': 'BlockdevOptionsQcow2' } } In the Client JSON Protocol, a simple union is represented by a dictionary that contains the 'type' member as a discriminator, and a 'data' member that is of the specified data type corresponding to the discriminator value, as in these examples: - { "type": "file", "data" : { "filename": "/some/place/my-image" } } - { "type": "qcow2", "data" : { "backing-file": "/some/place/my-image", - "lazy-refcounts": true } } + { "type": "file", "data": { "filename": "/some/place/my-image" } } + { "type": "qcow2", "data": { "backing": "/some/place/my-image", + "lazy-refcounts": true } } The generated C code uses a struct containing a union. Additionally, an implicit C enum 'NameKind' is created, corresponding to the union @@ -325,29 +325,29 @@ avoids nesting on the wire. All branches of the union must be complex types, and the top-level members of the union dictionary on the wire will be combination of members from both the base type and the appropriate branch type (when merging two dictionaries, there must be -no keys in common). The 'discriminator' member must be the name of an -enum-typed member of the base struct. +no keys in common). The 'discriminator' member must be the name of a +non-optional enum-typed member of the base struct. The following example enhances the above simple union example by -adding a common member 'readonly', renaming the discriminator to -something more applicable, and reducing the number of {} required on -the wire: +adding an optional common member 'read-only', renaming the +discriminator to something more applicable than the simple union's +default of 'type', and reducing the number of {} required on the wire: { 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] } - { 'struct': 'BlockdevCommonOptions', - 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } } + { 'struct': 'BlockdevOptionsBase', + 'data': { 'driver': 'BlockdevDriver', '*read-only': 'bool' } } { 'union': 'BlockdevOptions', - 'base': 'BlockdevCommonOptions', + 'base': 'BlockdevOptionsBase', 'discriminator': 'driver', - 'data': { 'file': 'FileOptions', - 'qcow2': 'Qcow2Options' } } + 'data': { 'file': 'BlockdevOptionsFile', + 'qcow2': 'BlockdevOptionsQcow2' } } Resulting in these JSON objects: - { "driver": "file", "readonly": true, + { "driver": "file", "read-only": true, "filename": "/some/place/my-image" } - { "driver": "qcow2", "readonly": false, - "backing-file": "/some/place/my-image", "lazy-refcounts": true } + { "driver": "qcow2", "read-only": false, + "backing": "/some/place/my-image", "lazy-refcounts": true } Notice that in a flat union, the discriminator name is controlled by the user, but because it must map to a base member with enum type, the @@ -382,7 +382,7 @@ data types (string, integer, number, or object, but currently not array) on the wire. The definition is similar to a simple union type, where each branch of the union names a QAPI type. For example: - { 'alternate': 'BlockRef', + { 'alternate': 'BlockdevRef', 'data': { 'definition': 'BlockdevOptions', 'reference': 'str' } } @@ -403,7 +403,7 @@ following example objects: { "file": "my_existing_block_device_id" } { "file": { "driver": "file", - "readonly": false, + "read-only": false, "filename": "/tmp/mydisk.qcow2" } } @@ -637,11 +637,11 @@ Union types { "name": "BlockdevOptions", "meta-type": "object", "members": [ { "name": "driver", "type": "BlockdevDriver" }, - { "name": "readonly", "type": "bool"} ], + { "name": "read-only", "type": "bool", "default": null } ], "tag": "driver", "variants": [ - { "case": "file", "type": "FileOptions" }, - { "case": "qcow2", "type": "Qcow2Options" } ] } + { "case": "file", "type": "BlockdevOptionsFile" }, + { "case": "qcow2", "type": "BlockdevOptionsQcow2" } ] } Note that base types are "flattened": its members are included in the "members" array. @@ -652,20 +652,20 @@ discriminator (called "type" on the wire, see section Union types). A simple union implicitly defines an object type for each of its variants. -Example: the SchemaInfo for simple union BlockdevOptions from section +Example: the SchemaInfo for simple union BlockdevOptionsSimple from section Union types - { "name": "BlockdevOptions", "meta-type": "object", + { "name": "BlockdevOptionsSimple", "meta-type": "object", "members": [ - { "name": "type", "type": "BlockdevOptionsKind" } ], + { "name": "type", "type": "BlockdevOptionsSimpleKind" } ], "tag": "type", "variants": [ - { "case": "file", "type": "q_obj-FileOptions-wrapper" }, - { "case": "qcow2", "type": "q_obj-Qcow2Options-wrapper" } ] } + { "case": "file", "type": "q_obj-BlockdevOptionsFile-wrapper" }, + { "case": "qcow2", "type": "q_obj-BlockdevOptionsQcow2-wrapper" } ] } - Enumeration type "BlockdevOptionsKind" and the object types - "q_obj-FileOptions-wrapper", "q_obj-Qcow2Options-wrapper" are - implicitly defined. + Enumeration type "BlockdevOptionsSimpleKind" and the object types + "q_obj-BlockdevOptionsFile-wrapper", "q_obj-BlockdevOptionsQcow2-wrapper" + are implicitly defined. The SchemaInfo for an alternate type has meta-type "alternate", and variant member "members". "members" is a JSON array. Each element is @@ -673,9 +673,9 @@ a JSON object with member "type", which names a type. Values of the alternate type conform to exactly one of its member types. There is no guarantee on the order in which "members" will be listed. -Example: the SchemaInfo for BlockRef from section Alternate types +Example: the SchemaInfo for BlockdevRef from section Alternate types - { "name": "BlockRef", "meta-type": "alternate", + { "name": "BlockdevRef", "meta-type": "alternate", "members": [ { "type": "BlockdevOptions" }, { "type": "str" } ] } -- cgit v1.2.3-55-g7522 From ac4338f8eb783fd421aae492ca262a586918471e Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Mar 2016 16:48:39 -0600 Subject: qapi: Allow anonymous base for flat union Rather than requiring all flat unions to explicitly create a separate base struct, we can allow the qapi schema to specify the common members via an inline dictionary. This is similar to how commands can specify an inline anonymous type for its 'data'. We already have several struct types that only exist to serve as a single flat union's base; the next commit will clean them up. In particular, this patch's change to the BlockdevOptions example in qapi-code-gen.txt will actually be done in the real QAPI schema. Now that anonymous bases are legal, we need to rework the flat-union-bad-base negative test (as previously written, it forms what is now valid QAPI; tweak it to now provide coverage of a new error message path), and add a positive test in qapi-schema-test to use an anonymous base (making the integer argument optional, for even more coverage). Note that this patch only allows anonymous bases for flat unions; simple unions are already enough syntactic sugar that we do not want to burden them further. Meanwhile, while it would be easy to also allow an anonymous base for structs, that would be quite redundant, as the members can be put right into the struct instead. Signed-off-by: Eric Blake Message-Id: <1458254921-17042-15-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- docs/qapi-code-gen.txt | 26 +++++++++++++------------- scripts/qapi-types.py | 10 ++++++---- scripts/qapi.py | 12 ++++++++++-- tests/qapi-schema/flat-union-bad-base.err | 2 +- tests/qapi-schema/flat-union-bad-base.json | 5 ++--- tests/qapi-schema/qapi-schema-test.json | 6 +----- tests/qapi-schema/qapi-schema-test.out | 10 +++++----- 7 files changed, 38 insertions(+), 33 deletions(-) (limited to 'docs') diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 12af1b8cef..0e4bafff08 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -284,7 +284,7 @@ better than open-coding the member to be type 'str'. === Union types === Usage: { 'union': STRING, 'data': DICT } -or: { 'union': STRING, 'data': DICT, 'base': STRUCT-NAME, +or: { 'union': STRING, 'data': DICT, 'base': STRUCT-NAME-OR-DICT, 'discriminator': ENUM-MEMBER-OF-BASE } Union types are used to let the user choose between several different @@ -320,13 +320,16 @@ an implicit C enum 'NameKind' is created, corresponding to the union the union can be named 'max', as this would collide with the implicit enum. The value for each branch can be of any type. -A flat union definition specifies a struct as its base, and -avoids nesting on the wire. All branches of the union must be -complex types, and the top-level members of the union dictionary on -the wire will be combination of members from both the base type and the -appropriate branch type (when merging two dictionaries, there must be -no keys in common). The 'discriminator' member must be the name of a -non-optional enum-typed member of the base struct. +A flat union definition avoids nesting on the wire, and specifies a +set of common members that occur in all variants of the union. The +'base' key must specifiy either a type name (the type must be a +struct, not a union), or a dictionary representing an anonymous type. +All branches of the union must be complex types, and the top-level +members of the union dictionary on the wire will be combination of +members from both the base type and the appropriate branch type (when +merging two dictionaries, there must be no keys in common). The +'discriminator' member must be the name of a non-optional enum-typed +member of the base struct. The following example enhances the above simple union example by adding an optional common member 'read-only', renaming the @@ -334,10 +337,8 @@ discriminator to something more applicable than the simple union's default of 'type', and reducing the number of {} required on the wire: { 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] } - { 'struct': 'BlockdevOptionsBase', - 'data': { 'driver': 'BlockdevDriver', '*read-only': 'bool' } } { 'union': 'BlockdevOptions', - 'base': 'BlockdevOptionsBase', + 'base': { 'driver': 'BlockdevDriver', '*read-only': 'bool' }, 'discriminator': 'driver', 'data': { 'file': 'BlockdevOptionsFile', 'qcow2': 'BlockdevOptionsQcow2' } } @@ -366,10 +367,9 @@ union has a struct with a single member named 'data'. That is, is identical on the wire to: { 'enum': 'Enum', 'data': ['one', 'two'] } - { 'struct': 'Base', 'data': { 'type': 'Enum' } } { 'struct': 'Branch1', 'data': { 'data': 'str' } } { 'struct': 'Branch2', 'data': { 'data': 'int' } } - { 'union': 'Flat', 'base': 'Base', 'discriminator': 'type', + { 'union': 'Flat': 'base': { 'type': 'Enum' }, 'discriminator': 'type', 'data': { 'one': 'Branch1', 'two': 'Branch2' } } diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 92ae619fa4..e09c8751a9 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -72,12 +72,14 @@ struct %(c_name)s { c_name=c_name(name)) if base: - ret += mcgen(''' + if not base.is_implicit(): + ret += mcgen(''' /* Members inherited from %(c_name)s: */ ''', - c_name=base.c_name()) + c_name=base.c_name()) ret += gen_struct_members(base.members) - ret += mcgen(''' + if not base.is_implicit(): + ret += mcgen(''' /* Own members: */ ''') ret += gen_struct_members(members) @@ -224,7 +226,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): return self._fwdecl += gen_fwd_object_or_array(name) self.decl += gen_object(name, base, members, variants) - if base: + if base and not base.is_implicit(): self.decl += gen_upcast(name, base) # TODO Worth changing the visitor signature, so we could # directly use rather than repeat type.is_implicit()? diff --git a/scripts/qapi.py b/scripts/qapi.py index d91af94759..a38ef52922 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -327,6 +327,8 @@ class QAPISchemaParser(object): def find_base_members(base): + if isinstance(base, dict): + return base base_struct_define = find_struct(base) if not base_struct_define: return None @@ -561,9 +563,10 @@ def check_union(expr, expr_info): # Else, it's a flat union. else: - # The object must have a string member 'base'. + # The object must have a string or dictionary 'base'. check_type(expr_info, "'base' for union '%s'" % name, - base, allow_metas=['struct']) + base, allow_dict=True, allow_optional=True, + allow_metas=['struct']) if not base: raise QAPIExprError(expr_info, "Flat union '%s' must have a base" @@ -1039,6 +1042,8 @@ class QAPISchemaMember(object): owner = owner[6:] if owner.endswith('-arg'): return '(parameter of %s)' % owner[:-4] + elif owner.endswith('-base'): + return '(base of %s)' % owner[:-5] else: assert owner.endswith('-wrapper') # Unreachable and not implemented @@ -1325,6 +1330,9 @@ class QAPISchema(object): base = expr.get('base') tag_name = expr.get('discriminator') tag_member = None + if isinstance(base, dict): + base = (self._make_implicit_object_type( + name, info, 'base', self._make_members(base, info))) if tag_name: variants = [self._make_variant(key, value) for (key, value) in data.iteritems()] diff --git a/tests/qapi-schema/flat-union-bad-base.err b/tests/qapi-schema/flat-union-bad-base.err index 79b8a71eb8..bee24a217a 100644 --- a/tests/qapi-schema/flat-union-bad-base.err +++ b/tests/qapi-schema/flat-union-bad-base.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-bad-base.json:9: 'base' for union 'TestUnion' should be a type name +tests/qapi-schema/flat-union-bad-base.json:8: 'string' (member of TestTypeA) collides with 'string' (base of TestUnion) diff --git a/tests/qapi-schema/flat-union-bad-base.json b/tests/qapi-schema/flat-union-bad-base.json index e2e622bb6e..74dd421708 100644 --- a/tests/qapi-schema/flat-union-bad-base.json +++ b/tests/qapi-schema/flat-union-bad-base.json @@ -1,5 +1,4 @@ -# we require the base to be an existing struct -# TODO: should we allow an anonymous inline base type? +# we allow anonymous base, but enforce no duplicate keys { 'enum': 'TestEnum', 'data': [ 'value1', 'value2' ] } { 'struct': 'TestTypeA', @@ -7,7 +6,7 @@ { 'struct': 'TestTypeB', 'data': { 'integer': 'int' } } { 'union': 'TestUnion', - 'base': { 'enum1': 'TestEnum', 'kind': 'str' }, + 'base': { 'enum1': 'TestEnum', 'string': 'str' }, 'discriminator': 'enum1', 'data': { 'value1': 'TestTypeA', 'value2': 'TestTypeB' } } diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index e72274811e..f571e1bb34 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -75,14 +75,10 @@ 'base': 'UserDefZero', 'data': { 'string': 'str', 'enum1': 'EnumOne' } } -{ 'struct': 'UserDefUnionBase2', - 'base': 'UserDefZero', - 'data': { 'string': 'str', 'enum1': 'QEnumTwo' } } - # this variant of UserDefFlatUnion defaults to a union that uses members with # allocated types to test corner cases in the cleanup/dealloc visitor { 'union': 'UserDefFlatUnion2', - 'base': 'UserDefUnionBase2', + 'base': { '*integer': 'int', 'string': 'str', 'enum1': 'QEnumTwo' }, 'discriminator': 'enum1', 'data': { 'value1' : 'UserDefC', # intentional forward reference 'value2' : 'UserDefB' } } diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index d49fe1d184..19cd214f6b 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -66,7 +66,7 @@ object UserDefFlatUnion case value2: UserDefB case value3: UserDefB object UserDefFlatUnion2 - base UserDefUnionBase2 + base q_obj_UserDefFlatUnion2-base tag enum1 case value1: UserDefC case value2: UserDefB @@ -111,10 +111,6 @@ object UserDefUnionBase base UserDefZero member string: str optional=False member enum1: EnumOne optional=False -object UserDefUnionBase2 - base UserDefZero - member string: str optional=False - member enum1: QEnumTwo optional=False object UserDefZero member integer: int optional=False object WrapAlternate @@ -156,6 +152,10 @@ object q_obj_EVENT_D-arg member b: str optional=False member c: str optional=True member enum3: EnumOne optional=True +object q_obj_UserDefFlatUnion2-base + member integer: int optional=True + member string: str optional=False + member enum1: QEnumTwo optional=False object q_obj___org.qemu_x-command-arg member a: __org.qemu_x-EnumList optional=False member b: __org.qemu_x-StructList optional=False -- cgit v1.2.3-55-g7522