Last Comment Bug 706010 - Remove NS_SPECIALIZE_TEMPLATE
: Remove NS_SPECIALIZE_TEMPLATE
Status: RESOLVED FIXED
[good_first_bug][mentor=Ms2ger]
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Andrew Quartey [:drexler]
:
: Nathan Froyd [:froydnj]
Mentors:
Depends on: 710491
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-29 00:43 PST by :Ms2ger (⌚ UTC+1/+2)
Modified: 2012-02-01 13:58 PST (History)
3 users (show)
emorley: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (24.42 KB, patch)
2011-12-04 09:22 PST, Andrew Quartey [:drexler]
Ms2ger: feedback+
Details | Diff | Splinter Review
Patch v2 (24.69 KB, patch)
2011-12-04 16:05 PST, Andrew Quartey [:drexler]
khuey: review-
Ms2ger: feedback+
Details | Diff | Splinter Review
Patch v3 (25.21 KB, patch)
2011-12-08 12:27 PST, Andrew Quartey [:drexler]
khuey: review+
Details | Diff | Splinter Review
Patch v4 (25.05 KB, patch)
2011-12-08 12:49 PST, Andrew Quartey [:drexler]
khuey: review+
Details | Diff | Splinter Review

Description :Ms2ger (⌚ UTC+1/+2) 2011-11-29 00:43:25 PST
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
Comment 1 Andrew Quartey [:drexler] 2011-12-04 09:22:16 PST
Created attachment 578910 [details] [diff] [review]
Patch v1
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2011-12-04 14:03:31 PST
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.
Comment 3 Andrew Quartey [:drexler] 2011-12-04 16:05:08 PST
Created attachment 578952 [details] [diff] [review]
Patch v2

Comment fixed in xpcom\base\nscore.h
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2011-12-05 05:09:35 PST
Comment on attachment 578952 [details] [diff] [review]
Patch v2

Thanks!
Comment 5 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-12-07 12:27:02 PST
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?
Comment 6 Andrew Quartey [:drexler] 2011-12-08 12:27:45 PST
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.
Comment 7 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-12-08 12:30:35 PST
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.
Comment 8 Andrew Quartey [:drexler] 2011-12-08 12:49:13 PST
Created attachment 580153 [details] [diff] [review]
Patch v4

Changed to suggested error message.
Comment 9 Ed Morley [:emorley] 2011-12-12 18:07:33 PST
https://tbpl.mozilla.org/?tree=Try&rev=73b8ab2861e1

Will push tomorrow presuming green :-)
Comment 10 Ed Morley [:emorley] 2011-12-13 06:32:43 PST
https://hg.mozilla.org/mozilla-central/rev/d401db5bf913

\o/

Note You need to log in before you can comment on or make changes to this bug.