GNOME Bugzilla – Bug 785554
glib-genmarshal: wrap prototypes in G_{BEGIN,END}_DECLS.
Last modified: 2017-08-07 16:07:55 UTC
Since --header --body has been deprecated and replaced with --body --prototypes, the generated body is not wrapped with G_{BEGIN,END}_DECLS any longer. Projects using C++ break due to that. Automatically wrap prototypes individually in G_{BEGIN,END}_DECLS instead.
Created attachment 356538 [details] [review] Wrap prototypes correctly.
Review of attachment 356538 [details] [review]: ::: gobject/glib-genmarshal.in @@ +557,3 @@ '''Generate a marshaller declaration with the given @visibility. If @va_marshal is True, the marshaller will use variadic arguments in place of a GValue array.''' + signature = ['G_BEGIN_DECLS'] I'm not really a fan of doing this unconditionally… Not everyone is using a C++ compiler, and I don't know how much of an impact this has.
Personally, I'd rather you changed the project to use two separate invocations - one for the header and one for the body; that would be backwards compatible, and would not require hacks. I also understand keeping backward compatibility, but I honestly think we should nudge people in doing the right thing, now that we have a chance.
Maybe we should call `G_BEGIN_DECLS` and `G_END_DECLS` in the whole body, when in compatibility mode.
That's fine. If using a C compiler, G_{BEGIN,END}_DECLS will be defined to an empty string.
Created attachment 356710 [details] [review] genmarshal: Generate C++ guards in compatibility mode When using `--header --body`, we should generate the C++ guards, as we generate the prototypes for the marshallers.
The project I hit this bug on is actually using two invocations to create a header and implementation file. They probably should not be using --header or --prototypes with --body at all - but by default, the body does not include any prototypes header file because it can't know if people will actually be generating a header file and if they do how it will be called. They probably should be using only --body, yes. Although that means that they also need to specify the generated header file (reasoning above.) Wrapping everything in C_{BEGIN,END}_DECLS is pretty much the old behavior (stemming from --header), yeah. I think we can do this more fine-grained though, by only wrapping the prototypes. That should also hold for the implementations.
(In reply to Mihai Moldovan from comment #7) > The project I hit this bug on is actually using two invocations to create a > header and implementation file. They probably should not be using --header > or --prototypes with --body at all - but by default, the body does not > include any prototypes header file because it can't know if people will > actually be generating a header file and if they do how it will be called. That's usually solved, compatibly, by using a compound call, e.g.: ( echo '#include "foo-marshal.h"' && glib-genmarshal --body foo-marshal.list ) > foo-marshal.c > I think we can do this more fine-grained though, by only wrapping the > prototypes. That should also hold for the implementations. Personally, I'm much more confident in using a global C++ guard; glib-genmarshal only guarantees you it's generating C code, not C++. Even if valid C code is somewhat valid C++ code *today*, I cannot make you any guarantee about whatever we generate in the future.
(In reply to Emmanuele Bassi (:ebassi) from comment #8) > That's usually solved, compatibly, by using a compound call, e.g.: > > ( echo '#include "foo-marshal.h"' && > glib-genmarshal --body foo-marshal.list ) > foo-marshal.c The better way to handle this would probably be passing --include-header foo-marshal.h. That is what I was referencing. The project I hit that on (vte) doesn't do any of that, though, which is probably a(/their) bug. > Personally, I'm much more confident in using a global C++ guard; > glib-genmarshal only guarantees you it's generating C code, not C++. Even if > valid C code is somewhat valid C++ code *today*, I cannot make you any > guarantee about whatever we generate in the future. That won't help anyone. 'extern "C"' only changes linkage, nothing else. It's a good idea and I agree in theory, but practically that's sadly not what 'extern "C"' provides.
Review of attachment 356538 [details] [review]: Okay, let's go with this one.
Review of attachment 356710 [details] [review]: We're going to use unconditional guards around prototypes, in case we end up compiling the marshallers with a C++ compiler.