[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 27/30] qapi: Track owner of each object memb
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v10 27/30] qapi: Track owner of each object member |
Date: |
Tue, 10 Nov 2015 17:17:40 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 11/09/2015 07:26 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> Future commits will migrate semantic checking away from parsing
>> and over to the various QAPISchema*.check() methods. But to
>> report an error message about an incorrect semantic use of a
>> member of an object type, it helps to know which type, command,
>> or event owns the member. In particular, when a member is
>> inherited from a base type, it is desirable to associate the
>> member name with the base type (and not the type calling
>> member.check()).
>>
>>
>> + def _pretty_owner(self):
>> + # See QAPISchema._make_implicit_object_type() - reverse the
>> + # mapping there to create a nice human-readable description
>
> This comment confused me briefly. It applies to the following
> conditionals if-part, but not its else-part. Move it into the if-part?
Done.
>> @@ -1082,9 +1117,11 @@ class QAPISchemaAlternateType(QAPISchemaType):
>> QAPISchemaType.__init__(self, name, info)
>> assert isinstance(variants, QAPISchemaObjectTypeVariants)
>> assert not variants.tag_name
>> + variants.set_owner(name)
>> self.variants = variants
>>
>> def check(self, schema):
>> + self.variants.tag_member.set_owner(self.name)
>> self.variants.tag_member.check(schema)
>> self.variants.check(schema, {})
>>
>
> Odd: all other .set_owner() calls are in .__init__() or .set_owner().
> Can we move this one to __init__() for consistency?
Yes, done.
>
> I think we can: __init__() requires its variants argument to have a
> tag_member (it even asserts not variants.tag_name).
>
>> @@ -1212,6 +1249,7 @@ class QAPISchema(object):
>> def _make_implicit_object_type(self, name, info, role, members):
>> if not members:
>> return None
>> + # See also QAPISchemaObjectTypeMember.describe()
>
> Should this point to ._pretty_owner() instead?
Good call.
>
>> name = ':obj-%s-%s' % (name, role)
>> if not self.lookup_entity(name, QAPISchemaObjectType):
>> self._def_entity(QAPISchemaObjectType(name, info, None,
>> @@ -1315,7 +1353,7 @@ class QAPISchema(object):
>> data = expr.get('data')
>> if isinstance(data, OrderedDict):
>> data = self._make_implicit_object_type(
>> - name, info, 'arg', self._make_members(data, info))
>> + name, info, 'data', self._make_members(data, info))
>
> This is necessary only to make .pretty_owner() say 'data of EVENT_NAME'
> instead of 'argument of EVENT_NAME'. Do we really need different
> wording for commands and events?
>
> I'd make it say 'parameter of' for both commands and events. I
> habitually use "parameter" for formal parameters, and "argument" for
> actual arguments. Guess I've worked with the C standard too much...
'parameter' sounds better for both, at which point the messages no
longer differ,...
>
> For what it's worth, the QAPI schema language uses 'data' for both (and
> many other things, too), and the introspection schema uses 'arg-type'
> for both.
>
>> self._def_entity(QAPISchemaEvent(name, info, data))
>>
>> def _def_exprs(self):
>> diff --git a/tests/qapi-schema/qapi-schema-test.out
>> b/tests/qapi-schema/qapi-schema-test.out
>> index 786024e..f78ef04 100644
>> --- a/tests/qapi-schema/qapi-schema-test.out
>> +++ b/tests/qapi-schema/qapi-schema-test.out
>> @@ -1,9 +1,9 @@
>> object :empty
>> -object :obj-EVENT_C-arg
>> +object :obj-EVENT_C-data
>> member a: int optional=True
>> member b: UserDefOne optional=True
>> member c: str optional=False
>> -object :obj-EVENT_D-arg
>> +object :obj-EVENT_D-data
...so I no longer need to rename the implicit struct for events to allow
the differentiation. v11 is thus a smaller patch :)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v10 27/30] qapi: Track owner of each object member, Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 28/30] qapi: Detect collisions in C member names, Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 30/30] qapi: Forbid case-insensitive clashes, Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 22/30] qapi: Simplify QAPISchemaObjectTypeVariants.check(), Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 29/30] cpu: Convert CpuInfo into flat union, Eric Blake, 2015/11/06