On Thu, 5 Aug 2021 at 03:01, Zebediah Figura (she/her) zfigura@codeweavers.com wrote:
On 8/4/21 7:48 AM, Henri Verbeet wrote:
On Wed, 4 Aug 2021 at 05:53, Zebediah Figura zfigura@codeweavers.com wrote:
+#if defined(__GNUC__) && !defined(__MINGW32__) +# define VKD3D_API __attribute__((visibility("default"))) +#else +# define VKD3D_API +#endif
Arguably this is more about _WIN32 than about __MINGW32__. On _WIN32 we'd want to add dllimport/dllexport instead, perhaps as a followup.
I was following the current code here (so one could argue that s/__MINGW32__/_WIN32/ deserves a followup anyway).
That said, it's not obvious to me why this should be _WIN32. My understanding is that mingw doesn't support visibility attributes at all, hence the empty definition, but presumably we'd still want to mark visibility on ELF win32 targets (which I guess means winelib).
True, it's perhaps more about the executable format than the platform, but I'd argue those are tied together a bit stronger than the compiler and the platform. E.g., in the hypothetical case of compiling with MSVC we'd want to use dllimport/dllexport there as well. In any case, the way I'd imagine this eventually looking for e.g. vkd3d-shader would be something like this:
#if defined(_WIN32) # if defined(LIBVKD3D_SHADER_SOURCE) # define VKD3D_API __declspec(dllexport) # else # define VKD3D_API __declspec(dllimport) # endif #elif defined(__GNUC__) # define VKD3D_API __attribute__((visibility("default"))) #else # define VKD3D_API #endif
It's certainly fine to keep using __MINGW32__ here if you prefer though; we don't really support compilers other than gcc (although Chip has been adding some Clang support lately), and this would hardly be the only place where we take __MINGW32__ and _WIN32 to essentially mean the same thing.