Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Marc-André Lureau]

Hi

On Thu, Aug 31, 2017 at 10:43 AM Markus Armbruster <address@hidden>
wrote:

> Marc-André Lureau <address@hidden> writes:
>
> > Hi
> >
> > ----- Original Message -----
> >> Marc-André Lureau <address@hidden> writes:
> >>
> >> > Hi
> >> >
> >> > ----- Original Message -----
> >> >> Marc-André Lureau <address@hidden> writes:
> >> >>
> >> >> > They are not considered constant expressions in C, producing an
> error
> >> >> > when compiling a const QLit.
> >> >>
> >> >> A const QLit?  Do you mean a non-const one?
> >> >
> >> > Really a const QLitObject:
> >> >
> >> >
> >> > 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[]) {
> >> >                                                                 ^
> >> > Removing the "compound literals" fixes this error.
> >>
> >> Does QLIT_QLIST(((const QLitObject[]) { ... } work?
> >
> > No. Even if I put "const" all over the place (in member, in compound
> type etc).
> >
> > Give it a try, see if you can make it const, I am out of luck.
>
> The commit message's explanation is wrong.  This isn't about const at
> all, it's about "constant expressions", which are something else
> entirely.
>

The point was that declaring a non const QLit with those "compound
literals" worked vs with const.


> For what it's worth, clang is cool with the compound literals.  On
> Fedora 26 with a minimized test case (appended):
>
>     $ clang -c -g -O -Wall compound-lit.c
>     $ gcc -c -g -O -Wall compound-lit.c
>     compound-lit.c:30:37: error: initializer element is not constant
>          .value.qdict = (QLitDictEntry[]){
>                                          ^
>     compound-lit.c:30:37: note: (near initialization for
> ‘(anonymous).value’)
>
> GCC bug or not?  A search of the GCC Bugzilla finds
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713
>
> Copying a few notorious language lawyers^W^W^Wtrusted advisers.
>
> Even if this turns out to be a gcc bug, we'll have to work around it.
> But the work-around needs a comment then.
>
> In any case, the commit message needs fixing.
>

What about adapting the bug comment:

qlit: remove compound literals

A compound literal (i.e., "(struct Str1){1}"), is not a constant
expression, and so it cannot be used to initialize an object with static
storage duration.

$ gcc -c -g -O -Wall compound-lit.c
    compound-lit.c:30:37: error: initializer element is not constant
         .value.qdict = (QLitDictEntry[]){
                                         ^
    compound-lit.c:30:37: note: (near initialization for
‘(anonymous).value’)

clang accepts it. In some cases, gcc accepts compound literals as
initializer, but not in this nested case. There is a gcc bug about it:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713.

Feel free to adapt on commit

thanks


>
>
> enum {
>     QTYPE_NONE, QTYPE_QSTRING, QTYPE_QDICT,
> };
>
> typedef struct QLitDictEntry QLitDictEntry;
> typedef struct QLitObject QLitObject;
>
> struct QLitObject {
>     int type;
>     union {
>         const char *qstr;
>         QLitDictEntry *qdict;
>     } value;
> };
>
> struct QLitDictEntry {
>     const char *key;
>     QLitObject value;
> };
>
> QLitObject qlit1 = (QLitObject){
>     .type = QTYPE_QDICT,
>     .value.qdict = (QLitDictEntry[]){
>         { "foo", {} },
>         {}
>     }};
>
> QLitObject qlit2 = (QLitObject){
>     .type = QTYPE_QDICT,
>     .value.qdict = (QLitDictEntry[]){
>         { "foo", (QLitObject){} },
>         {}
>     }};
>
> --
Marc-André Lureau