[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 12/46] qapi: Track location that created an i
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v5 12/46] qapi: Track location that created an implicit type |
Date: |
Tue, 29 Sep 2015 10:02:16 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 09/28/2015 06:56 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> A future patch will enable error detection in the various
>>> QapiSchema check() methods. But since all errors have to
>>> have an associated 'info' location, we need a location to
>>> be associated with all implicit types. Easiest is to reuse
>>> the location of the enclosing entity that includes the
>>> dictionary defining the implicit type.
>>>
>>> While at it, we were always passing None as the location of
>>> array types, making that parameter useless; sharing the
>>> location (if any) of the underlying element type makes sense.
>>
>> The parameter is useless only because all array types are implicit.
>> Once we change that, it won't be anymore.
>
> I guess it depends whether I pass in the existing info when creating the
> array or determine the info when resolving the string name of the array
> element during check() (it will ultimately be the same info either way,
> it's just a question of which approach looks cleaner for getting the
> information set).
The latter leaves info None until check(). Unhealthy state.
I suspect the appropriate info is readily available in all the places
where we create array types, so let's just pass it to
_make_array_type().
>>> @@ -917,6 +917,7 @@ class QAPISchemaArrayType(QAPISchemaType):
>>> def check(self, schema):
>>> self.element_type = schema.lookup_type(self._element_type_name)
>>> assert self.element_type
>>> + self._info = self.element_type._info
>>>
>>> def json_type(self):
>>> return 'array'
>>
>> Implicit array type's info is the element type's info. Okay.
>>
>>> @@ -928,6 +929,7 @@ class QAPISchemaArrayType(QAPISchemaType):
>>> class QAPISchemaObjectType(QAPISchemaType):
>>> def __init__(self, name, info, base, local_members, variants):
>>> QAPISchemaType.__init__(self, name, info)
>>> + assert info or name == ':empty'
>>
>> I think what we really want to assert is "we got info unless this is a
>> built-in entity", in QAPISchemaEntity.__init__().
>
> To do that, arrays would have to pass info in to __init__() rather than
> deferring to check() as I did above.
>
>>
>> Built-in entities are exactly the types defined by
>> QAPISchema._def_predefineds(), currently the built-in types and
>> ':empty'.
>
> I'm still wondering how best to test that, but agree that hoisting the
> assert into QAPISchemaEntity instead of just in QAPISchemaObjectType
> would be nice. Maybe some sort of boolean switch, initially off, then
> turned on after _def_predefineds() is called, and assuming that no types
> other than predefineds are initialized prior to that point.
Either that, or have a special info value for built-in types. Oh wait,
we got one already: None :)
Back to serious. If we have to work for the assertion, we should
consider the assertion's value: how likely are the actual mistakes it
can catch?
Can legitimate errors be reported for built-in types?
>>
>> Missing: implicit enum types' info.
>
> I'll add it; should be the info of the containing union type that caused
> the implicit enum.
Yup, just like for the union's wrapper objects.
- Re: [Qemu-devel] [PATCH v5 12/46] qapi: Track location that created an implicit type,
Markus Armbruster <=