| Message ID | 20170824103350.16400-5-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
Marc-André Lureau <marcandre.lureau@redhat.com> writes: > And misc code style fix. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > 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). 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.
Hi On Fri, Aug 25, 2017 at 8:20 AM Markus Armbruster <armbru@redhat.com> wrote: > Marc-André Lureau <marcandre.lureau@redhat.com> writes: > > > And misc code style fix. > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > 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
Marc-André Lureau <marcandre.lureau@gmail.com> writes: > Hi > > On Fri, Aug 25, 2017 at 8:20 AM Markus Armbruster <armbru@redhat.com> wrote: > >> Marc-André Lureau <marcandre.lureau@redhat.com> writes: >> >> > And misc code style fix. >> > >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> > --- >> > 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? This one: [...] >> 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. Rewrite your commit message so it explains your *actual* reason for converting the macro from compound literal to initializer.
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);
And misc code style fix. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/qapi/qmp/qlit.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)