312361 - Determine if the visibility("default") attribute is supported by using GCC version only

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Wan-Teh Chang]

NSPR header files have always been "self sufficient",
which means that our clients don't need to define any
macro when including our header files.

In bug 273336 we changed that.  prtypes.h now tests the
HAVE_VISIBILITY_PRAGMA macro, which our callers need to
define if the compiler (GCC) supports the visibility pragma:

#ifdef HAVE_VISIBILITY_PRAGMA
#define PR_VISIBILITY_DEFAULT __attribute__((visibility("default")))
#else
#define PR_VISIBILITY_DEFAULT
#endif

After a lot of research and experiments, I concluded that
it is safe to replace that test with a test of the GCC version
using GCC's built-in macros.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch

I looked at the GCC 3.0 - 4.0 manuals and found that the
visibility attribute is first documented in the GCC 3.3
manual.

Testing the GCC version works provided that no one (individual
or Linux distribution) has backported the visibility pragmas
or the -fvisibility flag to their special build of GCC 3.2.x
or earlier, because the visibility pragmas and the -fvisibility
flag are what make it crucial to define PR_VISIBILITY_DEFAULT
to be __attribute__((visibility("default"))).

-fvisilibility is new in GCC 4.0.0, so I doubt anyone would
backport it to GCC 3.2.x or earlier.  (Red Hat backported it
to its special build of GCC 3.4.x.)  I believe that the visibility
pragmas were added in the same GCC patch, but I'll need bryner to
confirm that.

I've tested this patch on
- Red Hat Enterprise Linux 3, gcc 3.2
- Red Hat Enterprise Linux 4, gcc 3.4 and gcc 4.0
- Mac OS X (unknown version), gcc 3.3
- Mac OS X Tiger, gcc 4.0

Comment 2

[Brian Ryner (not reading)]

Comment on attachment 199459 [details] [diff] [review]
Proposed patch

This should be ok, I think.

Comment 3

[Wan-Teh Chang]

Patch checked in on the NSPR tip (NSPR 4.7) and the
NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.9a).

Comment 4

[Wan-Teh Chang]

This patch should be carried back to the NSPR_4_6_BRANCH
and MOZILLA_1_8_BRANCH.

Comment 5

[Wan-Teh Chang]

I carried the patch back to the NSPR_4_6_BRANCH (NSPR 4.6.2).

Checking in prtypes.h;
/cvsroot/mozilla/nsprpub/pr/include/prtypes.h,v  <--  prtypes.h
new revision: 3.30.2.1; previous revision: 3.30
done

Comment 6

[Daniel Veditz [:dveditz]]

Comment on attachment 199459 [details] [diff] [review]
Proposed patch

not for 1.8.0.1

Comment 7

[Wan-Teh Chang]

I checked in the patch on the MOZILLA_1_8_BRANCH
for Mozilla/Gecko 1.8.1.

Checking in prtypes.h;
/cvsroot/mozilla/nsprpub/pr/include/prtypes.h,v  <--  prtypes.h
new revision: 3.20.4.10.22.1; previous revision: 3.20.4.10
done