[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals |
Date: |
Thu, 31 Aug 2017 15:50:12 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
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.
- Re: [Qemu-devel] [PATCH v2 01/14] qdict: add qdict_put_null() helper, (continued)
- [Qemu-devel] [PATCH v2 02/14] qlit: move qlit from check-qjson to qobject/, Marc-André Lureau, 2017/08/25
- [Qemu-devel] [PATCH v2 03/14] qlit: use QLit prefix consistently, Marc-André Lureau, 2017/08/25
- [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals, Marc-André Lureau, 2017/08/25
- Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals, Markus Armbruster, 2017/08/30
- Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals, Marc-André Lureau, 2017/08/30
- Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals, Markus Armbruster, 2017/08/30
- Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals, Marc-André Lureau, 2017/08/30
- Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals, Markus Armbruster, 2017/08/31
- Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals, Marc-André Lureau, 2017/08/31
- Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals, Laszlo Ersek, 2017/08/31
[Qemu-devel] [PATCH v2 05/14] qlit: rename compare_litqobj_to_qobj() to qlit_equal_qobject(), Marc-André Lureau, 2017/08/25
[Qemu-devel] [PATCH v2 06/14] qlit: make qlit_equal_qobject return a bool, Marc-André Lureau, 2017/08/25
[Qemu-devel] [PATCH v2 07/14] qlit: make qlit_equal_qobject() take const arguments, Marc-André Lureau, 2017/08/25
[Qemu-devel] [PATCH v2 08/14] qlit: add QLIT_QNULL and QLIT_BOOL, Marc-André Lureau, 2017/08/25
[Qemu-devel] [PATCH v2 09/14] qlit: Replace open-coded qnum_get_int() by call, Marc-André Lureau, 2017/08/25
[Qemu-devel] [PATCH v2 10/14] tests: add qlit tests, Marc-André Lureau, 2017/08/25
[Qemu-devel] [PATCH v2 11/14] qlit: improve QLit dict vs qdict comparison, Marc-André Lureau, 2017/08/25