Remove NS_SPECIALIZE_TEMPLATE

RESOLVED FIXED in mozilla11

Status

()

Core
XPCOM
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Ms2ger, Assigned: drexler)

Tracking

Trunk
mozilla11
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good_first_bug][mentor=Ms2ger])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
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

6 years ago
Created attachment 578910 [details] [diff] [review]
Patch v1
Attachment #578910 - Flags: feedback?(Ms2ger)
(Reporter)

Comment 2

6 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

6 years ago
Created attachment 578952 [details] [diff] [review]
Patch v2

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

6 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

6 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

6 years ago
Created attachment 580142 [details] [diff] [review]
Patch v3

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

6 years ago
Created attachment 580153 [details] [diff] [review]
Patch v4

Changed to suggested error message.
Attachment #580142 - Attachment is obsolete: true
Attachment #580153 - Flags: review?(khuey)
Attachment #580153 - Flags: review?(khuey) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed

Comment 9

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=73b8ab2861e1

Will push tomorrow presuming green :-)
Status: NEW → ASSIGNED
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/d401db5bf913

\o/
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Depends on: 710491
You need to log in before you can comment on or make changes to this bug.