Sorry for being off-topic... I don't want to talk about the complexity of C++, but about the actual article.

I wonder how the heck can an article end up on Artima, signed by Bjarne and have a serious programming error in it???

I talk about the exception safety of the clone_ptr assignment operator. Honestly, who write this article? What do the 10+ people on the editorial board do, if this gets published? Sorry for being upset a bit, but it may take literally *years* to "unteach" people who learn from material signed by Bjarne...

So PLEASE, change this:

clone_ptr& operator=(const clone_ptr& p)
{
if (this != &p)
{
delete ptr;
ptr = p.ptr ? p.ptr->clone() : 0;
}
return *this;
}

to something like this:

clone_ptr& operator=(const clone_ptr& p) {
if (this == &p) return *this;
T *ptrt== p.ptr ? p.ptr->clone() : 0;
delete ptr;
ptr=ptrt;
return *this;
}

or any of the possible many variantions that won't leave clone_ptr::ptr point to 0xDEADBEEF if p.ptr->clone() throws...

Attila aka ww

Sorry for being off-topic... I don't want to talk about the complexity of C++, but about the actual article.

I wonder how the heck can an article end up on Artima, signed by Bjarne and have a serious programming error in it???

I talk about the exception safety of the clone_ptr assignment operator. Honestly, who write this article? What do the 10+ people on the editorial board do, if this gets published? Sorry for being upset a bit, but it may take literally *years* to "unteach" people who learn from material signed by Bjarne...

So PLEASE, change this:

clone_ptr& operator=(const clone_ptr& p)
{
if (this != &p)
{
delete ptr;
ptr = p.ptr ? p.ptr->clone() : 0;
}
return *this;
}

to something like this:

clone_ptr& operator=(const clone_ptr& p) {
if (this == &p) return *this;
T *ptrt== p.ptr ? p.ptr->clone() : 0;
delete ptr;
ptr=ptrt;
return *this;
}

or any of the possible many variations that won't leave clone_ptr::ptr point to 0xDEADBEEF if p.ptr->clone() throws...

Attila aka ww

Now you can try it out in gcc 4.3.

> Now I wonder, should the next version of the standard
> library still provide overloads of std::swap
> for all of the STL containers? It looks like the most
> generic version, template <class T> swap(T&
> a, T& b), should be good enough by then!

Thanks for your kind comment Niels. To answer your question above, No. Explanation:

You are right that the generic swap is *much* more efficient now, using move construction and move assignment. However the generic swap is still 2-3 times as expensive as the overloaded swap for vector (for example). For quick coding/prototyping and for non-performance critical code, the generic swap can be much faster now (if the class provides move constructor and assignment). But std::containers such as vector are used *so* much that attention to detail is paramount. Thus even "twice as fast as blazingly fast" is worth a few extra lines of code.

Details: vector move construction, move assignment and swap are all approximately the same cost. The generic swap will do one move construction and two move assignments.

> I wonder how the heck can an article end up on Artima,
> signed by Bjarne and have a serious programming error in
> it???
>
> I talk about the exception safety of the clone_ptr
> assignment operator. Honestly, who write this article?

I did. And you are correct that there is an exception safety bug in this assignment operator. My apologies.

This paper was originally a committee paper aimed at getting this language feature accepted, and not targeted to Artima. This detail was not important to the original goal of the paper.

> What do the 10+ people on the editorial board do, if this
> s gets published?

They work hard at their day jobs and volunteer what little time they have left over (if any).

> So PLEASE, change this:
>
> clone_ptr& operator=(const clone_ptr& p)
> {
> if (this != &p)
> {
> delete ptr;
> ptr = p.ptr ? p.ptr->clone() : 0;
> }
> return *this;
> }
>
> to something like this:
>
> clone_ptr& operator=(const clone_ptr& p) {
> if (this == &p) return *this;
> T *ptrt== p.ptr ? p.ptr->clone() : 0;
> delete ptr;
> ptr=ptrt;
> return *this;
> }

I would have no objections to this change. Another acceptable possibility is:

clone_ptr& operator=(const clone_ptr& p)
{
if (this != &p)
{
delete ptr;
ptr = 0;
ptr = p.ptr ? p.ptr->clone() : 0;
}
return *this;
}

>
> I wonder how the heck can an article end up on Artima,
> signed by Bjarne and have a serious programming error in
> it???
>

By being a simple copy of a standards document focussed on a single topic written for relative experts. I would like to have had time to write a "perfect" article aimed for a more general audience, but there were no time and much demand.

-- Bjarne Stroustrup; http://www.research.att.com/~bs

and of course: sorry.

Phew. No I know that it wasn't someone else using your name(s). And I perfectly understand the reasons for the mistake. I am just very sorry that nobody has caught it in review and it has been published.

As usual: I did not intend to come through as an A-hole... and yet I succeeded. And unfortunately cheating does not work in my communication impairment: if I want to sound like one, I do then too. ;)

As for the time issue. I do not expect to be on the board or even mentioned, but I would be grateful if I would get the chance to quickly review stuff before it gets published. IOWs I volunteer, if anyone needs me. I am no guru or authority, but I have the advantage of knowing (hopefully enough) C++ and have no life... well, advantage for reviewing time anyway. :)

I am in Shanghai again until near the end of August, and due to that I won't be showing up on meetings until the fall.

I like Howards' example of the "quick fix", although I am not sure it is a good idea to delete the data if we are unable to copy it... In such a generic component we cannot tell if it was crucial to the program or not. Anyway, do you think it would be possible to update the article to provide some level of exception safety? I don't know how Artima works, but if it is possible, it would be valuable. IMHO.

Wow. I should.

Thanks for your reply, Howard.

> You are right that the generic swap is *much* more efficient now,
> using move construction and move assignment. However the generic
> swap is still 2-3 times as expensive as the overloaded swap for
> vector (for example).

Well... I doubt if an overloaded version for a specific container would be significantly faster than that clever generic swap you've presented, when doing all kinds of compiler optimizations. But when running a non-optimized "debug" version of a program, I guess you're right about the overloaded version being 2-3 times faster. Which sounds worthwhile to me.

Still there are quite a few more overloaded swap functions added to the draft of the next C++ Standard (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2521.pdf), having an rvalue reference as one of its arguments. Isn't a swap with an rvalue reference equivalent to a move assignment? Are the following two lines of code equivalent (assuming that GetSomeVector() returns a vector by value)?


swap(vec, GetSomeVector() ); // swap with an rvalue ref.
vec = GetSomeVector(); // Do a move assignment!


> Still there are quite a few more overloaded swap functions
> added to the draft of the next C++ Standard
> (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2
> 521.pdf), having an rvalue reference as one of its
> arguments. Isn't a swap with an rvalue reference
> equivalent to a move assignment? Are the following
> two lines of code equivalent (assuming that
> GetSomeVector() returns a vector by value)?
>
>
> swap(vec, GetSomeVector() ); // swap with an rvalue
> ue ref.
> vec = GetSomeVector(); // Do a move assignment!
>

You've hit upon an interesting grey area where the current working draft disagrees with the intent of the rvalue ref proposal. This disagreement is mainly one of getting the wording right, as opposed to a disagreement on the intent.

Assuming the desired intent, these two lines of code are nearly equivalent.

The first swaps the two vectors, and then destroys the data originally held in vec.

The second destroys the data originally held in vec and then swaps the two vectors.

The difference might be important when the vector holds items whose destructors have side effects whose timing might be significant (e.g. vector<thread>). LWG 675 (http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-active.html#675) is currently tracking this issue.

>> Are the following two lines of code equivalent?

swap(vec, GetSomeVector() ); // swap with an rvalue ref.
vec = GetSomeVector(); // Do a move assignment!

Howard Hinnant replied:
> The first swaps the two vectors, and then destroys the data originally held in vec.
> The second destroys the data originally held in vec and then swaps the two vectors.
> The difference might be important when the vector holds items whose destructors
> have side effects whose timing might be significant (e.g. vector<thread>).
> LWG 675 (http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-active.html#675) is
> currently tracking this issue.

Thanks, Howard! So in a worst case scenario, the move assignment of a vector might have O(n) complexity (according to LWG 675), while a vector swap would still have O(1) complexity, right?

Is there also any difference between doing std::swap, having an rvalue reference as argument, and calling the swap member function on a temporary? Or are the following two lines of code equivalent?

swap(vec, GetSomeVector() );
GetSomeVector().swap(vec);


> So in a worst case scenario, the move
> assignment of a vector might have O(n) complexity
> (according to LWG 675), while a vector swap would still
> have O(1) complexity, right?

Right, assuming equal allocators in all cases (unequal allocators bring in other issues which are still in flux.

Note if you're swapping with a temporary, then the destruction of that temporary may be O(n). But that cost isn't attributed to the swap itself.

> Is there also any difference between doing std::swap,
> having an rvalue reference as argument, and calling the
> swap member function on a temporary? Or are the following
> two lines of code equivalent?
>
> swap(vec, GetSomeVector() );
> GetSomeVector().swap(vec);
>

These should be equivalent. And this too:


vec.swap(GetSomeVector());


>> [...] are the following two lines of code equivalent?
>>
>> swap(vec, GetSomeVector() );
>> GetSomeVector().swap(vec);

Howard Hinnant replied:
> These should be equivalent. [...]

Thanks! Interestingly, the current working draft (N2521.pdf) doesn't have generic std::swap overloads, to support passing an rvalue reference argument. I guess they could have been added, as follows:

template<class T> void swap(T&& a, T& b);
template<class T> void swap(T& a, T&& b);

Are those overloads left out intentionally?

> Thanks! Interestingly, the current working draft
> (N2521.pdf) doesn't have generic std::swap
> overloads, to support passing an rvalue reference
> argument. I guess they could have been added, as follows:
>
> template<class T> void swap(T&& a, T& b);
> template<class T> void swap(T& a, T&& b);
>
> Are those overloads left out intentionally?

Yes, they were intentionally left out. I was nervous about allowing statements such as:


int i = 0;
std::swap(i, 2);


The motivation for allowing containers to swap with rvalues grew out of the desire to generalize the vector "swap trick":


vector<int> v(...);
vector<int>(v).swap(v); // C++03
v.swap(vector<int>(v)); // C++0X
swap(v, vector<int>(v)); // C++0X


I.e. just trying to make the swap trick a little easier to read and write.

Currently the draft generally contains 3 namespace scope swap signatures for library components (in general):


// pseudo code
swap(vector&, vector&);
swap(vector&&, vector&);
swap(vector&, vector&&);


The motivation for the three signatures is to prohibit clients from accidently swapping two rvalue vectors which is almost definitely a mistake. However there is currently some (quite understandable) feeling on the committee that this is overkill and we might want to replace the above three signatures with just this one:


// pseudo code
swap(vector&&, vector&&);


which would allow either argument to be lvalue or rvalue (in all 4 combinations). Neither solution has a performance or code size advantage over the other, and the safety enhancement of the 3-signature pack is quite minimal. So it is a close call in deciding the proper way to go, and I am comfortable with either choice.

Howard wrote:
> I was nervous about allowing statements such as:
> int i = 0;
> std::swap(i, 2);

Wouldn't it just make a temporary, having the value 2, and swap i and the temporary?

> The motivation for allowing containers to swap with
> rvalues grew out of the desire to generalize the vector
> "swap trick"

Okay, like you wrote at:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1858.html#23.2%20-%20Sequences

Personally I would prefer to have a dedicated shrink_to_fit function for this purpose. As proposed by Beman Dawes, LWG issue 755, std::vector and std:string lack explicit shrink-to-fit
operations http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-active.html#755

But anyway, the new std::array container doesn't have std::swap overloads for rvalue references either. Are those left out because an array cannot shrink?