[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 21/27] qapi: add #if conditions to generated
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH v6 21/27] qapi: add #if conditions to generated code members |
Date: |
Sat, 8 Dec 2018 15:19:49 +0400 |
Hi
On Thu, Dec 6, 2018 at 9:42 PM Markus Armbruster <address@hidden> wrote:
>
> Marc-André Lureau <address@hidden> writes:
>
> > Wrap generated enum/struct members and code with #if/#endif, using the
>
> enum and struct members
ok
>
> > .ifcond members added in the previous patches.
> >
> > Some types generate both enum and struct members for example, so a
> > step-by-step is unnecessarily complicated to deal with (it would
> > easily generate invalid intermediary code).
>
> Can you give an example of a schema definition that would lead to
> complications?
>
Honestly, I don't remember well (it's been a while I wrote that code).
It must be related to implicit enums, such as union kind... If there
is no strong need to split this patch, I would rather not do that
extra work.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> > scripts/qapi/common.py | 4 ++++
> > scripts/qapi/introspect.py | 13 +++++++++----
> > scripts/qapi/types.py | 4 ++++
> > scripts/qapi/visit.py | 6 ++++++
> > 4 files changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index b3b64a60bf..a66c035b91 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -2098,11 +2098,13 @@ const QEnumLookup %(c_name)s_lookup = {
> > ''',
> > c_name=c_name(name))
> > for m in members:
> > + ret += gen_if(m.ifcond)
> > index = c_enum_const(name, m.name, prefix)
> > ret += mcgen('''
> > [%(index)s] = "%(name)s",
> > ''',
> > index=index, name=m.name)
> > + ret += gen_endif(m.ifcond)
> >
> > ret += mcgen('''
> > },
> > @@ -2124,10 +2126,12 @@ typedef enum %(c_name)s {
> > c_name=c_name(name))
> >
> > for m in enum_members:
> > + ret += gen_if(m.ifcond)
> > ret += mcgen('''
> > %(c_enum)s,
> > ''',
> > c_enum=c_enum_const(name, m.name, prefix))
> > + ret += gen_endif(m.ifcond)
> >
> > ret += mcgen('''
> > } %(c_name)s;
> > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> > index 3f1ca99f6d..bf5db51f4a 100644
> > --- a/scripts/qapi/introspect.py
> > +++ b/scripts/qapi/introspect.py
> > @@ -148,6 +148,8 @@ const QLitObject %(c_name)s = %(c_string)s;
> > ret = {'name': member.name, 'type': self._use_type(member.type)}
> > if member.optional:
> > ret['default'] = None
> > + if member.ifcond:
> > + ret = (ret, member.ifcond)
> > return ret
> >
> > def _gen_variants(self, tag_name, variants):
> > @@ -155,14 +157,16 @@ const QLitObject %(c_name)s = %(c_string)s;
> > 'variants': [self._gen_variant(v) for v in variants]}
> >
> > def _gen_variant(self, variant):
> > - return {'case': variant.name, 'type': self._use_type(variant.type)}
> > + return ({'case': variant.name, 'type':
> > self._use_type(variant.type)},
> > + variant.ifcond)
>
> Looks different in your rebased version at
> https://github.com/elmarco/qemu.git branch qapi-if. I'm only skimming
> this version.
>
> Note to self: always creates the tuple form, even when there's no
> if-condition. Differs from _gen_qlit(), which creates a tuple only when
> there's a if-condition. Not wrong, just a bit inconsistent.
>
> >
> > def visit_builtin_type(self, name, info, json_type):
> > self._gen_qlit(name, 'builtin', {'json-type': json_type}, [])
> >
> > def visit_enum_type(self, name, info, ifcond, members, prefix):
> > self._gen_qlit(name, 'enum',
> > - {'values': [m.name for m in members]}, ifcond)
> > + {'values': [(m.name, m.ifcond) for m in members]},
> > + ifcond)
>
> Likewise.
>
> >
> > def visit_array_type(self, name, info, ifcond, element_type):
> > element = self._use_type(element_type)
> > @@ -178,8 +182,9 @@ const QLitObject %(c_name)s = %(c_string)s;
> >
> > def visit_alternate_type(self, name, info, ifcond, variants):
> > self._gen_qlit(name, 'alternate',
> > - {'members': [{'type': self._use_type(m.type)}
> > - for m in variants.variants]}, ifcond)
> > + {'members': [
> > + ({'type': self._use_type(m.type)}, m.ifcond)
> > + for m in variants.variants]}, ifcond)
>
> Likewise.
>
> >
> > def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
> > success_response, boxed, allow_oob, allow_preconfig):
> > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> > index 2d4a70f810..7d9eef6320 100644
> > --- a/scripts/qapi/types.py
> > +++ b/scripts/qapi/types.py
> > @@ -43,6 +43,7 @@ struct %(c_name)s {
> > def gen_struct_members(members):
> > ret = ''
> > for memb in members:
> > + ret += gen_if(memb.ifcond)
> > if memb.optional:
> > ret += mcgen('''
> > bool has_%(c_name)s;
> > @@ -52,6 +53,7 @@ def gen_struct_members(members):
> > %(c_type)s %(c_name)s;
> > ''',
> > c_type=memb.type.c_type(), c_name=c_name(memb.name))
> > + ret += gen_endif(memb.ifcond)
> > return ret
> >
> >
> > @@ -131,11 +133,13 @@ def gen_variants(variants):
> > for var in variants.variants:
> > if var.type.name == 'q_empty':
> > continue
> > + ret += gen_if(var.ifcond)
> > ret += mcgen('''
> > %(c_type)s %(c_name)s;
> > ''',
> > c_type=var.type.c_unboxed_type(),
> > c_name=c_name(var.name))
> > + ret += gen_endif(var.ifcond)
> >
> > ret += mcgen('''
> > } u;
> > diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> > index 24f85a2e85..82eab72b21 100644
> > --- a/scripts/qapi/visit.py
> > +++ b/scripts/qapi/visit.py
> > @@ -54,6 +54,7 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s
> > *obj, Error **errp)
> > c_type=base.c_name())
> >
> > for memb in members:
> > + ret += gen_if(memb.ifcond)
> > if memb.optional:
> > ret += mcgen('''
> > if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) {
> > @@ -73,6 +74,7 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s
> > *obj, Error **errp)
> > ret += mcgen('''
> > }
> > ''')
> > + ret += gen_endif(memb.ifcond)
> >
> > if variants:
> > ret += mcgen('''
> > @@ -84,6 +86,7 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s
> > *obj, Error **errp)
> > case_str = c_enum_const(variants.tag_member.type.name,
> > var.name,
> > variants.tag_member.type.prefix)
> > + ret += gen_if(var.ifcond)
> > if var.type.name == 'q_empty':
> > # valid variant and nothing to do
> > ret += mcgen('''
> > @@ -100,6 +103,7 @@ void visit_type_%(c_name)s_members(Visitor *v,
> > %(c_name)s *obj, Error **errp)
> > case=case_str,
> > c_type=var.type.c_name(),
> > c_name=c_name(var.name))
> >
> > + ret += gen_endif(var.ifcond)
> > ret += mcgen('''
> > default:
> > abort();
> > @@ -190,6 +194,7 @@ void visit_type_%(c_name)s(Visitor *v, const char
> > *name, %(c_name)s **obj, Error
> > c_name=c_name(name))
> >
> > for var in variants.variants:
> > + ret += gen_if(var.ifcond)
> > ret += mcgen('''
> > case %(case)s:
> > ''',
> > @@ -217,6 +222,7 @@ void visit_type_%(c_name)s(Visitor *v, const char
> > *name, %(c_name)s **obj, Error
> > ret += mcgen('''
> > break;
> > ''')
> > + ret += gen_endif(var.ifcond)
> >
> > ret += mcgen('''
> > case QTYPE_NONE:
thanks