Re: [Qemu-devel] [PATCH 04/14] qlit: remove needless type cast

[Marc-André Lureau]

Hi

On Fri, Aug 25, 2017 at 8:20 AM Markus Armbruster <address@hidden> wrote:

> Marc-André Lureau <address@hidden> writes:
>
> > And misc code style fix.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >  include/qapi/qmp/qlit.h | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
> > index f36bca4554..1e9696988a 100644
> > --- a/include/qapi/qmp/qlit.h
> > +++ b/include/qapi/qmp/qlit.h
> > @@ -36,13 +36,13 @@ struct QLitDictEntry {
> >  };
> >
> >  #define QLIT_QNUM(val) \
> > -    (QLitObject){.type = QTYPE_QNUM, .value.qnum = (val)}
> > +    { .type = QTYPE_QNUM, .value.qnum = (val) }
> >  #define QLIT_QSTR(val) \
> > -    (QLitObject){.type = QTYPE_QSTRING, .value.qstr = (val)}
> > +    { .type = QTYPE_QSTRING, .value.qstr = (val) }
> >  #define QLIT_QDICT(val) \
> > -    (QLitObject){.type = QTYPE_QDICT, .value.qdict = (val)}
> > +    { .type = QTYPE_QDICT, .value.qdict = (val) }
> >  #define QLIT_QLIST(val) \
> > -    (QLitObject){.type = QTYPE_QLIST, .value.qlist = (val)}
> > +    { .type = QTYPE_QLIST, .value.qlist = (val) }
> >
> >  int compare_litqobj_to_qobj(QLitObject *lhs, QObject *rhs);
>
> Technically, this isn't a type cast, it's a compound literal.  A
> compound literal is "an unnamed object whose value is given by the
> initializer list" (C99 §6.5.2.5).  The standard then cautions "that this
> differs from a cast expression" (ibid).
>


You are right, but actually the patch helps with another issue if we keep
the compound type:

const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
            QLIT_QNULL,
            {}
        }));

qmp-introspect.c:17:63: error: initializer element is not constant
 const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
                                                               ^
/home/elmarco/src/qemu/include/qapi/qmp/qlit.h:50:57: note: in definition
of macro ‘QLIT_QLIST’
     (QLitObject) { .type = QTYPE_QLIST, .value.qlist = (val) }
                                                         ^~~
qmp-introspect.c:17:63: note: (near initialization for ‘(anonymous).value’)
 const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
                                                               ^
/home/elmarco/src/qemu/include/qapi/qmp/qlit.h:50:57: note: in definition
of macro ‘QLIT_QLIST’
     (QLitObject) { .type = QTYPE_QLIST, .value.qlist = (val) }
                                                         ^~~
Any idea how to fix that?



>
> The previous paragraph probably makes sense only to language-lawyers, so
> let's consider an example.
>
> The macro
>
>    #define QLIT_QSTR(val) \
>        (QLitObject){.type = QTYPE_QSTRING, .value.qstr = (val)}
>
> expands into a compound literal of type QLitObject, which can be used as
> an initializer for a variable of type QLitObject.
>
> The macro
>
>    #define QLIT_QSTR(val) \
>        { .type = QTYPE_QSTRING, .value.qstr = (val) }
>
> expands into something that can be used as an initializer for a variable
> of *any* structure type with suitable members.  Duck typing, if you
> will.
>
> The original author's choice of compound literals is clearly
> intentional.  I'd prefer to respect it unless there's a compelling
> reason not to.
>
> --
Marc-André Lureau