[C++] swap

Started by
40 comments, last by iMalc 14 years, 7 months ago
Quote:Original post by MaulingMonkey
Quote:Original post by loufoque
That website is wrong.

Why? What does relying on ADL gain you? I can see what it loses you: performance when someone fucks up and calls the qualified std::swap directly -- a very easy 'mistake' to make. More importantly, a mistake you should expect your coworker to make. And your standard library writer, apparently. Why is the trivial task of making swap a specialization in the std namespace to avoid this being an issue "silly"?


Because it's undefined behaviour to add partial specializations to std namespace and if you do a lot of metaprogramming, you would almost always need one. Unfortunately, that's the only way that does work: http://accu.org/index.php/journals/466
Advertisement
Quote:Original post by rozz666
Because it's undefined behaviour to add partial specializations to std namespace

You cannot partially specialize function templates.
Quote:Original post by DevFred
Quote:Original post by rozz666
Because it's undefined behaviour to add partial specializations to std namespace

You cannot partially specialize function templates.


You are right. Still overloading results in undefined behaviour.
The standard (draft) says (17.3.3.1):

Quote:
A program may add template specializations for any standard library template to namespace std. Such a specialization (complete or partial) of a standard library template results in undefined behaviour unless the declaration depends on a user-defined name of external linkage and unless the specialization meets the standard library requirements for the original template.


If your swap function specialization works on user-defined types and it indeed swaps things, it's OK to put it in std namespace.

Also, the Wikipedia article on ADL mentions that over-dependence on it can lead to semantic problems.

I don't see why all the fuss.
Quote:Original post by visitor
I don't see why all the fuss.

The problem is that this is standards compliant:
namespace std{    template<>    void swap(String& a, String& b) // this is a specialization    {        std::cout << "I'm in your swap!" << std::endl;    }}

While this is not:
namespace std{    template<typename T>    void swap(Matrix<T>& a, Matrix<T>& b) // this is an overload of swap    {        std::cout << "I'm in your swap!" << std::endl;    }}

Because there is no such thing as partial specializations of function templates.
Quote:Also, the Wikipedia article on ADL mentions that over-dependence on it can lead to semantic problems.

Yes, and that's totally irrelevant.
swap is a particular case, a customization point, and the standard explicitly says to rely on ADL for that one.
Quote:Original post by loufoque
Quote:Also, the Wikipedia article on ADL mentions that over-dependence on it can lead to semantic problems.

Yes, and that's totally irrelevant.
swap is a particular case, a customization point, and the standard explicitly says to rely on ADL for that one.


Please quote where the standard explicitly says this. I just searched through my copy of the standard, checking every mention of "swap" in the standard, and found no mention of this at all. I'll be blunt: I'm convinced you're full of bullshit. It would be interesting to be proved wrong.



Quote:Original post by rozz666
Quote:Original post by MaulingMonkey
Quote:Original post by loufoque
That website is wrong.

Why? What does relying on ADL gain you? I can see what it loses you: performance when someone fucks up and calls the qualified std::swap directly -- a very easy 'mistake' to make. More importantly, a mistake you should expect your coworker to make. And your standard library writer, apparently. Why is the trivial task of making swap a specialization in the std namespace to avoid this being an issue "silly"?


Because it's undefined behaviour to add partial specializations to std namespace and if you do a lot of metaprogramming, you would almost always need one.


Whoops, no. The article you link quotes 17.4.3.1 to say that "It is undefined for a C++ program to add declarations or definitions to namespace std or namespaces within namespace std unless otherwise specified.".

Let's look at the rest of that paragraph, shall we?

Quote:It is undefined for a C + + program to add declarations or definitions to namespace std or namespaces
within namespace std unless otherwise specified. A program may add template specializations for any
standard library template to namespace std
. Such a specialization (complete or partial) of a standard
library template results in undefined behavior unless the declaration depends on a user-defined name of
external linkage and unless the specialization meets the standard library requirements for the original tem-
plate.
163)


While the second and third sentences imply, to me, that you may partially specialize things in the std namespace to your hearts content... as long as those specializations have external linkage and meet the other requirements of the standard library.

Please also note that overloading isn't the same thing as explicit specialization.



Quote:Original post by DevFred
Quote:Original post by visitor
I don't see why all the fuss.

The problem is ...

Oooh, I didn't expect someone to actually have a legitimate answer to my earlier loufoque-targeted question [lol]. I'll have to write a CREATE_STD_SWAP_SPECIALIZATIONS macro using BOOST_PP I guess :P
Quote:
Quote:Original post by DevFred
Quote:Original post by visitor
I don't see why all the fuss.

The problem is ...

Oooh, I didn't expect someone to actually have a legitimate answer to my earlier loufoque-targeted question [lol]. I'll have to write a CREATE_STD_SWAP_SPECIALIZATIONS macro using BOOST_PP I guess :P


#include <boost/preprocessor.hpp>#include <iostream>#include <algorithm>#define INDUSTRY_CSSS_NAME( product ) BOOST_PP_SEQ_HEAD(product) < BOOST_PP_SEQ_ENUM( BOOST_PP_SEQ_TAIL(product) ) >#define INDUSTRY_CSSS_PROD( r, product ) template<> void ::std::swap< INDUSTRY_CSSS_NAME(product) > \     ( INDUSTRY_CSSS_NAME(product)& lhs, INDUSTRY_CSSS_NAME(product)& rhs ) { lhs.swap(rhs); }#define INDUSTRY_CSSS_SPECIALIZATION( tempname, args ) BOOST_PP_SEQ_FOR_EACH_PRODUCT( INDUSTRY_CSSS_PROD, BOOST_PP_SEQ_INSERT( args, 0, (tempname) ) )#define INDUSTRY_CREATE_STD_SWAP_SPECIALIZATIONS( templatename, args ) INDUSTRY_CSSS_SPECIALIZATION( templatename, args )template < typename T, size_t N > struct Matrix { void swap( Matrix& other ) { std::cout << "custom swap called!\n"; } };INDUSTRY_CREATE_STD_SWAP_SPECIALIZATIONS( ::Matrix, ((signed char)(short)(int)(float)(double)(long double))  ((2)(3)(4)) )int main() {	Matrix<float,2> a, b;	std::swap( a, b );}


(Mind the space trailing the \ to prevent manglement by [source] tags if copypastaing)

I'm not sure how standard explicitly spelling out the FQN of ::std::swap is when creating the specializations, but it seems to work well on MSVC2k8:

/* no namespace */ INDUSTRY_CREATE_STD_SWAP_SPECIALIZATIONS( ::Matrix, ((signed char)(short)(int)(float)(double)(long double)) ((2)(3)(4)) )
Works fine, calls custom swap

namespace std { INDUSTRY_CREATE_STD_SWAP_SPECIALIZATIONS( ::Matrix, ((signed char)(short)(int)(float)(double)(long double)) ((2)(3)(4)) ) }
Works fine, calls custom swap

namespace fnord { INDUSTRY_CREATE_STD_SWAP_SPECIALIZATIONS( ::Matrix, ((signed char)(short)(int)(float)(double)(long double)) ((2)(3)(4)) ) }
Nice compiler errors instead of, say, silently not specializing the right function:
1>i:\home\projects\test\test\main.cpp(11) : error C2888: 'void std::swap<Matrix<T,N>>(Matrix<T,N> &,Matrix<T,N> &)' : symbol cannot be defined within namespace 'fnord'
1> with
1> [
1> T=signed char,
1> N=2
1> ]
1>i:\home\projects\test\test\main.cpp(11) : error C2888: 'void std::swap<Matrix<T,N>>(Matrix<T,N> &,Matrix<T,N> &)' : symbol cannot be defined within namespace 'fnord'
1> with
1> [
1> T=signed char,
1> N=3
1> ]
...
Quote:I just searched through my copy of the standard, checking every mention of "swap" in the standard, and found no mention of this at all.

The current version of the C++0x draft, which is still concept-based, uses concepts instead of ADL, as concepts were made to have similar properties to that of ADL.
As you can see, it makes use of a Swappable concept that is defined as a type which has a swap(T&, T&) overload visible in its namespace, which is quite equivalent.
Similarly, the Range concept, upon which the new for-loop depends, is being reworked so that it uses ADL to find begin and end.

Quote:While the second and third sentences imply, to me, that you may partially specialize things in the std namespace to your hearts content... as long as those specializations have external linkage and meet the other requirements of the standard library.

Please also note that overloading isn't the same thing as explicit specialization.

The point was that the standard doesn't allow overloading in the std namespace, which is required to define swap generically.
Your preprocessing solution is of course not generic, since any new instantiation of your template for different types that those you listed won't work.
Quote:Original post by loufoque
Quote:I just searched through my copy of the standard, checking every mention of "swap" in the standard, and found no mention of this at all.

The current version of the C++0x draft, which is still concept-based, uses concepts instead of ADL, as concepts were made to have similar properties to that of ADL.
As you can see, it makes use of a Swappable concept that is defined as a type which has a swap(T&, T&) overload visible in its namespace, which is quite equivalent.

Concepts are being removed from what will be C++1x. So you're saying we shouldn't specialize std::swap because the standard after the next may imply to you that ADL is your god and savior.

The only thing you've shown me is that they've decided to fix the standard library so it can take advantage of ADL. Missing is any quotation of anything suggesting it should be preferred, or that the std:: namespace will be ignored by std::Swappable.

Quote:The point
which I've already credited DevFred for actually making.

Quote:Your preprocessing solution is of course not generic, since any new instantiation of your template for different types that those you listed won't work.

...until added. Meanwhile, your 'solution' of not doing it at all is even less generic, since it won't work period.

This topic is closed to new replies.

Advertisement