v10: MinGW DLL build behavior change

[chris-se]

The commit https://github.com/leethomason/tinyxml2/commit/cc4c1df7ec28101eaacc6388cd8a0a857bffd297 caused a huge change in how DLLs are built on MinGW.

Previously, #ifdef _WIN32 was used to check for Windows, and then __declspec(dllexport) and __declspec(dllimport) were used to declare the public API as exported / imported from DLLs. GCC/MinGW also supports those attributes, and everything worked fine.

That specific commit (without any explanation why this was broken before) now changed that to detect just MSVC, and not any Windows system. This is problematic, because

• There are other compilers than MSVC and MinGW on Windows, and to my knowledge all support the __declspec Syntax for DLLs.
• MinGW now uses __attribute__((visibility("default"))), which as no effect at all.

This means that on MinGW after this change no public APIs are marked as exported in the DLL.

Now due to a quirk of GCC this doesn't mean the DLL doesn't work at all, because if GCC compiles a DLL that contains no symbols that are exported, it will assume that all symbols are exported. But this also means that symbols that were not explicitly marked as exported are now exported on MinGW (while they aren't on MSVC).

To demonstrate the behavior, I've created the following test program:

CMakeLists.txt:

cmake_minimum_required(VERSION 3.15)
project(link_test VERSION 1.0)

set(CMAKE_CXX_VISIBILITY_PRESET hidden)
set(CMAKE_VISIBILITY_INLINES_HIDDEN YES)

option(NO_DECLSPEC_MINGW "Don't use __declspec(dllimport/dllexport) on MinGW" OFF)

add_library(link_test link_test.cpp link_test.h)
target_compile_definitions(link_test
    PUBLIC $<$<BOOL:${NO_DECLSPEC_MINGW}>:NO_DECLSPEC_MINGW>
    INTERFACE $<$<BOOL:${BUILD_SHARED_LIBS}>:LINK_TEST_IMPORT>
)
set_target_properties(link_test PROPERTIES DEFINE_SYMBOL "LINK_TEST_EXPORT")

link_test.h:

#ifndef LINK_TEST_H
#define LINK_TEST_H

#if defined(_WIN32) && (!defined(__GNUC__) || !defined(NO_DECLSPEC_MINGW))
#  if defined(LINK_TEST_IMPORT)
#    define   LINK_TEST_API     __declspec(dllimport)
#  elif defined(LINK_TEST_EXPORT)
#    define   LINK_TEST_API     __declspec(dllexport)
#  else
#    define   LINK_TEST_API
#  endif
#elif __GNUC__ >= 4
#  if defined(LINK_TEST_IMPORT) || defined(LINK_TEST_EXPORT)
#    define   LINK_TEST_API     __attribute__((visibility("default")))
#  else
#    define   LINK_TEST_API
#  endif
#else
#  define   LINK_TEST_API
#endif

class LINK_TEST_API DemoClass {
public:
    int getFoo();
};

class NonExportedClass {
public:
    int getBar();
};

#endif // LINK_TEST_H

link_test.cpp:

#include "link_test.h"

int DemoClass::getFoo()
{
    return 42;
}

int NonExportedClass::getBar()
{
    return 23;
}

The DLL compiled with __declspec(dllexport) exports the following symbols (using https://github.com/lucasg/Dependencies):

The DLL compiled with __attribute__((visibility("default"))) exports the following symbols:

In the future this could be even more problematic: if GCC decides to stop exporting all symbols in DLLs without an explicit dllexport marking at some point in the future (don't know if that's planned at all or why GCC behaves like this currently, but it could happen), then the current logic would mean that tinyxml2's DLL would then export no symbols at all.

In my eyes the previous check #ifdef _WIN32 was correct, and I have compiled tinyxml2 on MinGW as a DLL for a couple of years now, without any issues. I think this change should therefore be reverted.