Ignore unused function warnings from external headers when compiling with GCC and Clang

[ashn-dot-dev]

Removes existing unused function warnings generated when compiling functions declared with static linkage in external headers. This changeset suppresses warnings with a set of pragmas around external header includes using the pattern:

#if defined(__GNUC__) // GCC and Clang
    #pragma GCC diagnostic push
    #pragma GCC diagnostic ignored "-Wunused-function"
#endif

// include the libraries here

#if defined(__GNUC__) // GCC and Clang
    #pragma GCC diagnostic pop
#endif

This significantly reduces the warning noise when compiling on Linux, and has been tested with CC=gcc and CC=clang. I do not have a machine with MSVC set up, so I cannot test or confirm the behavior on Windows, but the #if defined(__GNUC__) guard should only have an effect with GCC and Clang and not produce any "unknown pragma GCC" warnings on Windows.

Putting the #pragma GCC diagnostic push and #pragma GCC diagnostic pop code around every external include is a little ugly, but is the simplest way to suppress these warnings for a specific set of includes as far as I can tell.

[raysan5]

@ashn-dot-dev I'm merging this review BUT personally I'm not a fan of #pragma directives, they are compiler-specific and they are more related to build system than code itself. Also, hidding some warning does not look like the best solution to me.

[ashn-dot-dev]

@raysan5 I am also not a huge fan of the pragmas. This PR was made based on some of the information in #2952, specifically:

module: rtext

REVIEW: External libraries warnings. Some of the external libraries used generate warning on compilation, those warnings should be reviewed and the improvements sent to original authors. Most notable case is stb_truetype because most of the exposed advance API is not used by raylib.

I think with libs like stb_truetype we are going to encounter the -Wunused-function warnings as long as the library is compiled with STBTT_STATIC. Same goes for the other header-only libraries that allow consumers to specify static linkage. If there is a better way to silence some of these warnings (which in this case are not actually indicative of errors) then I would love to use that instead. This PR reduces the noise, but I agree that it is ugly. Would it be worth moving this higher up by adding -Wno-unused-function into the Makefile CFLAGS?

[ghost]

Sorry for reposting, but (considering solving these warnings for any platform) would this be worth trying?

void unused_stb_truetype(void)
{
    (void)&fn1;
    (void)&fn2;
    ...
    return;
}

[orcmid]

@ashn-dot-dev

I think with libs like stb_truetype we are going to encounter the -Wunused-function warnings as long as the library is compiled with STBTT_STATIC. Same goes for the other header-only libraries that allow consumers to specify static linkage.

Since static bindings are desired in the first place, isn't another way by (1) compiling such a header-only-library into a standalone implementation object file (and not recompiling it unless there is a new source for it). (2) Then use the header-only-library as a header case without implementation in the app, referencing externals instead. (3) Then include that standalone implementation object at link time. There should then be no problem about unused functions and unused functions will not clutter the resulting executable.

This is pretty much the pattern for static binding to raylib.h functions with compilation of the raylib *.c files into separate object files for static linking into an app. The reduction in size of the executable can be quite noticeable.

Maybe this simple (and economical) case could be explained as an alternative to the #pragma approach. I am uncertain that suppressing the warning does anything about the clutter of an app with unused function code, and all the wasted compile-time that goes into creating such a thing.

This approach also works well if the app executable is itself built from multiple source-file modules and app-specific headers. The linking of it all will still include functions from the implementation object at most once each.

[ashn-dot-dev]

@orcmid

There should then be no problem about unused functions and unused functions will not clutter the resulting executable.

I might be misunderstanding your suggestion, but wouldn't separate compilation require the linkage of the definitions to be extern rather than static in order for them to be visible to the linker. I assumed the reason for using STBTT_STATIC was to keep the stbtt_* functions visible only to the .c file including the library (and ditto for other header-only libs). I am specifically referring to what the C standard calls "internal linkage" here when I say "static", not static vs. dynamic libraries.

[ashn-dot-dev]

@ubkp

RE the unused_stb_truetype idea => It still feels kinda weird... Like I imagine it would add a similar level of clutter to using #pragma, and the #pragma solution is already kinda clutterful. I guess you could go ahead and prepare a changeset with the idea and see how it looks/feels. My gut instinct would be to go another direction, but I don't have any reason to disagree if the end product is better with the unused_stb_truetype implementation. In the end it would ultimately be up to Ray though.

[orcmid]

@ashn-dot-dev

I might be misunderstanding your suggestion, but wouldn't separate compilation require the linkage of the definitions to be extern rather than static in order for them to be visible to the linker.

That's correct. Of course it is also the default for function declarations.

I assumed the reason for using STBTT_STATIC was to keep the stbtt_* functions visible only to the .c file including the library (and ditto for other header-only libs).

I don't understand what the concern is here. What is your insight about that?

The compiled implementation objects are in the possession of the developer. Likewise for the compiled objects of the developer's app. Once linking has been performed there is static binding among the components and with appropriate options, the unused implementations of functions just disappear. The used implementations of functions will be there in compiled form and statically linked. Is there some mistaken notion of privacy or efficiency here?

I am ignoring debugging compiles, speaking only of release versions. Although one can export symbols for release builds I presume that would not the case here. And, as far as I know, compiling internal linkage doesn't prevent discovery of the functions used and the symbols used for them in debug builds.

PS: I can imagine a case where one might want to have the equivalent of static/internal linking within a linkable object. Making a new library object that employed the header-only privately and not wanting to expose its functions in the distributed linkable object. There might be a name-collision avoidance issue as well. That might take some linker magic. I don't think it is an use case that should have any priority in something like raylib.

[ashn-dot-dev]

@orcmid

I don't understand what the concern is here. What is your insight about that?

PS: I can imagine a case where one might want to have the equivalent of static/internal linking within a linkable object. Making a new library object that employed the header-only privately and not wanting to expose its functions in the distributed linkable object. There might be a name-collision avoidance issue as well.

Bingo, this was my thinking. Although I do suppose you are correct in noting that this is a use case that is probably not the highest priority for something like raylib. If a project depending on raylib got to the point where this mattered, then that project would most likely be customizing their build of raylib and additional dependencies anyway. So perhaps separate compilation would be a viable way to go in this case. 👍

[raysan5]

@ashn-dot-dev @ubkp @orcmid thank you very much for your comments and detailed analysis of this issue and possible approaches. Personally I think the best approach is in the build side (-Wno-unused-function or equivalent), considering that it's more related to the build process than the code itself. Also, as commented on the wishlist, it would be nice that unused functions get reviewed on the original library or some flag mechanism added to avoid them (I use that approach on raylib, allowing to support certain features with flags on config.h).

In any case, it's not a dramatic issue, just some compilation warnings, current #pragma approach is ok for now.

@ubkp Your approach is very interesting, good to have a list of the unused functions but I'm afraid the downsides you comment on the PR (#3237) are considerable concerns, not to mention that maybe some of those functions get used on raylib at some point.