qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH RFC v4 02/32] qapi: New QAPISchema intermediate


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH RFC v4 02/32] qapi: New QAPISchema intermediate reperesentation
Date: Fri, 04 Sep 2015 16:40:58 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Eric Blake <address@hidden> writes:

> On 09/03/2015 08:29 AM, Markus Armbruster wrote:
>> The QAPI code generators work with a syntax tree (nested dictionaries)
>> plus a few symbol tables (also dictionaries) on the side.
>> 
>
>> +
>> +class QAPISchemaArrayType(QAPISchemaType):
>> +    def __init__(self, name, info, element_type):
>> +        QAPISchemaType.__init__(self, name, info)
>> +        assert isinstance(element_type, str)
>> +        self.element_type_name = element_type
>> +        self.element_type = None
>
> Would it make sense to have:
>
> self._element_type_name = element_type
> self.element_type = None
>
>> +    def check(self, schema):
>> +        self.element_type = schema.lookup_type(self.element_type_name)
>> +        assert self.element_type
>> +
>
> and the corresponding
> self.element_type = schema.lookup_type(self._element_type_name)
>
> to make it obvious that ._element_type_name is for internal use only,
> while .element_type is the preferred member for queries/manipulation by
> qapi-*.py, which should only be using this object after check() has been
> run?
>
>> +class QAPISchemaObjectType(QAPISchemaType):
>> +    def __init__(self, name, info, base, local_members, variants):
>> +        QAPISchemaType.__init__(self, name, info)
>> +        assert base == None or isinstance(base, str)
>> +        for m in local_members:
>> +            assert isinstance(m, QAPISchemaObjectTypeMember)
>> +        assert variants == None \
>> +            or isinstance(variants, QAPISchemaObjectTypeVariants)
>> +        self.base_name = base
>> +        self.base = None
>> +        self.local_members = local_members
>> +        self.variants = variants
>> +        self.members = None
>
> Similarly for self._base_name
>
>> +
>> +class QAPISchemaObjectTypeMember(object):
>> +    def __init__(self, name, typ, optional):
>> +        assert isinstance(name, str)
>> +        assert isinstance(typ, str)
>> +        assert isinstance(optional, bool)
>> +        self.name = name
>> +        self.type_name = typ
>> +        self.type = None
>> +        self.optional = optional
>
> and for self._type_name
>
>> +class QAPISchemaObjectTypeVariants(object):
>> +    def __init__(self, tag_name, tag_enum, variants):
>> +        assert tag_name == None or isinstance(tag_name, str)
>> +        assert tag_enum == None or isinstance(tag_enum, str)
>> +        for v in variants:
>> +            assert isinstance(v, QAPISchemaObjectTypeVariant)
>> +        self.tag_name = tag_name
>> +        if tag_name:
>> +            assert not tag_enum
>> +            self.tag_member = None
>> +        else:
>> +            self.tag_member = QAPISchemaObjectTypeMember('kind', tag_enum,
>> +                                                         False)
>
> and eventually for self._tag_name (except that we MUST expose
> self._tag_name until after we fix the C struct to use 'type' instead of
> 'kind').

Python lets you use variants._tag_name.  The leading underscore is
advisory (except for wildcard imports, which don't apply here).

>> +class QAPISchemaCommand(QAPISchemaEntity):
>> + def __init__(self, name, info, arg_type, ret_type, gen,
>> success_response):
>> +        QAPISchemaEntity.__init__(self, name, info)
>> +        assert not arg_type or isinstance(arg_type, str)
>> +        assert not ret_type or isinstance(ret_type, str)
>> +        self.arg_type_name = arg_type
>> +        self.arg_type = None
>> +        self.ret_type_name = ret_type
>> +        self.ret_type = None
>> +        self.gen = gen
>> +        self.success_response = success_response
>
> and for self._art_type_name and self._ret_type_name
>
>> +
>> +class QAPISchemaEvent(QAPISchemaEntity):
>> +    def __init__(self, name, info, arg_type):
>> +        QAPISchemaEntity.__init__(self, name, info)
>> +        assert not arg_type or isinstance(arg_type, str)
>> +        self.arg_type_name = arg_type
>> +        self.arg_type = None
>
> and for self._arg_type_name
>
> Basically, if I'm understanding the python conventions correctly, naming
> an instance member with leading underscore implies that it is for
> private use only; and if nothing else, the rename proves whether any of
> the other python files are relying on a leaky abstraction.

I followed that convention for methods, but didn't bother for instance
variables.  Perhaps I should.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]