On Sep 7, 2009, at 6:50 PM, David Abrahams wrote:
on Mon Sep 07 2009, Howard Hinnant <howard.hinnant-AT-gmail.com> wrote:
Do we think swap(x,y) is also an error when x == y?
This is a decent argument to not assert in self-move-assignment.
And not to call it undefined behavior. self-std:swapping was allowed in C++03 and I don't think we can break that.
But this is also almost certainly harmless:
#include <algorithm> #include <iostream>
class A { int data_; public: explicit A(int data = 0) : data_(data) {} A(A&& a) : data_(a.data_) {a.data_ = 0;} A& operator=(A&& a) {data_ = a.data_; a.data_ = 0; return *this;} friend std::ostream& operator<<(std::ostream& os, const A& a) { return os << a.data_; } };
int main() { A a(5); std::cout << "Before swap a = " << a << '\n'; std::swap(a, a); std::cout << "After swap a = " << a << '\n'; }
Before swap a = 5 After swap a = 5
Yes, it's harmless, but I don't see how it's a "but." I must be missing your point, because that example seems to support my argument.
But if I caught my own code doing a self-swap, yeah, I would treat it as a bug and correct it. For example, every time I've written reverse (which does nothing but swap x and y while x and y move closer to each other in the sequence), I'm careful to break out of the loop before &x == &y.
Yes, but if it's reverse's job to avoid that, then it's also swap's job (both are algorithms, neh?) So should we put a self-swap test there?
No, I don't think so. std::swap will only do a self-move-assignment using a moved-from object. Of course vector will never use this definition. But if it did, then: template <class _Tp, class _Allocator> inline vector<_Tp, _Allocator>& vector<_Tp, _Allocator>::operator=(vector&& __x) { clear(); swap(__x); return *this; } would work just fine. clear() is a no-op, and then you swap an empty capacity vector with itself. My point is that I don't want to be told I need to put a if(this != &x) in my move assignment operator. It is a useless performance penalty. Everyone should not have to pay (in terms of lost performance) for someone's bad code just to make sure self-move- assignment does a no-op instead of crashes. If you want to cast an lvalue to an rvalue, that's fine. Just make sure your code logic is such that such a cast is safe. I believe std::swap's use of move assignment is safe because by the time it move-assigns, if it is self referencing, all of your resources have already been moved to a local temp. -Howard