Reconsider forcibly exporting format_error symbol on GCC

[feltech]

In e0fc0e8 the export attribute macro for the format_error exception in format.h was switched from the conditional FMT_API to the unconditional FMT_VISIBILITY("default")

This causes an ODR violation when fmt is bundled in a (shared) library, intended as a private internal dependency, but consumers of the library are using their own copy of fmt too.

Kinda tricky to show in a Compiler Explorer example. A useful nm command-line to check exported symbols in a library:

nm --demangle --dynamic --defined-only --extern-only mylibrary.so | grep fmt

After reverting to 9.1.0, nm reveals no errant public symbols.

[vitaut]

We can't use FMT_API there because of windows but we could allow overriding FMT_VISIBILITY similarly to other macros. A PR is welcome.

[feltech]

We can't use FMT_API there because of windows

Could you elaborate on that a little? From what I understand, the usage of FMT_VISIBILITY rather than FMT_API on Windows means that the format_error exception is not exported at all on Windows? I'd be curious to understand why (I'm no Windows guru, but sadly still have to build for it).

Am I correct in my understanding that if we're using fmt header-only, then FMT_API expands to nothing (v9 and v10), so nothing is exported at all regardless of platform (except for format_error in v10 on GCC/Clang - i.e. this issue)? So we don't have to worry about Windows shenanigans when header-only?

[vitaut]

I don't remember the details of windows issues from the top of my head and it's irrelevant anyway because you need not to export something on windows but avoid changing symbol visibility on POSIX. And this should be achieved by means of changing the definition of FMT_VISIBILITY and not by switching to FMT_API.

[vitaut]

So I think the correct solution is to define FMT_VISIBILITY to the default visibility if defined(FMT_LIB_EXPORT) || defined(FMT_SHARED) and to nothing otherwise.

[feltech]

The fix in #3627 looks good to me.

I'd be interested, just to plug a gap in my own knowledge, if anyone can explain why this special case for format_error is needed?

[vitaut]

As commented on the PR we don't need a special case there and should change FMT_VISIBILITY directly.