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. Yet then, thinkking more generally, imo it would be a good idea to always for either the "yes" or the "no" option for warnings we care about controlling ourselves. But that's nothing belonging in your change, of course. Jan
