Use a macro to specialize std::swap

Question

So, I have a macro.

// swap_specialize.hpp
#include <algorithm>

#ifndef STD_SWAP_SPECIALIZE
#define STD_SWAP_SPECIALIZE( CLASSNAME )            \
    namespace std {                                 \
    template<> inline                               \
    void swap( CLASSNAME & lhs, CLASSNAME & rhs )   \
    { lhs.swap(rhs); } }
#endif

So then I have a class

// c.hpp
#include <vector>
#include "swap_specialize.hpp"
class C
{
    public:
        C();

        void swap(C& rhs)
        {
            data_.swap(rhs.data_);
        }
        C& operator=(C rhs)
        {
            rhs.swap(*this);
            return *this;
        }
    private:
        std::vector<int> data_;
}

STD_SWAP_SPECIALIZE(C)

Does the usage of a macro to specialize std::swap in this way follow coding conventions?

I'm not sure I understand. What's wrong with just manually calling `c.swap(d);`? — GManNickG, Jul 17 '09 at 23:47
I agree...is there any real reason for wanting to call "swap(c,d)" instead of "c.swap(d)"? Is it simply you think the former "looks better"? Macros are generally a bad idea, and you should avoid them if at all possible...next thing you know you'll be writing "#define BEGIN {" and "#define END }", and it's all downhill from there. — davr, Jul 18 '09 at 00:56
If you are in a template, and you call c.swap(d), and c happens to have type "int", then you are lost in a bunch of template error messages — Johannes Schaub - litb, Jul 18 '09 at 01:05
Btw, that's a funky `operator=` you've got going on there. `operator=` should take a const parameter, and not modify it. They did it differently with `auto_ptr`, and that causes no end of confusion. — Steve Jessop, Jul 18 '09 at 02:30
@onebyone, his `operator=` is using an optimization opportunity: https://www.boostpro.com/trac/wiki/BoostCon09/RValue101#copy-elision-and-the-rvo . The idea is to let the compiler optimize the copy: It can detect the copy on the call-side and do RVO. Note the non-reference parameter he uses: The copy is the parameter itself, and he just finally swaps it with *this. This is a valid copy assignment operator. — Johannes Schaub - litb, Jul 18 '09 at 02:45
You definitely need to comment what you are doing in the assignment operator. I had to do a double take to make sure I understood what was happening. I would rather pass by const reference then create a temp inside the body and swap that. — Martin York, Jul 18 '09 at 19:34
This is a commonly used optimization, and a common idiom to optimize the copy-and-swap. As litb pointed out, it IS an optimization. — rlbond, Jul 18 '09 at 21:03
Btw, what does it do about self-assignment? Does it still avoid the copy then? Is it still fashionable to care about self-assignment these days? Is this in point of fact all explained in some relevant section of "Effective C++" that I've read and obviously forgotten? — Steve Jessop, Jul 19 '09 at 00:27
@onebyone: If the class is large and **you think that it will be relatively common to self-assign**, you can instead pass by const reference and check if they're equal. However, that's just an optimization. A lot of people argue that self-assignment is sufficiently rare to ignore it and remove the test. — rlbond, Jul 19 '09 at 12:23
He's doing what's the right way in many other cases: *If you always need to create a copy, then pass by value, and modify the parameter.* — Johannes Schaub - litb, Jul 19 '09 at 13:14
See this answer: http://stackoverflow.com/questions/270408/is-it-better-in-c-to-pass-by-value-or-pass-by-constant-reference/271344#271344 — Johannes Schaub - litb, Jul 19 '09 at 13:16

score 6 · Accepted Answer · answered Jul 18 '09 at 00:52

6

I would say it's OK if it increases readability. Judge yourself. Just my two cents: Specializing std::swap isn't really the right way to do this. Consider this situation:

my_stuff::C c, b;
// ...
swap(c, b);
// ...

This won't find std::swap if you haven't done using std::swap or something similar. You should rather declare your own swap in C's namespace:

void swap(C &a, C &b) {
  a.swap(b);
}

Now, this will work also in the above case, because argument dependent lookup searches in the namespace of the class. Code swapping generic things where the type isn't known should do it like this:

using std::swap;
swap(a, b);

Regardless of the type, this will use the best matching swap, and fall-back to std::swap if there wasn't a better matching one in the namespaces of a. Hard-coding the call to std::swap will cut too short on types that don't specialize std::swap but rather decide to provide their own swap in their namespace.

This is superious in another way: Imagine C is a template. You cannot specialize std::swap in this case. But just defining your own swap, that's perfectly fine.

template<typename T>
void swap(C<T> &a, C<T> &b) {
  a.swap(b);
}

This is the way how the swap for std::string and other classes is implemented too.

answered Jul 18 '09 at 00:52

Johannes Schaub - litb

496,577
130
894
1,212

The thing is, don't STL algorithms use std::swap? Such as std::reverse. – rlbond Jul 18 '09 at 01:01
I think it isn't specified whether std::swap is used or whether a unqualified call to "swap" is done (i would be glad for a reference on that, though). I looked, and gcc uses an unqualified call to "swap" - this will just work fine with the free swap function defined in the class' namespace. But i think any good Standard lib implementation won't call "std::swap" directly. That would be weird to do: It's already within "std", so qualification isn't necessary, and for another reason, it would stop argument dependent lookup working. – Johannes Schaub - litb Jul 18 '09 at 01:27
You're right. Section 20.1.4: The Swappable requirement is met by satisfying one or more of the following conditions: — T is Swappable if T satisfies the CopyConstructible requirments (20.1.3) and the Assignable requirements (23.1); — T is Swappable if a namespace scope function named swap exists in the same namespace as the definition of T, such that the expression swap(t,u) is valid and has the semantics described in Table 32. – rlbond Jul 18 '09 at 02:05
While I don't disagree with any of this, I will say it sucks that unlike any other function template in , std::swap alone should not be called by its full name just in case it's ADL overloaded. Stupid exceptions to the rule. I assume std::hash will be joining it, come C++0x. – Steve Jessop Jul 18 '09 at 02:27
@litb: It seems that Visual Studio's stl does make fully qualified calls to std::swap. – rlbond Jul 18 '09 at 02:55
Weird library, then, or broken ADL lookup of the compiler :) Maybe also a dependent name lookup issue of its compiler. Try including the `` (or whatever algorithm you use) header *after* the header defining your own `swap`, if not done already. Anyway, i think the above quote is from some C++1x draft? I don't find it in C++03 (section 20.1.4 doesn't exist in the latest draft n2914 either, though) – Johannes Schaub - litb Jul 18 '09 at 03:05
@onebyone : the "swap" is somewhat special in C++. when a particular swap is written with a "nothrow" exception guarantee, it can be used to enable stronger exception guarantees in user code while avoiding too much copying. This is the reason for the strange "call convention" of swap. In this, swap can be as special as a destructor or an operator =, and so, its special semantics must be known to use it best – paercebal Jul 18 '09 at 06:24
... Apparently, swap is so important boost features a standalone "boost::swap" enhanced version. For more info, please read: Scott Meyers, Effective C++ Third Edition, Item 25: "Consider support for a non-throwing swap" – paercebal Jul 18 '09 at 06:29
I have read Scott Meyers "Consider support for a non-throwing swap". I know why its desirable to overload swap, I'm just saying that it's inelegant to have one thing in `` be the odd one out. All the other algorithms are expected to be generic. For example unqualified find is not optimised for `std::set::iterator`, although it probably could have been defined to be, just as unqualified swap is optimised for sets. – Steve Jessop Jul 18 '09 at 09:56
As you say, it boils down to the fact that swap is more like an operator with a default implementation, than it is like algorithms copy or binary_search. Its implementation depends heavily on the class, it's not generic at all. But there's no punctuation mark for it, so it can't be a true operator. And unlike auto-generated `operator=`, because it's an algorithm the default implementation doesn't even do field-by-field swap. Meh. – Steve Jessop Jul 18 '09 at 10:04
Discussed in ISO C++ Library Working Group. Libraries, including the Standard lib should call swap() as litb describes. This applies to more than just swap(). Libraries also cannot assume that they can call `u.operator<(v)` but must use the `u – MSalters Jul 20 '09 at 11:20

score 0 · Answer 2 · answered Jul 17 '09 at 23:42

0

I don't see it buying you much. For non-template classes, it saves you very few lines. And for template classes, when you want to specialize generically (i.e. for all T), it just won't work.

answered Jul 17 '09 at 23:42

Pavel Minaev

99,783
25
219
289

score 0 · Answer 3 · answered Jul 17 '09 at 23:43

Assuming you are doing to use STD_SWAP_SPECIALIZE() for a number of other classes, I this this is perfectly reasonable. After all it's much more readable to have a series of

STD_SWAP_SPECIALIZE(Foo)
STD_SWAP_SPECIALIZE(Bar)
STD_SWAP_SPECIALIZE(Baz)

Than the equivilent expanded code. Also, if the expansion of STD_SWAP_SPECIALIZE was a little larger, then the macro definition gives you the code in a single place if it needed changing. (As it is the template definition is pretty small in your example then it's probably a moot point).

Use a macro to specialize std::swap

3 Answers3