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

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

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

The keyword const is a red herring here, and that's precisely what's
wrong with the commit message.  The minimized test case demonstrates the
problem without 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.

That's better.

It's weird that a compound literal isn't a constant expression, but
arguing about it here won't make gcc treat it as one.

> $ 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

Using this:

    qlit: Change compound literals to initializers

    The QLIT_QFOO() macros expand into compound literals.  Sadly, gcc
    doesn't recognizes these as constant expressions (clang does), which
    makes the macros useless for initializing objects with static storage
    duration.

    There is a gcc bug about it:
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713

    Change the macros to expand into initializers.

Avoids passing judgement on "bug or no bug", and avoids referring to the
compount-lit.c example without actually including it.

I might still add a comment.