On 06.12.2023 12:07, Julien Grall wrote: > Hi Jan, > > On 06/12/2023 11:05, Jan Beulich wrote: >> On 06.12.2023 11:52, Julien Grall wrote: >>> Hi Jan, >>> >>> On 06/12/2023 08:52, Jan Beulich wrote: >>>> On 05.12.2023 19:32, Julien Grall wrote: >>>>> From: Julien Grall <jgr...@amazon.com> >>>>> >>>>> Right now, all tools and hypervisor will be complied with the option >>>>> -Wdeclaration-after-statement. While most of the code in the hypervisor >>>>> is controlled by us, for tools we may import external libraries. >>>>> >>>>> The build will fail if one of them are using the construct we are >>>>> trying to prevent. This is the case when building against Python 3.12 >>>>> and Yocto: >>>>> >>>>> | 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 >>>>> >>>>> Looking at the tools directory, a fair few directory already add >>>>> -Wno-declaration-after-statement to inhibit the default behavior. >>>>> >>>>> We have always build the hypervisor with the flag, so for now remove >>>>> only the flag for anything but the hypervisor. We can decide at later >>>>> time whether we want to relax. >>>>> >>>>> Also remove the -Wno-declaration-after-statement in some subdirectory >>>>> as the flag is now unnecessary. >>>> >>>> With all these removals, don't you need to add the option centrally >>>> somewhere? Or else are you sure that no compiler version, including >>>> distro-customized ones, would ever come with the warning enabled by >>>> default? >>> >>> I can't really go through the dozens of different built compilers... But >>> I would find odd that a compiler will force this option. I view it as a >>> style enforcement option and that's not up to a distro to decide what >>> every projects do. >>> >>> Also, Allowing your thinking, we would need to add -Wno-switch-default & >>> co just in case a compiler decide to enable it. So where would we stop >>> adding -Wno-*? >>> >>> I don't think this is very scalable. >> >> I agree on this point, but: With the change you do there's a (slim) risk >> of introducing build breakage. With other -W* / -Wno-* options we haven't >> had reports of build issues. > > The chance is very unlikely here. So I am not in favor of doing this. I > would like the opinion from the others.
Well, it's Anthony anyway who has the final say. I'm not going to insist, I merely wanted to point out a possible issue. Jan
