839347 - Mac opt builds spam hundreds of "skia/SkPostConfig.h:73:25: warning: unused function 'SkNO_RETURN_HINT' [-Wunused-function]"

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Daniel Holbert [:dholbert]]

On Mac opt builds, we spam 342 instances of a warning like this:
{
SourceSurfaceSkia.cpp
In file included from ../../../../gfx/2d/DrawTargetCairo.cpp:11:
In file included from ../../../../gfx/2d/ScaledFontBase.h:12:
In file included from ../../dist/include/skia/SkTypeface.h:13:
In file included from ../../dist/include/skia/SkAdvancedTypefaceMetrics.h:13:
In file included from ../../dist/include/skia/SkRect.h:13:
In file included from ../../dist/include/skia/SkPoint.h:13:
In file included from ../../dist/include/skia/SkMath.h:13:
In file included from ../../dist/include/skia/SkTypes.h:15:
../../dist/include/skia/SkPostConfig.h:73:25: warning: unused function 'SkNO_RETURN_HINT' [-Wunused-function]
            inline void SkNO_RETURN_HINT() {}
                        ^
1 warning generated.
}
Sample clobber-build log:
  https://tbpl.mozilla.org/php/getParsedLog.php?id=19552151&tree=Try

This is a warning in a header, so we spam an instance of the warning for every file that (indirectly) includes SkPostConfig.h. :-/

This generates tons of spam in our build logs, and it's one of the things blocking bug 839269.

This is in an upstream library, but it's a file we've made other tweaks to (I think I saw a .patch file for it in-tree), so I suspect we can fix it and upstream the fix, rather than having to wait for upstream to fix it.

Comment 1

[Daniel Holbert [:dholbert]]

MXR link:
https://mxr.mozilla.org/mozilla-central/source/gfx/skia/include/core/SkPostConfig.h#71

So I *think* this only happens in opt builds because debug builds end up invoking this function via SkASSERT()  (which, in debug builds, invokes SK_DEBUGBREAK, which invokes SkNO_RETURN_HINT, the function in question here)

In opt builds, SkASSERT is turned off (it's #defined to be nothing), so there, we don't end up invoking SkNO_RETURN_HINT.

Anyway -- I think we might want to just remove the "namespace { ... }" around SkNO_RETURN_HINT. That's what puts it in an anyonymous namespace, e.g. file-scope to whatever .cpp file is including it.  So, it's what prompts the compiler to warn when it doesn't find any callers in that .cpp file.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix: remove anonymous namespace

Comment 3

[Daniel Holbert [:dholbert]]

Some googling turned up similar bug in WebRTC:
 https://code.google.com/p/webrtc/issues/detail?id=1228
(w/ clang warning about an unused method defined in an anonymous/unnamed namespace in a header file)

On that bug, cpeterson pointed out that Google's C++ style guide only recommends unnamed namespaces in .cc files (not in headers):
https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces

So, this makes the file conform more to Google's C++ standards, which means it's likely to be the right fix (and likely to be accepted upstream, I imagine).

Comment 4

[Daniel Holbert [:dholbert]]

pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/08fb57d88c01

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/08fb57d88c01