Re: [Mesa-dev] [PATCH 2/2] Don't use -fvisibilty=hidden on cygwin

2011-04-27 Thread Dan Nicholson
On Tue, Apr 26, 2011 at 5:00 AM, Jon TURNEY jon.tur...@dronecode.org.uk wrote:
 Using -fvisibility=hidden makes the current cygwin gcc emit a warning
 warning: visibility attribute not supported in this configuration; ignored
 for every single non-static symbol

 $ make clean  make 21 | grep ignored | wc -l
 6457

 This is pretty annoying and makes it hard to spot other errors and warnings
 in the compiler output.

 It's not clear how to fix this.  Possibly the correct solution is explicitly
 decorate all symbols with macros which annotate the expected visibility and
 evaluate to attribute(hidden/default) or dllexport depending on the target,
 as suggested at [1], but this would be a lot of work and misses the main
 advantage of -fvisibility=hidden (that you don't have to explicitly annotate
 everything :-)), so I guess we are supposed to know when -fvisibility isn't
 going to be useful and not use it.

 [1] http://gcc.gnu.org/wiki/Visibility

 Alternatively, the configuration check could be made more complex to avoid
 using this flag is the target isn't ELF or using the flag generates a warning

You could add -Werror to the CFLAGS used for the visibility test since
any host that's throwing warnings for something so simple probably has
issues with visibility. Something like:

diff --git a/configure.ac b/configure.ac
index 8989c2b..ec662a3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -152,7 +152,7 @@ if test x$GCC = xyes; then
 save_CFLAGS=$CFLAGS
 AC_MSG_CHECKING([whether $CC supports -fvisibility=hidden])
 VISIBILITY_CFLAGS=-fvisibility=hidden
-CFLAGS=$CFLAGS $VISIBILITY_CFLAGS
+CFLAGS=$CFLAGS $VISIBILITY_CFLAGS -Werror
 AC_LINK_IFELSE([AC_LANG_PROGRAM()], AC_MSG_RESULT([yes]),
   [VISIBILITY_CFLAGS=; AC_MSG_RESULT([no])]);

Does that do the right thing?

--
Dan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] Don't use -fvisibilty=hidden on cygwin

2011-04-27 Thread Jon TURNEY
On 27/04/2011 11:39, Dan Nicholson wrote:
 On Tue, Apr 26, 2011 at 5:00 AM, Jon TURNEY wrote:
 Alternatively, the configuration check could be made more complex to avoid
 using this flag is the target isn't ELF or using the flag generates a warning
 
 You could add -Werror to the CFLAGS used for the visibility test since
 any host that's throwing warnings for something so simple probably has
 issues with visibility. Something like:
 
 diff --git a/configure.ac b/configure.ac
 index 8989c2b..ec662a3 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -152,7 +152,7 @@ if test x$GCC = xyes; then
  save_CFLAGS=$CFLAGS
  AC_MSG_CHECKING([whether $CC supports -fvisibility=hidden])
  VISIBILITY_CFLAGS=-fvisibility=hidden
 -CFLAGS=$CFLAGS $VISIBILITY_CFLAGS
 +CFLAGS=$CFLAGS $VISIBILITY_CFLAGS -Werror
  AC_LINK_IFELSE([AC_LANG_PROGRAM()], AC_MSG_RESULT([yes]),
[VISIBILITY_CFLAGS=; AC_MSG_RESULT([no])]);
 
 Does that do the right thing?

Yes, this works, and in fact this was the approach I initially tried, and
seems more elegant.

However, I was concerned about false negatives.  As far as I can tell, nothing
guarantees that the test program compiled by AC_LANG_PROGRAM() compiles
without warnings, only without errors, so using -Werror might cause this test
to fail for reasons unrelated to -fvisibility.

Looking at this a bit more, though, a bit of googling seems to indicate we
wouldn't be the first to use this construction, so perhaps it's ok :-)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] Don't use -fvisibilty=hidden on cygwin

2011-04-26 Thread Jon TURNEY
Using -fvisibility=hidden makes the current cygwin gcc emit a warning
warning: visibility attribute not supported in this configuration; ignored
for every single non-static symbol

$ make clean  make 21 | grep ignored | wc -l
6457

This is pretty annoying and makes it hard to spot other errors and warnings
in the compiler output.

It's not clear how to fix this.  Possibly the correct solution is explicitly
decorate all symbols with macros which annotate the expected visibility and
evaluate to attribute(hidden/default) or dllexport depending on the target,
as suggested at [1], but this would be a lot of work and misses the main
advantage of -fvisibility=hidden (that you don't have to explicitly annotate
everything :-)), so I guess we are supposed to know when -fvisibility isn't
going to be useful and not use it.

[1] http://gcc.gnu.org/wiki/Visibility

Alternatively, the configuration check could be made more complex to avoid
using this flag is the target isn't ELF or using the flag generates a warning

For the moment, I've gone for the crude solution of explicitly disabling
the use of this flag for the cygwin target

Signed-off-by: Jon TURNEY jon.tur...@dronecode.org.uk
---
 configure.ac |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index 37ea5e7..b1e1de1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -182,6 +182,15 @@ if test x$GXX = xyes; then
 CXXFLAGS=$CXXFLAGS -fno-strict-aliasing
 fi
 
+dnl even if the compiler appears to support it, using visibility attributes 
isn't
+dnl going to do anything useful currently on cygwin apart from emit lots of 
warnings
+case $host_os in
+cygwin*)
+VISIBILITY_CFLAGS=
+VISIBILITY_CXXFLAGS=
+;;
+esac
+
 AC_SUBST([VISIBILITY_CFLAGS])
 AC_SUBST([VISIBILITY_CXXFLAGS])
 
-- 
1.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev