719698 - GCC 4.6 build warning: thread_helper.h:107:13: warning: ‘bool mozilla::tls::create(mozilla::tls::key*)’ defined but not used [-Wunused-function]

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Daniel Holbert [:dholbert]]

We get 15 instances of this build warning when building:

dist/include/thread_helper.h:107:13: warning: ‘bool mozilla::tls::create(mozilla::tls::key*)’ defined but not used [-Wunused-function]

This function is declared as "static", which makes it trigger this warning when it's defined but unused.

At least in some cases, this happens for .cpp files that #include sampler.h, which #includes sps_sampler.h, which #includes thread_helper.h (& thereby gets a static "create" impl) but never uses create().

According to the GCC documentation for this this warning, GCC only warns when _non-inline_ static methods go unused.  So, we should be able to silence this warning (and hint to the compiler to optimize away this method when it's unused) by labeling it "inline".

Comment 1

[Daniel Holbert [:dholbert]]

Ah, looks like this was already fixed-when-checked-in in the "#if defined(XP_WIN)" chunk of this file:
> 84  static inline bool create(key* mykey) {
but not in the "#else" chunk:
> 107 static bool create(key* mykey) {
http://mxr.mozilla.org/mozilla-central/source/tools/profiler/sps/thread_helper.h?mark=84-84,107-107#84

Also -- as noted in the first comment on bug 714422, clang complains when "static inline" methods go unused, even if GCC doesn't.  So rather than making these methods "static inline", I think we want to *actually* just make them "inline".  I don't think we're getting any benefit from the "static" keyword right now here anyway, right?

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix v1: s/static inline/inline/ & s/static/inline/

Comment 3

[Daniel Holbert [:dholbert]]

Landed:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/b7684cc4cb78

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b7684cc4cb78