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

RESOLVED FIXED in 4.6.2

Status

defect
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: wtc, Assigned: wtc)

Tracking

({fixed1.8.1})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Assignee

Description

14 years ago
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.
Assignee

Comment 1

14 years ago
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
Assignee

Updated

14 years ago
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+
Assignee

Comment 3

14 years ago
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: 14 years ago
Resolution: --- → FIXED
Whiteboard: [4.7]
Assignee

Comment 4

14 years ago
This patch should be carried back to the NSPR_4_6_BRANCH
and MOZILLA_1_8_BRANCH.
Whiteboard: [4.7]
Target Milestone: --- → 4.7

Updated

14 years ago
Attachment #199459 - Flags: approval1.8.1?
Attachment #199459 - Flags: approval1.8.0.1?
Assignee

Comment 5

14 years ago
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-
Assignee

Comment 7

14 years ago
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.