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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4186 bytes --]

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').


> +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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]