[PATCH libICE] Enable visibility annotations

[Emil Velikov]

On 16 May 2016 at 14:59, Jon Turney <jon.turney at dronecode.org.uk> wrote:
> On 12/05/2016 14:16, Emil Velikov wrote:
>>
>> On the WIN32 (original?) topic
>>  - _X_EXPORT implemented via __declspec dllimport/dllexport
>> Works fine on mingw-w64 - and the resulting binary has only the
>> annotated symbols as exported.
>>  - fvisibility=hidden
>> Does nothing on mingw64, although it does not cause any warning/errors.
>>
>> So in general something like the following should work
>> Note: mingw and cygwin will need to drop fvisibility and/or alike and
>> set the BUILDING_DLL define
>>
>> --- a/Xfuncproto.h.orig
>> +++ b/Xfuncproto.h.new
>> @@ -89,8 +89,15 @@ in this Software without prior written
>> authorization from The Open Group.
>> #endif /* GNUC >= 4 */
>>
>> /* Added in X11R6.9, so available in any version of modular xproto */
>> -#if (__has_attribute(visibility) || (defined(__GNUC__) && (__GNUC__ >=
>> 4))) \
>> -    && !defined(__CYGWIN__) && !defined(__MINGW32__)
>> +#if defined(_WIN32) || defined(__CYGWIN__) && defined(__MINGW32__)
>
>
> I think this should be __CYGWIN__ || __MINGW32__, but otherwise this seems
> correct.
>
Indeed it should. Thanks.

>> +# ifdef BUILDING_DLL // should we prefix this with _X_ ?
>> +#  define _X_EXPORT __declspec(dllexport)
>> +# else
>> +#  define _X_EXPORT __declspec(dllimport)
>> +# endif
>> +# define _X_HIDDEN
>> +# define _X_INTERNAL
>> +#elif __has_attribute(visibility) || (defined(__GNUC__) && (__GNUC__ >=
>> 4))
>> # define _X_EXPORT      __attribute__((visibility("default")))
>> # define _X_HIDDEN      __attribute__((visibility("hidden")))
>> # define _X_INTERNAL    __attribute__((visibility("internal")))
>>
>>
>> Jon, have I lost the plot with any of the above ? Please don't be shy
>> to say "yes you have" ;-)
>> Can you give it a quick test on your end ?
>
>
> This also requires updating everything which uses Xfuncproto.h and builds a
> shared library to define _X_BUILDING_DLL (or whatever the define ends up
> being called)
>
One could add 'the lot' into a m4 function/macro and each user can
just add a single line in their configure.ac. Or even fold it in
XORG_DEFAULT_OPTIONS and alike ?

'the lot' being
 - Sets _X_BUILDING_DLL, where needed
 - Does the fvisibility=hidden and alike magic
 - and optionally adds a configure --enable-symbol-visibilty.

Personally I think the last one should not be needed.

> Unfortunately, this isn't a project I can look at right now, but don't let
> that block fixing this on non-Win32 platforms...
>
Ack. Thanks you very much for the input.

-Emil