Supressing a warning for unused macro

[namandixit]

If thread_local is declared as a macro (as is done in Glibc), the LIBCO_MP macro never comes into use. On Clang, this results is a -Wunused-macros warning.

This commit adds some dummy code to trick the compiler into thinking that the macro is in fact being used. Unless the compilers give an option to disable these warnings for particular symbols, this is the best workaround AFAIK.

[Screwtapello]

First, sorry it's taken me so long to get around to reviewing this PR.

Secondly, I confess I'm confused. Looking at settings.h, so far as I can tell the basic rules are:

• if the user provides their own custom definition of thread_local, use that
• if we're building with a C11 or higher compiler, use <thread.h> to provide thread_local
• if we're building with a C++11 or higher compiler, thread_local is already a keyword
• if we're building with GCC, Clang or MSVC we can make our own thread_local
• otherwise, pretend it's meaningless

The thread_local definition provided by glibc should not be present unless we're building with a C11 or higher compiler, and then only after we've already checked LIBCO_MP, so I'm not sure how the circumstances you describe can arise.

There is a special rule for environments where thread_local is not defined but LIBCO_MP is... but LIBCO_MP is not mentioned anywhere else in the code or in the documentation, and the value of LIBCO_MP is never actually used for anything, it's a flag that disables something rather than enables it.

My gut reaction is if somebody's using a multithreading-aware compiler in multithreading mode and wants to build multithreading-ignorant code, they should probably -Dthread_local on the compiler command-line, not just for libco but for all the third-party libraries they want to use. References to LIBCO_MP can be removed from settings.h and the problem goes away.

Alternatively, LIBCO_MP should be defined positively — that is, defining it should enable something that otherwise wouldn't happen. If it's something that we want to happen by default, we can make it a #define at the top of settings.h and let people comment or uncomment it as they see fit. If LIBCO_MP is actually tested directly, I imagine clang will be happy with it, and the problem goes away.

[namandixit]

Yeah, I should have explained it better. Imagine the following scenario:

Someone is planning on using libco in a multi-threaded environment. So, they go forward and define LIBCO_MP, and then include the settings.h (perhaps indirectly).

Now, some LibCs define thread_local as a macro (perhaps expanding to __thread or something). In this case, the entire macro-conditional-spaghetti for selecting correct thread_local is short circuited. However, this also means that the only usage of LIBCO_MP is skipped over. The compiler notices this and complains that the LIBCO_MP macro – that the user defined – never actually got used anywhere.

In this situation, there are two options: either the user figures out a way to silence the warning (which in my opinion is bad UX), or the library does it for the user. This PR tries to do take the second path, by silencing this warning through the only portable way I could figure out.

References to LIBCO_MP can be removed from settings.h and the problem goes away.

This might break the builds of the existing users of this library, since they might be relying to LIBCO_MP to magically make libco thread-aware. Remember that LIBCO_MP was the only way of doing this, and I tried to preserve that property in https://github.com/higan-emu/libco/pull/7.

[Screwtapello]

So, they go forward and define LIBCO_MP, and then include the settings.h (perhaps indirectly).

This confuses me. As far as I understand, libco users should never include settings.h, only libco.h (and libco.h does not include settings.h). The only way to include settings.h would be to include it directly (which is unnecessary), or to include it indirectly via libco.c, which seems like a Bad Plan.

[namandixit]

Ah, I see. Yes, in my project, I am directly including libco.c since I use a Unity build. I am not sure if this problem will also exist if -DLIBCO_MP was passed to the compiler through the Makefile, as would be done in more conventional setups. Regardless, feel free to close this since this is not the common case (and I don't particularly like the workaround either, but couldn't figure out anything better).

[Screwtapello]

When just compiling libco.c on its own, with -DLIBCO_MP defined on the command line, no warnings are issued:

$ clang -DLIBCO_MP -c libco.c
$ clang --version
Debian clang version 11.0.1-2
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

If you find a neat solution for unity builds, that might be good to put in the documentation somewhere. However, I suspect for unity builds to work correctly, you'd need to be able to push a new pre-processor context, do your defines and includes, then pop the context to discard any changes to the pre-processor state, and I don't think any widely-available CPP supports such a feature. :/