If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

configure test for extern template specializations

VERIFIED FIXED in mozilla1.3beta

Status

SeaMonkey
Build Config
VERIFIED FIXED
15 years ago
13 years ago

People

(Reporter: Benjamin Smedberg, Assigned: hacker formerly known as seawood@netscape.com)

Tracking

Trunk
mozilla1.3beta

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

15 years ago
For bug 180264 I will need a configure test, whether compilers support extern
template specializations. I have a patch that I think will work.
(Reporter)

Comment 1

15 years ago
Created attachment 108552 [details] [diff] [review]
diff -u of configure.in
Our resident language lawyers will have to review the test.  I'm not worthy.

Comment 3

15 years ago
bbaetz is another great guy to review this.
I'm confused. I don't see this used in teh patch on bug 180264. From that bug:

"If the compiler does not support this syntax, we can degrade gracefully by
omitting the extern declaration (at the price of a little code duplication):

extern template myClass<T>;"

I'm not really sure that that will work, though. |extern template| is an
extention, AFAIK - does it even mean the same thing on all compilers?

I'm also not sure how much code duplication there will be, anyway - g++ uses
.linkonce sections to only have the resulting function appear once anyway.

Won't the |extern| suppress inlining, though, whcih is what you're trying to
avoid for bug 180264? (I'm not too sure about that bit, though)
(Reporter)

Comment 5

15 years ago
The basic idea of this syntax is to avoid duplicate instantiations of template
functions *across shared-libraries*.  Compilers fold duplicate instantiations
within a single library, but not across shared libraries.  Instantiate all of
the common templatizations of nsTHashtable within the XPCOM shared lib, and it
won't need to be duplicated in other shared libs.

"extern template class" is an extension, but it has a standard meaning on GCC
and MSVC: it declares all the class functions with external linkage, just like
"extern var".

I want to add this test to bug 180264, because I know that this syntax breaks
mac compilers, and others I'm sure. I would wrap the "extern template class"
declarations in an #ifdef HAVE_CPP_EXTERN_INSTANTIATION to protect Mac and other
compilers that don't support the external linkage.

> Won't the |extern| suppress inlining, though, whcih is what you're trying to
> avoid for bug 180264? (I'm not too sure about that bit, though)

"extern class" shouldn't suppress inlining, but that isn't the point of bug
180264 in any case.  That bug avoids multiple heap allocations in hashtables, by
allocating keys and data "inline" in the hashtable.
(Reporter)

Comment 6

15 years ago
Created attachment 108894 [details] [diff] [review]
patch version 2 (includes changes to nscore.h)

I forgot, that since we don't support configure tests for MSVC, that it had to
be declared manually in nscore.h for Win32 platforms.
(Reporter)

Updated

15 years ago
Attachment #108552 - Attachment is obsolete: true
(Reporter)

Updated

15 years ago
Attachment #108894 - Flags: review?(dbaron)
(Reporter)

Comment 7

15 years ago
Created attachment 109224 [details] [diff] [review]
patch -u without CRs

Same patch, without carriage returns.
Attachment #108894 - Attachment is obsolete: true
(Reporter)

Updated

15 years ago
Attachment #108894 - Flags: review?(dbaron)
(Reporter)

Updated

15 years ago
Attachment #109224 - Flags: review?(scc)

Comment 8

15 years ago
Comment on attachment 109224 [details] [diff] [review]
patch -u without CRs

and did you find that the results from this test matched the actual
capabilities of the compilers?	It looks like a reasonable test to me.

r=scc
Attachment #109224 - Flags: review?(scc) → review+

Comment 9

15 years ago
depending on the result of discussion about maintaining the classic Mac build,
I'll test Metrowerks for compliance and add the appropriate declaration to the
Mac flags section
Patch has been checked in.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.3beta
(Reporter)

Comment 11

15 years ago
Verifying... tinderbox logs look reasonable.  Thanks, cls and scc!
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.