Skip to content

Commit

Permalink
Properly use the "Std Swap Two Step".
Browse files Browse the repository at this point in the history
I hate this. Why it isn't intuitive like proposed in here?
http://ericniebler.com/2014/10/21/customization-point-design-in-c11-and-beyond/
  • Loading branch information
mosra committed Feb 12, 2015
1 parent fbc3920 commit 65a2e56
Show file tree
Hide file tree
Showing 20 changed files with 71 additions and 51 deletions.
5 changes: 3 additions & 2 deletions src/Magnum/AbstractQuery.h
Expand Up @@ -186,8 +186,9 @@ inline AbstractQuery::AbstractQuery(AbstractQuery&& other) noexcept: _id(other._
}

inline AbstractQuery& AbstractQuery::operator=(AbstractQuery&& other) noexcept {
std::swap(_id, other._id);
std::swap(_target, other._target);
using std::swap;
swap(_id, other._id);
swap(_target, other._target);
return *this;
}

Expand Down
3 changes: 2 additions & 1 deletion src/Magnum/AbstractShaderProgram.cpp
Expand Up @@ -245,7 +245,8 @@ AbstractShaderProgram::~AbstractShaderProgram() {
}

AbstractShaderProgram& AbstractShaderProgram::operator=(AbstractShaderProgram&& other) noexcept {
std::swap(_id, other._id);
using std::swap;
swap(_id, other._id);
return *this;
}

Expand Down
7 changes: 4 additions & 3 deletions src/Magnum/AbstractTexture.h
Expand Up @@ -638,9 +638,10 @@ inline AbstractTexture::AbstractTexture(AbstractTexture&& other) noexcept: _targ
}

inline AbstractTexture& AbstractTexture::operator=(AbstractTexture&& other) noexcept {
std::swap(_target, other._target);
std::swap(_id, other._id);
std::swap(_created, other._created);
using std::swap;
swap(_target, other._target);
swap(_id, other._id);
swap(_created, other._created);
return *this;
}

Expand Down
3 changes: 2 additions & 1 deletion src/Magnum/Audio/Buffer.h
Expand Up @@ -112,7 +112,8 @@ inline Buffer::Buffer(Buffer&& other): _id(other._id) {
}

inline Buffer& Buffer::operator=(Buffer&& other) {
std::swap(_id, other._id);
using std::swap;
swap(_id, other._id);
return *this;
}

Expand Down
3 changes: 2 additions & 1 deletion src/Magnum/Audio/Source.h
Expand Up @@ -594,7 +594,8 @@ inline Source::Source(Source&& other): _id(other._id) {
}

inline Source& Source::operator=(Source&& other) {
std::swap(_id, other._id);
using std::swap;
swap(_id, other._id);
return *this;
}

Expand Down
9 changes: 5 additions & 4 deletions src/Magnum/Buffer.h
Expand Up @@ -1307,12 +1307,13 @@ inline Buffer::Buffer(Buffer&& other) noexcept: _id{other._id}, _targetHint{othe
}

inline Buffer& Buffer::operator=(Buffer&& other) noexcept {
std::swap(_id, other._id);
std::swap(_targetHint, other._targetHint);
using std::swap;
swap(_id, other._id);
swap(_targetHint, other._targetHint);
#ifdef CORRADE_TARGET_NACL
std::swap(_mappedBuffer, other._mappedBuffer);
swap(_mappedBuffer, other._mappedBuffer);
#endif
std::swap(_created, other._created);
swap(_created, other._created);
return *this;
}

Expand Down
5 changes: 3 additions & 2 deletions src/Magnum/BufferImage.h
Expand Up @@ -145,8 +145,9 @@ template<UnsignedInt dimensions> inline BufferImage<dimensions>::BufferImage(Buf

template<UnsignedInt dimensions> inline BufferImage<dimensions>& BufferImage<dimensions>::operator=(BufferImage<dimensions>&& other) noexcept {
AbstractImage::operator=(std::move(other));
std::swap(_size, other._size);
std::swap(_buffer, other._buffer);
using std::swap;
swap(_size, other._size);
swap(_buffer, other._buffer);
return *this;
}
#else
Expand Down
7 changes: 4 additions & 3 deletions src/Magnum/Framebuffer.h
Expand Up @@ -701,9 +701,10 @@ inline Framebuffer::Framebuffer(Framebuffer&& other) noexcept {
}

inline Framebuffer& Framebuffer::operator=(Framebuffer&& other) noexcept {
std::swap(_id, other._id);
std::swap(_viewport, other._viewport);
std::swap(_created, other._created);
using std::swap;
swap(_id, other._id);
swap(_viewport, other._viewport);
swap(_created, other._created);
return *this;
}

Expand Down
5 changes: 3 additions & 2 deletions src/Magnum/Image.h
Expand Up @@ -165,8 +165,9 @@ template<UnsignedInt dimensions> inline Image<dimensions>::Image(Image<dimension

template<UnsignedInt dimensions> inline Image<dimensions>& Image<dimensions>::operator=(Image<dimensions>&& other) noexcept {
AbstractImage::operator=(std::move(other));
std::swap(_size, other._size);
std::swap(_data, other._data);
using std::swap;
swap(_size, other._size);
swap(_data, other._data);
return *this;
}

Expand Down
5 changes: 3 additions & 2 deletions src/Magnum/Math/Algorithms/GaussJordan.h
Expand Up @@ -62,8 +62,9 @@ template<std::size_t size, std::size_t rows, class T> bool gaussJordanInPlaceTra
rowMax = row2;

/* Swap the rows */
std::swap(a[row], a[rowMax]);
std::swap(t[row], t[rowMax]);
using std::swap;
swap(a[row], a[rowMax]);
swap(t[row], t[rowMax]);

/* Singular */
if(TypeTraits<T>::equals(a[row][row], T(0)))
Expand Down
3 changes: 2 additions & 1 deletion src/Magnum/Math/Functions.h
Expand Up @@ -190,9 +190,10 @@ template<class T> inline typename std::enable_if<std::is_arithmetic<T>::value, s
return a < b ? std::make_pair(a, b) : std::make_pair(b, a);
}
template<std::size_t size, class T> std::pair<Vector<size, T>, Vector<size, T>> minmax(const Vector<size, T>& a, const Vector<size, T>& b) {
using std::swap;
std::pair<Vector<size, T>, Vector<size, T>> out{a, b};
for(std::size_t i = 0; i != size; ++i)
if(out.first[i] > out.second[i]) std::swap(out.first[i], out.second[i]);
if(out.first[i] > out.second[i]) swap(out.first[i], out.second[i]);
return out;
}
#endif
Expand Down
31 changes: 16 additions & 15 deletions src/Magnum/Mesh.cpp
Expand Up @@ -132,27 +132,28 @@ Mesh::Mesh(Mesh&& other) noexcept: _id(other._id), _created{other._created}, _pr
}

Mesh& Mesh::operator=(Mesh&& other) noexcept {
std::swap(_id, other._id);
std::swap(_created, other._created);
std::swap(_primitive, other._primitive);
std::swap(_count, other._count);
std::swap(_baseVertex, other._baseVertex);
std::swap(_instanceCount, other._instanceCount);
using std::swap;
swap(_id, other._id);
swap(_created, other._created);
swap(_primitive, other._primitive);
swap(_count, other._count);
swap(_baseVertex, other._baseVertex);
swap(_instanceCount, other._instanceCount);
#ifndef MAGNUM_TARGET_GLES
std::swap(_baseInstance, other._baseInstance);
swap(_baseInstance, other._baseInstance);
#endif
#ifndef MAGNUM_TARGET_GLES2
std::swap(_indexStart, other._indexStart);
std::swap(_indexEnd, other._indexEnd);
swap(_indexStart, other._indexStart);
swap(_indexEnd, other._indexEnd);
#endif
std::swap(_indexOffset, other._indexOffset);
std::swap(_indexType, other._indexType);
std::swap(_indexBuffer, other._indexBuffer);
std::swap(_attributes, other._attributes);
swap(_indexOffset, other._indexOffset);
swap(_indexType, other._indexType);
swap(_indexBuffer, other._indexBuffer);
swap(_attributes, other._attributes);
#ifndef MAGNUM_TARGET_GLES2
std::swap(_integerAttributes, other._integerAttributes);
swap(_integerAttributes, other._integerAttributes);
#ifndef MAGNUM_TARGET_GLES
std::swap(_longAttributes, other._longAttributes);
swap(_longAttributes, other._longAttributes);
#endif
#endif

Expand Down
3 changes: 2 additions & 1 deletion src/Magnum/MeshTools/CombineIndexedArrays.h
Expand Up @@ -131,7 +131,8 @@ template<class T> void writeCombinedArray(const UnsignedInt stride, const Unsign
CORRADE_ASSERT(index < array.size(), "MeshTools::combineIndexedArrays(): index out of range", );
output.push_back(array[index]);
}
std::swap(output, array);
using std::swap;
swap(output, array);
}

/* Terminator for recursive calls */
Expand Down
3 changes: 2 additions & 1 deletion src/Magnum/MeshTools/FlipNormals.cpp
Expand Up @@ -32,8 +32,9 @@ namespace Magnum { namespace MeshTools {
void flipFaceWinding(std::vector<UnsignedInt>& indices) {
CORRADE_ASSERT(!(indices.size()%3), "MeshTools::flipNormals(): index count is not divisible by 3!", );

using std::swap;
for(std::size_t i = 0; i != indices.size(); i += 3)
std::swap(indices[i+1], indices[i+2]);
swap(indices[i+1], indices[i+2]);
}

void flipNormals(std::vector<Vector3>& normals) {
Expand Down
3 changes: 2 additions & 1 deletion src/Magnum/MeshTools/Tipsify.cpp
Expand Up @@ -125,7 +125,8 @@ void Tipsify::operator()(std::size_t cacheSize) {
}

/* Swap original index buffer with optimized */
std::swap(indices, outputIndices);
using std::swap;
swap(indices, outputIndices);
}

void Tipsify::buildAdjacency(std::vector<UnsignedInt>& liveTriangleCount, std::vector<UnsignedInt>& neighborOffset, std::vector<UnsignedInt>& neighbors) const {
Expand Down
5 changes: 3 additions & 2 deletions src/Magnum/Renderbuffer.h
Expand Up @@ -222,8 +222,9 @@ inline Renderbuffer::Renderbuffer(Renderbuffer&& other) noexcept: _id{other._id}
}

inline Renderbuffer& Renderbuffer::operator=(Renderbuffer&& other) noexcept {
std::swap(_id, other._id);
std::swap(_created, other._created);
using std::swap;
swap(_id, other._id);
swap(_created, other._created);
return *this;
}

Expand Down
7 changes: 4 additions & 3 deletions src/Magnum/Shader.h
Expand Up @@ -576,9 +576,10 @@ inline Shader::Shader(Shader&& other) noexcept: _type(other._type), _id(other._i
}

inline Shader& Shader::operator=(Shader&& other) noexcept {
std::swap(_type, other._type);
std::swap(_id, other._id);
std::swap(_sources, other._sources);
using std::swap;
swap(_type, other._type);
swap(_id, other._id);
swap(_sources, other._sources);
return *this;
}

Expand Down
5 changes: 3 additions & 2 deletions src/Magnum/Shapes/Composition.cpp
Expand Up @@ -91,8 +91,9 @@ template<UnsignedInt dimensions> Composition<dimensions>& Composition<dimensions
}

template<UnsignedInt dimensions> Composition<dimensions>& Composition<dimensions>::operator=(Composition<dimensions>&& other) {
std::swap(other._shapes, _shapes);
std::swap(other._nodes, _nodes);
using std::swap;
swap(other._shapes, _shapes);
swap(other._nodes, _nodes);
return *this;
}

Expand Down
5 changes: 3 additions & 2 deletions src/Magnum/Trade/ImageData.h
Expand Up @@ -136,8 +136,9 @@ template<UnsignedInt dimensions> inline ImageData<dimensions>::ImageData(ImageDa

template<UnsignedInt dimensions> inline ImageData<dimensions>& ImageData<dimensions>::operator=(ImageData<dimensions>&& other) noexcept {
AbstractImage::operator=(std::move(other));
std::swap(_size, other._size);
std::swap(_data, other._data);
using std::swap;
swap(_size, other._size);
swap(_data, other._data);
return *this;
}

Expand Down
5 changes: 3 additions & 2 deletions src/Magnum/TransformFeedback.h
Expand Up @@ -376,8 +376,9 @@ inline TransformFeedback::TransformFeedback(TransformFeedback&& other) noexcept:
}

inline TransformFeedback& TransformFeedback::operator=(TransformFeedback&& other) noexcept {
std::swap(_id, other._id);
std::swap(_created, other._created);
using std::swap;
swap(_id, other._id);
swap(_created, other._created);
return *this;
}

Expand Down

3 comments on commit 65a2e56

@LB--
Copy link
Contributor

@LB-- LB-- commented on 65a2e56 Feb 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it isn't intuitive

Because prefixing with std:: shrinks the scope of ADL in such a way that custom (better) swap implementations are not found. It's a nuance, yes, but I don't think we'll see it out of the language any time soon.

@mosra
Copy link
Owner Author

@mosra mosra commented on 65a2e56 Feb 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it would be nice if they said that since C++17 one can write just std::swap(a, b) and the generic implementation will finally do the argument-dependent lookup voodoo on its own.

@mosra
Copy link
Owner Author

@mosra mosra commented on 65a2e56 Mar 14, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a proposal for this now: http://ericniebler.github.io/std/wg21/D4381.html

Please sign in to comment.