Closed Bug 312361 Opened 15 years ago Closed 15 years ago

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

Categories

(NSPR :: NSPR, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wtc, Assigned: wtc)

Details

(Keywords: fixed1.8.1)

Attachments

(1 file)

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.
Attached patch Proposed patchSplinter Review
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
Attachment #199459 - Flags: review?(bryner)
Comment on attachment 199459 [details] [diff] [review]
Proposed patch

This should be ok, I think.
Attachment #199459 - Flags: review?(bryner) → review+
Patch checked in on the NSPR tip (NSPR 4.7) and the
NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.9a).
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [4.7]
This patch should be carried back to the NSPR_4_6_BRANCH
and MOZILLA_1_8_BRANCH.
Whiteboard: [4.7]
Target Milestone: --- → 4.7
Attachment #199459 - Flags: approval1.8.1?
Attachment #199459 - Flags: approval1.8.0.1?
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
Target Milestone: 4.7 → 4.6.2
Comment on attachment 199459 [details] [diff] [review]
Proposed patch

not for 1.8.0.1
Attachment #199459 - Flags: approval1.8.1?
Attachment #199459 - Flags: approval1.8.1+
Attachment #199459 - Flags: approval1.8.0.1?
Attachment #199459 - Flags: approval1.8.0.1-
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
Keywords: fixed1.8.1
You need to log in before you can comment on or make changes to this bug.