GNOME Bugzilla – Bug 614326
Ensure that G_INLINE_FUNC functions are used inline internally
Last modified: 2017-11-21 12:49:33 UTC
The current pattern of implementing non-inline emissions of functions defined with G_INLINE_FUNC is faulty: G_IMPLEMENT_INLINES is defined in ordinary source files that have some other code, which can use the affected functions. This may lead to suboptimal use of non-inline instances inside GLib.
http://git.collabora.co.uk/?p=user/zabaluev/glib.git;a=shortlog;h=refs/heads/implement-inlines This change consigns all non-inline emissions for functions declared with G_INLINE_FUNC to one dedicated source file, to make sure no other source file needs to have G_IMPLEMENT_INLINES defined and possibly ends up using non-inlined implementations of these functions inside the library.
So the problem here is that inside glib, the compiler may choose to use the non-inline version rather than inlining? Can you please attach a "git format-patch" series of patches rather than linking to a git repository? "git bz" ( http://blog.fishsoup.net/2008/11/16/git-bz-bugzilla-subcommand-for-git/ ) makes it easy to keep a Bugzilla bug in sync with a git series.
Created attachment 157483 [details] [review] The patch
(In reply to comment #2) > So the problem here is that inside glib, the compiler may choose to use the > non-inline version rather than inlining? Yes. > Can you please attach a "git format-patch" series of patches rather than > linking to a git repository? Done; there is only one commit.
1. Can we avoid having G_IMPLEMENT_INLINES in the header files? Something like this for example: ginline.c: #include <ginline.h> ===================== #undef inline #define inline #include <gthread.h> --------------------- 2. Can you add a test to make sure all headers with inline functions are included in ginline.c? Shee various check.sh scripts in glib/glib/.
(In reply to comment #5) > 1. Can we avoid having G_IMPLEMENT_INLINES in the header files? Something like > this for example: > > ginline.c: > > #include <ginline.h> > ===================== > > #undef inline > #define inline > > #include <gthread.h> > --------------------- It's not only about the inline keyword, the non-inline definition of G_INLINE_FUNC sheds 'static' linkage as well.
Created attachment 157859 [details] [review] Include ginline.h in the master glib header
Created attachment 157860 [details] [review] Added a script to check that ginline.c includes all headers with G_INLINE_FUNC
(In reply to comment #5) > 2. Can you add a test to make sure all headers with inline functions are > included in ginline.c? Shee various check.sh scripts in glib/glib/. I have attached my attempt as attachment #157860 [details].
These patches are all redundant, since this was fixed in bug #757374. Sorry for the delay in reviewing them. If there are still problems with inline support inside/outside GLib, please re-open this bug. Thanks. *** This bug has been marked as a duplicate of bug 757374 ***