Re: [Qemu-devel] [PATCH v4 06/51] qapi: pass 'if' condition into QAPISchemaEntity objects
Marc-André Lureau writes: > Hi > > On Tue, Feb 6, 2018 at 11:12 AM, Markus Armbruster wrote: >> Marc-André Lureau writes: >> >>> Built-in objects remain unconditional. Explicitly defined objects >>> use the condition specified in the schema. Implicitly defined >>> objects inherit their condition from their users. For most of them, >>> there is exactly one user, so the condition to use is obvious. The >>> exception is the wrapped type's generated for simple union variants, >>> which can be shared by any number of simple unions. The tight >>> condition would be the disjunction of the conditions of these simple >>> unions. For now, use wrapped type's condition instead. Much >>> simpler and good enough for now. >>> >>> Signed-off-by: Marc-André Lureau >>> Reviewed-by: Markus Armbruster >>> --- >>> scripts/qapi.py | 98 >>> ++--- >>> 1 file changed, 66 insertions(+), 32 deletions(-) >>> >>> diff --git a/scripts/qapi.py b/scripts/qapi.py >>> index 27df0fcf48..8f54dead8d 100644 >>> --- a/scripts/qapi.py >>> +++ b/scripts/qapi.py >>> @@ -991,8 +991,17 @@ def check_exprs(exprs): >>> # Schema compiler frontend >>> # >>> >>> +def listify_cond(ifcond): >>> +if not ifcond: >>> +return [] >>> +elif not isinstance(ifcond, list): >>> +return [ifcond] >>> +else: >>> +return ifcond >> >> pylint complains: >> >> R:995, 4: Unnecessary "else" after "return" (no-else-return) >> >> Matter of taste. Mine happens to agree with pylint's. Suggest: >> >>def listify_cond(ifcond): >>if not ifcond: >>return [] >>if not isinstance(ifcond, list): >>return [ifcond] >>return ifcond >> > > There are so many errors with pylint, that I don't bother running it. pylint reports lots of stuff that is actually just fine. > How did you notice? you run diff of error output between commits? Yes. > pycodestyle --diff is more convenient, and silent on this code. Formerly known as pep8. Confusingly, Fedora 26 packages both separately. Thanks for the pointer. > Feel free to touch if you pick the patch. Okay. >>> + >>> + >>> class QAPISchemaEntity(object): >>> -def __init__(self, name, info, doc): >>> +def __init__(self, name, info, doc, ifcond=None): >>> assert isinstance(name, str) >>> self.name = name >>> # For explicitly defined entities, info points to the (explicit) >> [...] >>
Re: [Qemu-devel] [PATCH v4 06/51] qapi: pass 'if' condition into QAPISchemaEntity objects
Hi On Tue, Feb 6, 2018 at 11:12 AM, Markus Armbruster wrote: > Marc-André Lureau writes: > >> Built-in objects remain unconditional. Explicitly defined objects >> use the condition specified in the schema. Implicitly defined >> objects inherit their condition from their users. For most of them, >> there is exactly one user, so the condition to use is obvious. The >> exception is the wrapped type's generated for simple union variants, >> which can be shared by any number of simple unions. The tight >> condition would be the disjunction of the conditions of these simple >> unions. For now, use wrapped type's condition instead. Much >> simpler and good enough for now. >> >> Signed-off-by: Marc-André Lureau >> Reviewed-by: Markus Armbruster >> --- >> scripts/qapi.py | 98 >> ++--- >> 1 file changed, 66 insertions(+), 32 deletions(-) >> >> diff --git a/scripts/qapi.py b/scripts/qapi.py >> index 27df0fcf48..8f54dead8d 100644 >> --- a/scripts/qapi.py >> +++ b/scripts/qapi.py >> @@ -991,8 +991,17 @@ def check_exprs(exprs): >> # Schema compiler frontend >> # >> >> +def listify_cond(ifcond): >> +if not ifcond: >> +return [] >> +elif not isinstance(ifcond, list): >> +return [ifcond] >> +else: >> +return ifcond > > pylint complains: > > R:995, 4: Unnecessary "else" after "return" (no-else-return) > > Matter of taste. Mine happens to agree with pylint's. Suggest: > >def listify_cond(ifcond): >if not ifcond: >return [] >if not isinstance(ifcond, list): >return [ifcond] >return ifcond > There are so many errors with pylint, that I don't bother running it. How did you notice? you run diff of error output between commits? pycodestyle --diff is more convenient, and silent on this code. Feel free to touch if you pick the patch. >> + >> + >> class QAPISchemaEntity(object): >> -def __init__(self, name, info, doc): >> +def __init__(self, name, info, doc, ifcond=None): >> assert isinstance(name, str) >> self.name = name >> # For explicitly defined entities, info points to the (explicit) > [...] > -- Marc-André Lureau
Re: [Qemu-devel] [PATCH v4 06/51] qapi: pass 'if' condition into QAPISchemaEntity objects
Marc-André Lureau writes: > Built-in objects remain unconditional. Explicitly defined objects > use the condition specified in the schema. Implicitly defined > objects inherit their condition from their users. For most of them, > there is exactly one user, so the condition to use is obvious. The > exception is the wrapped type's generated for simple union variants, > which can be shared by any number of simple unions. The tight > condition would be the disjunction of the conditions of these simple > unions. For now, use wrapped type's condition instead. Much > simpler and good enough for now. > > Signed-off-by: Marc-André Lureau > Reviewed-by: Markus Armbruster > --- > scripts/qapi.py | 98 > ++--- > 1 file changed, 66 insertions(+), 32 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 27df0fcf48..8f54dead8d 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -991,8 +991,17 @@ def check_exprs(exprs): > # Schema compiler frontend > # > > +def listify_cond(ifcond): > +if not ifcond: > +return [] > +elif not isinstance(ifcond, list): > +return [ifcond] > +else: > +return ifcond pylint complains: R:995, 4: Unnecessary "else" after "return" (no-else-return) Matter of taste. Mine happens to agree with pylint's. Suggest: def listify_cond(ifcond): if not ifcond: return [] if not isinstance(ifcond, list): return [ifcond] return ifcond > + > + > class QAPISchemaEntity(object): > -def __init__(self, name, info, doc): > +def __init__(self, name, info, doc, ifcond=None): > assert isinstance(name, str) > self.name = name > # For explicitly defined entities, info points to the (explicit) [...]
Re: [Qemu-devel] [PATCH v4 06/51] qapi: pass 'if' condition into QAPISchemaEntity objects
Marc-André Lureau writes: > Built-in objects remain unconditional. Explicitly defined objects > use the condition specified in the schema. Implicitly defined > objects inherit their condition from their users. For most of them, > there is exactly one user, so the condition to use is obvious. The > exception is the wrapped type's generated for simple union variants, Editing accident: you changed "is the wrapper types" to "is the wrapped type's" here instead of > which can be shared by any number of simple unions. The tight > condition would be the disjunction of the conditions of these simple > unions. For now, use wrapped type's condition instead. Much changing "use wrapped type's" to "use the wrapped type's" here. Can touch up on commit. > simpler and good enough for now. > > Signed-off-by: Marc-André Lureau > Reviewed-by: Markus Armbruster R-by stands.
[Qemu-devel] [PATCH v4 06/51] qapi: pass 'if' condition into QAPISchemaEntity objects
Built-in objects remain unconditional. Explicitly defined objects use the condition specified in the schema. Implicitly defined objects inherit their condition from their users. For most of them, there is exactly one user, so the condition to use is obvious. The exception is the wrapped type's generated for simple union variants, which can be shared by any number of simple unions. The tight condition would be the disjunction of the conditions of these simple unions. For now, use wrapped type's condition instead. Much simpler and good enough for now. Signed-off-by: Marc-André Lureau Reviewed-by: Markus Armbruster --- scripts/qapi.py | 98 ++--- 1 file changed, 66 insertions(+), 32 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index 27df0fcf48..8f54dead8d 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -991,8 +991,17 @@ def check_exprs(exprs): # Schema compiler frontend # +def listify_cond(ifcond): +if not ifcond: +return [] +elif not isinstance(ifcond, list): +return [ifcond] +else: +return ifcond + + class QAPISchemaEntity(object): -def __init__(self, name, info, doc): +def __init__(self, name, info, doc, ifcond=None): assert isinstance(name, str) self.name = name # For explicitly defined entities, info points to the (explicit) @@ -1002,6 +1011,7 @@ class QAPISchemaEntity(object): # such place). self.info = info self.doc = doc +self.ifcond = listify_cond(ifcond) def c_name(self): return c_name(self.name) @@ -1118,8 +1128,8 @@ class QAPISchemaBuiltinType(QAPISchemaType): class QAPISchemaEnumType(QAPISchemaType): -def __init__(self, name, info, doc, values, prefix): -QAPISchemaType.__init__(self, name, info, doc) +def __init__(self, name, info, doc, ifcond, values, prefix): +QAPISchemaType.__init__(self, name, info, doc, ifcond) for v in values: assert isinstance(v, QAPISchemaMember) v.set_owner(name) @@ -1154,7 +1164,7 @@ class QAPISchemaEnumType(QAPISchemaType): class QAPISchemaArrayType(QAPISchemaType): def __init__(self, name, info, element_type): -QAPISchemaType.__init__(self, name, info, None) +QAPISchemaType.__init__(self, name, info, None, None) assert isinstance(element_type, str) self._element_type_name = element_type self.element_type = None @@ -1162,6 +1172,7 @@ class QAPISchemaArrayType(QAPISchemaType): def check(self, schema): self.element_type = schema.lookup_type(self._element_type_name) assert self.element_type +self.ifcond = self.element_type.ifcond def is_implicit(self): return True @@ -1183,11 +1194,12 @@ class QAPISchemaArrayType(QAPISchemaType): class QAPISchemaObjectType(QAPISchemaType): -def __init__(self, name, info, doc, base, local_members, variants): +def __init__(self, name, info, doc, ifcond, + base, local_members, variants): # struct has local_members, optional base, and no variants # flat union has base, variants, and no local_members # simple union has local_members, variants, and no base -QAPISchemaType.__init__(self, name, info, doc) +QAPISchemaType.__init__(self, name, info, doc, ifcond) assert base is None or isinstance(base, str) for m in local_members: assert isinstance(m, QAPISchemaObjectTypeMember) @@ -1375,8 +1387,8 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): class QAPISchemaAlternateType(QAPISchemaType): -def __init__(self, name, info, doc, variants): -QAPISchemaType.__init__(self, name, info, doc) +def __init__(self, name, info, doc, ifcond, variants): +QAPISchemaType.__init__(self, name, info, doc, ifcond) assert isinstance(variants, QAPISchemaObjectTypeVariants) assert variants.tag_member variants.set_owner(name) @@ -1412,9 +1424,9 @@ class QAPISchemaAlternateType(QAPISchemaType): class QAPISchemaCommand(QAPISchemaEntity): -def __init__(self, name, info, doc, arg_type, ret_type, +def __init__(self, name, info, doc, ifcond, arg_type, ret_type, gen, success_response, boxed): -QAPISchemaEntity.__init__(self, name, info, doc) +QAPISchemaEntity.__init__(self, name, info, doc, ifcond) assert not arg_type or isinstance(arg_type, str) assert not ret_type or isinstance(ret_type, str) self._arg_type_name = arg_type @@ -1451,8 +1463,8 @@ class QAPISchemaCommand(QAPISchemaEntity): class QAPISchemaEvent(QAPISchemaEntity): -def __init__(self, name, info, doc, arg_type, boxed): -QAPISchemaEntity.__init__(self, name, info, doc) +def __init__(self, name, info, doc, ifcond, arg_type, boxed): +QAPISchemaEntity.__init__(self, name, info,
