After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 723899 - G_DEFINE_TYPE() causes compiler warnings with clang due to foo_get_instance_private
G_DEFINE_TYPE() causes compiler warnings with clang due to foo_get_instance_p...
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gobject
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-02-08 13:41 UTC by Sebastian Dröge (slomo)
Modified: 2014-03-08 14:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gtype – Mark _get_instance_private() function as G_GNUC_UNUSED (929 bytes, patch)
2014-02-09 22:09 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Sebastian Dröge (slomo) 2014-02-08 13:41:59 UTC
G_DEFINE_TYPE() recently added a static inline foo_get_instance() function. This is obviously not used by a lot of code out there and as it's static will cause clang to complain about unused functions, e.g.

  CC       libgstreamer_1.0_la-gstobject.lo
gstobject.c:156:1: error: unused function 'gst_object_get_instance_private'
      [-Werror,-Wunused-function]
G_DEFINE_ABSTRACT_TYPE (GstObject, gst_object, G_TYPE_INITIALLY_UNOWNED);
^
/usr/include/glib-2.0/gobject/gtype.h:1375:51: note: expanded from macro
      'G_DEFINE_ABSTRACT_TYPE'
#define G_DEFINE_ABSTRACT_TYPE(TN, t_n, T_P)                G_DEFINE_TYP...
                                                            ^
/usr/include/glib-2.0/gobject/gtype.h:1467:60: note: expanded from macro
      'G_DEFINE_TYPE_EXTENDED'
#define G_DEFINE_TYPE_EXTENDED(TN, t_n, T_P, _f_, _C_)      _G_DEFINE_TY...
                                                            ^
/usr/include/glib-2.0/gobject/gtype.h:1677:24: note: expanded from macro
      '_G_DEFINE_TYPE_EXTENDED_BEGIN'
static inline gpointer \
                       ^
<scratch space>:180:1: note: expanded from here
gst_object_get_instance_private



One could in theory put a
> #pragma clang diagnostic ignored "-Wunused-function"
around every *use* of G_DEFINE_TYPE but that's incredibly ugly.

Or it could be defined as non-static inline function... which might leak into the exported functions then.

Any other ideas? :)
Comment 1 Allison Karlitskaya (desrt) 2014-02-09 21:31:12 UTC
We have G_GNUC_UNUSED for exactly this sort of thing.  Does it help?
Comment 2 Sebastian Dröge (slomo) 2014-02-09 22:09:48 UTC
Created attachment 268607 [details] [review]
gtype – Mark _get_instance_private() function as G_GNUC_UNUSED

clang likes to complain about it being unused.
Comment 3 Sebastian Dröge (slomo) 2014-02-09 22:11:05 UTC
Yes, it does. I thought that it will then complain if the function is actually used... but fortunately it doesn't. With G_GNUC_UNUSED the warning is gone if the function is not used, and there is no warning if it is used.

Ok to push?
Comment 4 Emmanuele Bassi (:ebassi) 2014-02-09 23:59:42 UTC
Review of attachment 268607 [details] [review]:

I don't have any particular objection; it may be interesting to see if gcc/clang generates the same assembly code if the 'unused' attribute is not respected (i.e. if the 'unused' function is actually used).

we could also have a:

  (void) type_name##_get_instance_private (inst);

inside an intern_instance_init() function, which would likely be optimized away, but it would also prevent the warning.
Comment 5 Sebastian Dröge (slomo) 2014-02-14 21:47:02 UTC
That should also work, but then we would need to add such a internal instance_init function that calls the real instance_init.

So how would you prefer to move forward with this?
Comment 6 Emmanuele Bassi (:ebassi) 2014-02-15 11:14:10 UTC
(In reply to comment #5)
> That should also work, but then we would need to add such a internal
> instance_init function that calls the real instance_init.

we already do for class_init(), so it wouldn't be a stretch.

> So how would you prefer to move forward with this?

I'll leave the decision to Ryan.
Comment 7 Allison Karlitskaya (desrt) 2014-02-15 13:37:49 UTC
Review of attachment 268607 [details] [review]:

I prefer this approach -- unless you know anything wrong with it, please commit.
Comment 8 Gustau Pérez i Querol 2014-03-08 08:18:07 UTC
This patch was tested against glib 2.38.2 running FreeBSD 11.0-CURRENT #6 r262728 with clang 3.4 as the compiler. 

We began to have problems with cinnamon's nemo package. It has heavy use of G_DEFINE_TYPE_WITH_CODE macros. One way (as stated in this bug) is to patch the code pushing and poping  -Wno-unused-function to the clang diagnostic stack.

We patched our glib port with the patch attached to this br and the compilation of nemo went fine. We guess many other packages would benefit from that glib patch.

I'd suggest you to commit that patch to glib.
Comment 9 Allison Karlitskaya (desrt) 2014-03-08 14:02:40 UTC
Thanks for the input.  I've pushed this now.