Closed
Bug 706010
Opened 14 years ago
Closed 14 years ago
Remove NS_SPECIALIZE_TEMPLATE
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: Ms2ger, Assigned: drexler)
References
Details
(Whiteboard: [good_first_bug][mentor=Ms2ger])
Attachments
(1 file, 3 obsolete files)
25.05 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
We're using 'template<>' and 'template <>' all over the place without ifdefs and nobody complained, afaict. Filing this bug to replace the macro and its remaining users, as well as the HAVE_CPP_MODERN_SPECIALIZE_TEMPLATE_SYNTAX define.
See:
http://mxr.mozilla.org/mozilla-central/ident?i=NS_SPECIALIZE_TEMPLATE
http://mxr.mozilla.org/mozilla-central/search?string=HAVE_CPP_MODERN_SPECIALIZE_TEMPLATE_SYNTAX&find=&findi=&filter=&hitlimit=&tree=mozilla-central
Assignee | ||
Comment 1•14 years ago
|
||
Attachment #578910 -
Flags: feedback?(Ms2ger)
Reporter | ||
Comment 2•14 years ago
|
||
Comment on attachment 578910 [details] [diff] [review]
Patch v1
>--- a/xpcom/base/nscore.h
>+++ b/xpcom/base/nscore.h
>@@ -389,39 +389,32 @@ typedef PRUint32 nsrefcnt;
> |template <>| notation. The compiler that don't understand this notation
> just omit it for specialization.
>
> Need to add an autoconf test for this.
> */
You'll want to fix that comment as well. Looks good otherwise.
Attachment #578910 -
Flags: review?(khuey)
Attachment #578910 -
Flags: feedback?(Ms2ger)
Attachment #578910 -
Flags: feedback+
Assignee | ||
Comment 3•14 years ago
|
||
Comment fixed in xpcom\base\nscore.h
Attachment #578910 -
Attachment is obsolete: true
Attachment #578910 -
Flags: review?(khuey)
Attachment #578952 -
Flags: feedback?(Ms2ger)
Reporter | ||
Comment 4•14 years ago
|
||
Comment on attachment 578952 [details] [diff] [review]
Patch v2
Thanks!
Attachment #578952 -
Flags: review?(khuey)
Attachment #578952 -
Flags: feedback?(Ms2ger)
Attachment #578952 -
Flags: feedback+
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → andrew.quartey
Comment on attachment 578952 [details] [diff] [review]
Patch v2
Review of attachment 578952 [details] [diff] [review]:
-----------------------------------------------------------------
::: configure.in
@@ -4005,5 @@
> ac_cv_cpp_modern_specialize_template_syntax=yes,
> ac_cv_cpp_modern_specialize_template_syntax=no)])
> -if test "$ac_cv_cpp_modern_specialize_template_syntax" = yes ; then
> - AC_DEFINE(HAVE_CPP_MODERN_SPECIALIZE_TEMPLATE_SYNTAX)
> -fi
If we're going to require that the compiler support this we want this to be a configure error, no?
Attachment #578952 -
Flags: review?(khuey) → review-
Assignee | ||
Comment 6•14 years ago
|
||
changes to configure.in in /src and js/src to indicate error if compiler doesn't support template specialization.
Attachment #578952 -
Attachment is obsolete: true
Attachment #580142 -
Flags: review?(khuey)
Comment on attachment 580142 [details] [diff] [review]
Patch v3
Review of attachment 580142 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks for fixing that. r=me on the build bits.
::: configure.in
@@ +4005,5 @@
> ac_cv_cpp_modern_specialize_template_syntax=yes,
> ac_cv_cpp_modern_specialize_template_syntax=no)])
> +if test "$ac_cv_cpp_modern_specialize_template_syntax" = no ; then
> + AC_MSG_ERROR([The C++ compiler's version: $CXX_VERSION does not support template specialization. See https://developer.mozilla.org/En/Developer_Guide])
> +fi
Just "The C++ compiler does not support template specialization" is sufficient, I think.
Attachment #580142 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 8•14 years ago
|
||
Changed to suggested error message.
Attachment #580142 -
Attachment is obsolete: true
Attachment #580153 -
Flags: review?(khuey)
Attachment #580153 -
Flags: review?(khuey) → review+
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Comment 9•14 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=73b8ab2861e1
Will push tomorrow presuming green :-)
Status: NEW → ASSIGNED
Flags: in-testsuite-
Comment 10•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in
before you can comment on or make changes to this bug.
Description
•