| Message ID | 20231128174729.3880113-1-alex@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Config.mk: drop -Wdeclaration-after-statement | expand |
On 28.11.2023 18:47, Alexander Kanavin wrote: > Such constructs are fully allowed by C99: > https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Mixed-Labels-and-Declarations.html#Mixed-Labels-and-Declarations There's more to this: It may also be a policy of ours (or of any sub-component) to demand that declarations and statements are properly separated. This would therefore need discussing first. > If the flag is present, then building against python 3.12 will fail thusly: > > | In file included from /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/Python.h:44, > | from xen/lowlevel/xc/xc.c:8: > | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/object.h: In function 'Py_SIZE': > | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/object.h:233:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] > | 233 | PyVarObject *var_ob = _PyVarObject_CAST(ob); > | | ^~~~~~~~~~~ > | In file included from /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/Python.h:53: > | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/cpython/longintrepr.h: In function '_PyLong_CompactValue': > | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/cpython/longintrepr.h:121:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] > | 121 | Py_ssize_t sign = 1 - (op->long_value.lv_tag & _PyLong_SIGN_MASK); > | | ^~~~~~~~~~ > | cc1: all warnings being treated as errors At least by the specific wording of the diagnostic I'm inclined to call this a compiler bug: There's no point in mentioning C90 when -std=gnu99 was passed. > --- a/Config.mk > +++ b/Config.mk > @@ -177,8 +177,6 @@ CFLAGS += -std=gnu99 Just up from here there is CFLAGS += -std=gnu99 Yet there's no HOSTCFLAGS += -std=gnu99 anywhere. Hence ... > CFLAGS += -Wall -Wstrict-prototypes > > -$(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement) ... imo this removal is inappropriate. Jan > -$(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement) > $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable) > $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs) >
On 11/29/23 08:51, Jan Beulich wrote: > On 28.11.2023 18:47, Alexander Kanavin wrote: >> Such constructs are fully allowed by C99: >> https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Mixed-Labels-and-Declarations.html#Mixed-Labels-and-Declarations > There's more to this: It may also be a policy of ours (or of any sub-component) > to demand that declarations and statements are properly separated. This would > therefore need discussing first. The error is coming from python 3.12 headers and not from anything in xen tree, no? As you don't have control over those headers, I'm not sure what other solution there could be. Alex
Hi, + Anthony for the tools + Juergen for Xenstored On 29/11/2023 11:34, Alexander Kanavin wrote: > On 11/29/23 08:51, Jan Beulich wrote: > >> On 28.11.2023 18:47, Alexander Kanavin wrote: >>> Such constructs are fully allowed by C99: >>> https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Mixed-Labels-and-Declarations.html#Mixed-Labels-and-Declarations >> There's more to this: It may also be a policy of ours (or of any >> sub-component) >> to demand that declarations and statements are properly separated. >> This would >> therefore need discussing first. > > The error is coming from python 3.12 headers and not from anything in > xen tree, no? As you don't have control over those headers, I'm not sure > what other solution there could be. We seem to add -Wno-declaration-after-statement for some components in tools/. So one possibility would be to move the flags to an hypervisor specific makefile (in xen/). Anthony/Juergen, do you have any concern if the tools are built without -Wdeclaration-after-statement? Cheers,
On 29.11.2023 11:34, Alexander Kanavin wrote: > On 11/29/23 08:51, Jan Beulich wrote: > >> On 28.11.2023 18:47, Alexander Kanavin wrote: >>> Such constructs are fully allowed by C99: >>> https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Mixed-Labels-and-Declarations.html#Mixed-Labels-and-Declarations >> There's more to this: It may also be a policy of ours (or of any sub-component) >> to demand that declarations and statements are properly separated. This would >> therefore need discussing first. > > The error is coming from python 3.12 headers and not from anything in > xen tree, no? As you don't have control over those headers, I'm not sure > what other solution there could be. Suppress the option just for Python's C-binding? Jan
On Wed, Nov 29, 2023 at 11:47:24AM +0100, Julien Grall wrote: > Hi, > > + Anthony for the tools > + Juergen for Xenstored > > On 29/11/2023 11:34, Alexander Kanavin wrote: > > On 11/29/23 08:51, Jan Beulich wrote: > > > > > On 28.11.2023 18:47, Alexander Kanavin wrote: > > > > Such constructs are fully allowed by C99: > > > > https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Mixed-Labels-and-Declarations.html#Mixed-Labels-and-Declarations > > > There's more to this: It may also be a policy of ours (or of any > > > sub-component) > > > to demand that declarations and statements are properly separated. > > > This would > > > therefore need discussing first. > > > > The error is coming from python 3.12 headers and not from anything in > > xen tree, no? As you don't have control over those headers, I'm not sure > > what other solution there could be. > > We seem to add -Wno-declaration-after-statement for some components in > tools/. So one possibility would be to move the flags to an hypervisor > specific makefile (in xen/). You mean xen/Makefile I hope. > Anthony/Juergen, do you have any concern if the tools are built without > -Wdeclaration-after-statement? I don't, and as you said, there's already quite a few -Wno-declaration-after-statement. It can be nice to add a new variable in the middle of a function, it's like creating a new scope without adding extra indentation (if we wanted a new scope, we would need {} thus the intend). Cheers,
On 29.11.23 11:47, Julien Grall wrote: > Hi, > > + Anthony for the tools > + Juergen for Xenstored > > On 29/11/2023 11:34, Alexander Kanavin wrote: >> On 11/29/23 08:51, Jan Beulich wrote: >> >>> On 28.11.2023 18:47, Alexander Kanavin wrote: >>>> Such constructs are fully allowed by C99: >>>> https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Mixed-Labels-and-Declarations.html#Mixed-Labels-and-Declarations >>> There's more to this: It may also be a policy of ours (or of any sub-component) >>> to demand that declarations and statements are properly separated. This would >>> therefore need discussing first. >> >> The error is coming from python 3.12 headers and not from anything in xen >> tree, no? As you don't have control over those headers, I'm not sure what >> other solution there could be. > > We seem to add -Wno-declaration-after-statement for some components in tools/. > So one possibility would be to move the flags to an hypervisor specific makefile > (in xen/). > > Anthony/Juergen, do you have any concern if the tools are built without > -Wdeclaration-after-statement? I could live with that. Juergen
On 29.11.2023 14:10, Anthony PERARD wrote: > On Wed, Nov 29, 2023 at 11:47:24AM +0100, Julien Grall wrote: >> + Anthony for the tools >> + Juergen for Xenstored >> >> On 29/11/2023 11:34, Alexander Kanavin wrote: >>> On 11/29/23 08:51, Jan Beulich wrote: >>> >>>> On 28.11.2023 18:47, Alexander Kanavin wrote: >>>>> Such constructs are fully allowed by C99: >>>>> https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Mixed-Labels-and-Declarations.html#Mixed-Labels-and-Declarations >>>> There's more to this: It may also be a policy of ours (or of any >>>> sub-component) >>>> to demand that declarations and statements are properly separated. >>>> This would >>>> therefore need discussing first. >>> >>> The error is coming from python 3.12 headers and not from anything in >>> xen tree, no? As you don't have control over those headers, I'm not sure >>> what other solution there could be. >> >> We seem to add -Wno-declaration-after-statement for some components in >> tools/. So one possibility would be to move the flags to an hypervisor >> specific makefile (in xen/). > > You mean xen/Makefile I hope. > >> Anthony/Juergen, do you have any concern if the tools are built without >> -Wdeclaration-after-statement? > > I don't, and as you said, there's already quite a few > -Wno-declaration-after-statement. > > It can be nice to add a new variable in the middle of a function, it's > like creating a new scope without adding extra indentation (if we wanted > a new scope, we would need {} thus the intend). To be clear, I wouldn't mind this in the hypervisor either. But then I also don't see why we should further request people to separate declarations from statements in an easily noticeable way. Thing is that imo something like this wants spelling out in ./CODING_STYLE. Jan
Hi Jan, On 30/11/2023 08:36, Jan Beulich wrote: > On 29.11.2023 14:10, Anthony PERARD wrote: >> On Wed, Nov 29, 2023 at 11:47:24AM +0100, Julien Grall wrote: >>> + Anthony for the tools >>> + Juergen for Xenstored >>> >>> On 29/11/2023 11:34, Alexander Kanavin wrote: >>>> On 11/29/23 08:51, Jan Beulich wrote: >>>> >>>>> On 28.11.2023 18:47, Alexander Kanavin wrote: >>>>>> Such constructs are fully allowed by C99: >>>>>> https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Mixed-Labels-and-Declarations.html#Mixed-Labels-and-Declarations >>>>> There's more to this: It may also be a policy of ours (or of any >>>>> sub-component) >>>>> to demand that declarations and statements are properly separated. >>>>> This would >>>>> therefore need discussing first. >>>> >>>> The error is coming from python 3.12 headers and not from anything in >>>> xen tree, no? As you don't have control over those headers, I'm not sure >>>> what other solution there could be. >>> >>> We seem to add -Wno-declaration-after-statement for some components in >>> tools/. So one possibility would be to move the flags to an hypervisor >>> specific makefile (in xen/). >> >> You mean xen/Makefile I hope. >> >>> Anthony/Juergen, do you have any concern if the tools are built without >>> -Wdeclaration-after-statement? >> >> I don't, and as you said, there's already quite a few >> -Wno-declaration-after-statement. >> >> It can be nice to add a new variable in the middle of a function, it's >> like creating a new scope without adding extra indentation (if we wanted >> a new scope, we would need {} thus the intend). > > To be clear, I wouldn't mind this in the hypervisor either. But then I also > don't see why we should further request people to separate declarations > from statements in an easily noticeable way. Thing is that imo something > like this wants spelling out in ./CODING_STYLE. So I agree that if we were to remove -Wdeclaration-after-statement then we should also update the CODING_STYLE. However, I am not entirely sure I would want to mix code and declaration in the hypervisor. Anyway, I think this is a separate discussion from resolving the immediate problem (i.e. building the python bindings). So for now, I think it would make sense to push the -Wdeclaration-after-statement to the tools. @Alexander, are you going to send a new version? If not, I would be happy to do it. Cheers,
On 12/1/23 20:14, Julien Grall wrote: > > So I agree that if we were to remove -Wdeclaration-after-statement > then we should also update the CODING_STYLE. However, I am not > entirely sure I would want to mix code and declaration in the hypervisor. > > Anyway, I think this is a separate discussion from resolving the > immediate problem (i.e. building the python bindings). > > So for now, I think it would make sense to push the > -Wdeclaration-after-statement to the tools. > > @Alexander, are you going to send a new version? If not, I would be > happy to do it. Please do it, as in the meantime, my attention has focused entirely elsewhere, so I'd have to switch context and find time to study the xen source. I don't have specific interest in xen, the reason I looked into it is that we're updating python to 3.12 in yocto and this one was one of the many issues that came up all over the userspace stack.
Hi Alexander. On 04/12/2023 09:28, Alexander Kanavin wrote: > On 12/1/23 20:14, Julien Grall wrote: >> >> So I agree that if we were to remove -Wdeclaration-after-statement >> then we should also update the CODING_STYLE. However, I am not >> entirely sure I would want to mix code and declaration in the hypervisor. >> >> Anyway, I think this is a separate discussion from resolving the >> immediate problem (i.e. building the python bindings). >> >> So for now, I think it would make sense to push the >> -Wdeclaration-after-statement to the tools. >> >> @Alexander, are you going to send a new version? If not, I would be >> happy to do it. > > Please do it, as in the meantime, my attention has focused entirely > elsewhere, so I'd have to switch context and find time to study the xen > source. I don't have specific interest in xen, the reason I looked into > it is that we're updating python to 3.12 in yocto and this one was one > of the many issues that came up all over the userspace stack. Thanks, I have sent a patch [1]. I decided to add a Reported-by tag rather than Signed-off-by on my version. I hope this is fine. Cheers, [1] https://lore.kernel.org/xen-devel/20231205183226.26636-1-julien@xen.org/ > >
diff --git a/Config.mk b/Config.mk index 2c43702958..7e67b91de2 100644 --- a/Config.mk +++ b/Config.mk @@ -177,8 +177,6 @@ CFLAGS += -std=gnu99 CFLAGS += -Wall -Wstrict-prototypes -$(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement) -$(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement) $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable) $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
Such constructs are fully allowed by C99: https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Mixed-Labels-and-Declarations.html#Mixed-Labels-and-Declarations If the flag is present, then building against python 3.12 will fail thusly: | In file included from /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/Python.h:44, | from xen/lowlevel/xc/xc.c:8: | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/object.h: In function 'Py_SIZE': | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/object.h:233:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] | 233 | PyVarObject *var_ob = _PyVarObject_CAST(ob); | | ^~~~~~~~~~~ | In file included from /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/Python.h:53: | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/cpython/longintrepr.h: In function '_PyLong_CompactValue': | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/cpython/longintrepr.h:121:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] | 121 | Py_ssize_t sign = 1 - (op->long_value.lv_tag & _PyLong_SIGN_MASK); | | ^~~~~~~~~~ | cc1: all warnings being treated as errors Signed-off-by: Alexander Kanavin <alex@linutronix.de> --- Config.mk | 2 -- 1 file changed, 2 deletions(-)