Hi Jan,
On 06/12/2023 11:08, Jan Beulich wrote:
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.
Anthony approved it. So I have committed the patch.
Cheers,
--
Julien Grall
