[LLVMdev] [Mesa-dev] [PATCH 1/2 v3] configure: add visibility macro detectionto configure

[Emil Velikov]

On 26/02/15 15:26, Marc Dietrich wrote:
> Am Donnerstag, 26. Februar 2015, 14:44:25 schrieb Emil Velikov:
>> Hi Marc
>>
>> On 17/02/15 09:40, Marc Dietrich wrote:
>>> This adds clang/gcc visibility macro detection to configure and
>>> util/macros.h. This is can be used to conveniently add e.g. a "HIDDEN"
>>> attribute to a function.
>> I believe this should be OK to go in regardless of the status of patch
>> 2. There are just a couple of trivial nitpicks.
> 
> as pointed out, not if there is a better solution. It would be nice if people 
> could test the alternative first.
> 
Can you point out which alternative you have in mind. I'm a bit lost in
this thread.

...
>>> @@ -245,17 +246,13 @@ if test "x$GCC" = xyes; then
>>>
>>>  		   AC_MSG_RESULT([yes]),
>>>  		   [CFLAGS="$save_CFLAGS -Wmissing-prototypes";
>>>  		   
>>>  		    AC_MSG_RESULT([no])]);
>>>
>>> +    CFLAGS=$save_CFLAGS
>>
>> I'm not sure we want/need this one ?
> 
> it restores the CFLAGS from the test above. In fact I just moved it from the 
> blow upwards. Maybe the diff could be shorter.
> 
Indeed it seems like it's slightly busted currently.

Although the extra line does not seem to make it better though :-(

Presently it adds to the final CFLAGS,
-Werror=implicit-function-declaration
-Werror=missing-prototypes

or optionally
-Wmissing-prototypes

but with your change it won't add either one.

...
> I always build with --enable-asm (or better without --disable-asm) and saw no 
> symbol clash. I guess because the required header is not included at the same 
> time. Is this possible at all (asm vs. c files)?
> 
Pretty sure you can - #include and #define are preprocessor commands.
But as you noticed there was no clash, so we're ok.

> Another interesting point is in which case we can build without shared-glapi. 
> I failed to build ES1 or ES2 alone, because it seem to always require glapi. 
Yes, to prevent even greater chaos or build permutations we've been
mandating shared-glapi if more than one GL* api is selected. To solve
this, just drop es{1,2} from the configure line.

> This would mean that the dispatch table always begins with the glapi. Maybe I 
> confuse things, but in this case, just using 
> 	extern void shared_dispatch_stub_0();
> would solve all problems without the HIDDEN hacks.
> 
The so called hack is patch 2, afaict. Having a single generic
definition of the attribute sounds like good patch regardless of that patch.

I'm guessing scons and Android could use a
-DHAVE_FUNC_ATTRIBUTE_VISIBILITY somewhere but that can be done as a
follow-up.

Emil