Correctly set default visibility on non-windows compilers

[milianw]

trafficstars

This fixes compilation of pugixml with -fvisibility=hidden. Without this patch, one would get lots of unresolved symbols when consuming pugixml as a shared library.

[milianw]

Just for context, see e.g. https://github.com/gabime/spdlog/commit/58e2b455fb99e00d263f1ceb0c18a5ac6a0d1578

[zeux]

This should not be done in pugixml.hpp - it should instead be done in CMakeLists.txt similarly to dllexport, when configuring pugixml-shared project.

The reason for why it's wrong to do it in pugixml.hpp is that when pugixml is built as a static library that is then linked into a shared library, you usually don't want pugixml symbols to be exposed - they would need to be private.

[milianw]

@zeux sorry for the long delay, what about this version?

[zeux]

Should this be part of PRIVATE definitions? When I test this patch, I get the dependent modules to use the default visibility switch, but pugixml.cpp itself is built without it, presumably because INTERFACE settings don't affect the .so build itself.

[milianw]

ah yes I believe you are right @zeux - generally though I wonder - would you be opposed if I just refactor the code to rely on GenerateExportHeader instead? see https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html

[zeux]

I think we should stick to the current solution for now. It's not clear what the interaction between export headers and the existing pugiconfig would be (what if the users have changed pugiconfig, will CMake overwrite that? etc.), and defining the macros seems cleaner at this point.

[neheb]

should this not be dllimport instead of dllexport?

edit: nvm that should be done by consumers of pugixml. Maybe in pkgconfig.