On Tue, Feb 23, 2021 at 03:43:48PM +0000, Peter Maydell wrote:
> On Tue, 23 Feb 2021 at 15:03, Christian Ehrhardt
> <christian.ehrha...@canonical.com> wrote:
> >
> > glib2.0 introduced a change in 2.67.3 and later which triggers an
> > issue [1] for anyone including it's headers in a "extern C" context
> > which a few files in disas/* do. An example of such an include chain
> > and error look like:
> >
> > ../../disas/arm-a64.cc
> > In file included from /usr/include/glib-2.0/glib/gmacros.h:241,
> > from
> > /usr/lib/x86_64-linux-gnu/glib-2.0/include/glibconfig.h:9,
> > from /usr/include/glib-2.0/glib/gtypes.h:32,
> > from /usr/include/glib-2.0/glib/galloca.h:32,
> > from /usr/include/glib-2.0/glib.h:30,
> > from /<<BUILDDIR>>/qemu-5.2+dfsg/include/glib-compat.h:32,
> > from /<<BUILDDIR>>/qemu-5.2+dfsg/include/qemu/osdep.h:126,
> > from ../../disas/arm-a64.cc:21:
> > /usr/include/c++/10/type_traits:56:3: error: template with C linkage
> > 56 | template<typename _Tp, _Tp __v>
> > | ^~~~~~~~
> > ../../disas/arm-a64.cc:20:1: note: ‘extern "C"’ linkage started here
> > 20 | extern "C" {
> > | ^~~~~~~~~~
> >
> > To fix that move the include of osdep.h out of that section. It was added
> > already as C++ fixup by e78490c44: "disas/arm-a64.cc: Include osdep.h
> > first".
> >
> > [1]: https://gitlab.gnome.org/GNOME/glib/-/issues/2331
>
> I'm not convinced by this as a fix, though I'm happy to be corrected
> by somebody with a fuller understanding of C++. glib.h may be supposed
> to work as a C++ header, but osdep.h as a whole is definitely a C header,
> so I think it ought to be inside 'extern C'; and it has to be
> the first header included; and it happens to want to include glib.h.
>
> Fixing glib.h seems like it would be nicer, assuming they haven't
> already shipped this breakage. Failing that, does it work to do:
This was raised in Fedora and upstream GLib already
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1935
https://lists.fedoraproject.org/archives/list/de...@lists.fedoraproject.org/thread/J3P4TRHLWNDIKXF76OLYZNAPTABCZ3U5/#7LXFUDBBBIT23FE44QJYWX3I7U4EHW6M
The key comment there is this one:
"Note that wrapping the header include in an extern declaration violates
C++ standard requirements. ("A translation unit shall include a header
only outside of any declaration or definition", [using.headers]/3)"
IOW, if we need to make osdep.h safe to use from C++, then we
need to put the 'extern "C" { ... }' bit in osdep.h itself,
not in the things which include it.
>
> /*
> * glib.h expects to be included as a C++ header if we're
> * building a C++ file, but osdep.h and thus glib-compat.h are
> * C headers and should be inside an "extern C" block.
> */
> #ifdef __cplusplus
> extern "C++" {
> #include <glib.h>
> #if defined(G_OS_UNIX)
> #include <glib-unix.h>
> #endif
> }
>
> in include/glib-compat.h ?
That'd be even worse.
We need to make headers that need to be used from C++ code follow
the pattern:
#include <foo1>
#include <foo2>
#include <foo3>
...all other includs..
extern "C" {
..
only the declarations, no #includes
...
};
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
