Provide move semantics for PIMPLHandle objects

simbody · Jul 30, 2021 · a44476f · a44476f
1 parent a8f49c8
commit a44476f
Show file tree

Hide file tree

Showing 3 changed files with 164 additions and 0 deletions.
diff --git a/SimTKcommon/include/SimTKcommon/internal/PrivateImplementation.h b/SimTKcommon/include/SimTKcommon/internal/PrivateImplementation.h
@@ -162,6 +162,15 @@ class PIMPLHandle {
     /// @see referenceAssign()
     PIMPLHandle& copyAssign(const HANDLE& source);
 
+    /// Move assignment either does a shallow copy of PTR handle objects
+    /// or a move assignment for non-PTR handle objects. The logic is similar
+    /// to either referenceAssign() for PTR handles or copyAssign() for PTR handles.
+    /// The difference is that moveAssign does not increment reference count and
+    /// invalidates the handle implementation pointer of the source object.
+    /// @see copyAssign()
+    /// @see referenceAssign()
+    PIMPLHandle& moveAssign(HANDLE&& source) noexcept;
+
     /// Make this an empty handle, deleting the implementation object if
     /// this handle is the owner of it. A call to isEmptyHandle() will return
     /// true after this.
@@ -207,6 +216,13 @@ class PIMPLHandle {
     /// @see copyAssign
     PIMPLHandle(const PIMPLHandle& source);
 
+    /// The move constructor calls moveAssign function to do either
+    /// a shallow copy for PTR handle objects or move for non-PTR
+    /// handle objects. Note that the move constructor must be noexcept-qualified
+    /// to get proper move constructs of STL containers.
+    /// @see moveAssign
+    PIMPLHandle(PIMPLHandle &&source) noexcept;
+
     /// Copy assignment makes the current handle either a deep (value) or shallow
     /// (reference) copy of the supplied source PIMPL object, based on whether this
     /// is a "pointer sematics" (PTR=true) or "object (value) semantics" (PTR=false,
@@ -219,6 +235,12 @@ class PIMPLHandle {
     /// @see copyAssign
     PIMPLHandle& operator=(const PIMPLHandle& source);
 
+    /// Move assignment operator delegates the move operation to moveAssign.
+    /// Note that the move assignment operator must be noexcept-qualified
+    /// to get proper move assignments of STL containers.
+    /// @see moveAssign
+    PIMPLHandle& operator=(PIMPLHandle&& source) noexcept;
+
     /// Set the implementation for this empty handle. This may result in either
     /// an owner or reference handle, depending on the owner handle reference
     /// stored in the implementation object. This will throw an exception if the

diff --git a/SimTKcommon/include/SimTKcommon/internal/PrivateImplementation_Defs.h b/SimTKcommon/include/SimTKcommon/internal/PrivateImplementation_Defs.h
@@ -145,6 +145,12 @@ PIMPLHandle<HANDLE,IMPL,PTR>::PIMPLHandle(const PIMPLHandle& src) : impl(0) {
     else     copyAssign(src.downcastToHandle());
 }                   
 
+// copy constructor
+template <class HANDLE, class IMPL, bool PTR>
+PIMPLHandle<HANDLE,IMPL,PTR>::PIMPLHandle(PIMPLHandle&& src) noexcept : impl(0) {
+    moveAssign(static_cast<HANDLE&&>(src));
+}
+
 // copy assignment
 template <class HANDLE, class IMPL, bool PTR>
 PIMPLHandle<HANDLE,IMPL,PTR>& PIMPLHandle<HANDLE,IMPL,PTR>::
@@ -154,6 +160,13 @@ operator=(const PIMPLHandle& src) {
     return *this;
 } 
 
+// move assignment
+template <class HANDLE, class IMPL, bool PTR>
+PIMPLHandle<HANDLE,IMPL,PTR>& PIMPLHandle<HANDLE,IMPL,PTR>::
+operator=(PIMPLHandle &&src) noexcept {
+    return moveAssign(static_cast<HANDLE&&>(src));
+}
+
 template <class HANDLE, class IMPL, bool PTR>
 bool PIMPLHandle<HANDLE,IMPL,PTR>::isOwnerHandle() const {
     return impl && impl->hasOwnerHandle() &&
@@ -218,6 +231,24 @@ copyAssign(const HANDLE& src) {
     return *this;
 }
 
+template <class HANDLE, class IMPL, bool PTR>
+PIMPLHandle<HANDLE,IMPL,PTR>& PIMPLHandle<HANDLE,IMPL,PTR>::
+moveAssign(HANDLE &&src) noexcept {
+    if (isSameHandle(src)) return *this; // that was easy!
+    clearHandle();
+    if (src.impl) {
+        if (PTR)
+            impl = src.impl;
+        else {
+            impl = std::move(src.impl);
+            impl->replaceOwnerHandle(updDowncastToHandle());
+            assert(impl->getHandleCount() == 1);
+        }
+        src.impl = nullptr;
+    }
+    return *this;
+}
+
 // Provide an implementation for this empty handle, bumping the handle count.
 // We do not assume this handle is the owner of the implementation; the caller
 // must handle that separately.

diff --git a/SimTKcommon/tests/TestMoveAssignment.cpp b/SimTKcommon/tests/TestMoveAssignment.cpp
@@ -0,0 +1,111 @@
+// vim: expandtab ts=4 sts=4 sw=4 :
+
+#include "SimTKcommon/internal/PrivateImplementation_Defs.h"
+
+#define ASSERT(cond) {SimTK_ASSERT_ALWAYS(cond, "Assertion failed");}
+
+class ValueFish;
+class PointerFish;
+
+static const std::string CHK_STR{"I am a fish"};
+
+#define CHKVAL(fish) ASSERT(fish.getS() == CHK_STR)
+
+class FishImpl {
+public:
+    FishImpl() :
+        m_s{"I am a fish"}
+    {}
+
+    const std::string & getS() const { return m_s; }
+
+private:
+    std::string m_s;
+};
+
+class ValueFishRep : public SimTK::PIMPLImplementation<ValueFish, ValueFishRep>, public FishImpl {
+public:
+    ValueFishRep* clone() const { return new ValueFishRep(); }
+    int refCount() const { return getHandleCount(); }
+};
+
+class ValueFish : public SimTK::PIMPLHandle<ValueFish, ValueFishRep, false> {
+public:
+    ValueFish() :
+        HandleBase{ new ValueFishRep{} }
+    {}
+
+    const std::string & getS() const { return getImpl().getS(); }
+    bool isMovedAway() const { return isEmptyHandle(); }
+    int refCount() const { return getImpl().refCount(); }
+};
+
+class PointerFishRep : public SimTK::PIMPLImplementation<PointerFish, PointerFishRep>, public FishImpl {
+public:
+    PointerFishRep* clone() const { return new PointerFishRep(); }
+    int refCount() const { return getHandleCount(); }
+};
+
+class PointerFish : public SimTK::PIMPLHandle<PointerFish, PointerFishRep, true> {
+public:
+    PointerFish() :
+        HandleBase{ new PointerFishRep{} }
+    {}
+
+    const std::string & getS() const { return getImpl().getS(); }
+    bool isMovedAway() const { return isEmptyHandle(); }
+    int refCount() const { return getImpl().refCount(); }
+};
+
+void testValuePIMPL()
+{
+    ValueFish fish;
+    ValueFish fishCopy = fish;
+    ValueFish fishCCopy{fish};
+    ValueFish fishMove = std::move(fish);
+
+    ASSERT(fishCopy.isMovedAway() == false)
+    ASSERT(fishCCopy.isMovedAway() == false)
+    ASSERT(fishMove.isMovedAway() == false)
+    ASSERT(fish.isMovedAway() == true)
+    ASSERT(fishCopy.refCount() == 1); CHKVAL(fishCopy)
+    ASSERT(fishCCopy.refCount() == 1); CHKVAL(fishCCopy)
+    ASSERT(fishMove.refCount() == 1); CHKVAL(fishMove)
+
+    ValueFish fishCMove{std::move(fishCopy)};
+    ASSERT(fishCopy.isMovedAway() == true)
+}
+
+void testPointerPIMPL()
+{
+    PointerFish pFish;
+    PointerFish pFishCopy = pFish;
+    PointerFish pFishCCopy{pFish};
+
+    ASSERT(pFish.isMovedAway() == false)
+    ASSERT(pFishCopy.isMovedAway() == false)
+    ASSERT(pFishCCopy.isMovedAway() == false)
+    ASSERT(pFish.refCount() == 3); CHKVAL(pFish)
+    ASSERT(pFishCopy.refCount() == 3); CHKVAL(pFishCopy)
+    ASSERT(pFishCCopy.refCount() == 3); CHKVAL(pFishCCopy)
+
+    PointerFish pFishMove = std::move(pFish);
+    ASSERT(pFish.isMovedAway() == true);
+    ASSERT(pFishCopy.refCount() == 3); CHKVAL(pFishCopy)
+    ASSERT(pFishCCopy.refCount() == 3); CHKVAL(pFishCCopy);
+    ASSERT(pFishMove.refCount() == 3); CHKVAL(pFishMove);
+
+    PointerFish pFishCMove(std::move(pFishCopy));
+    ASSERT(pFishCopy.isMovedAway() == true)
+    ASSERT(pFishCCopy.refCount() == 3); CHKVAL(pFishCCopy)
+    ASSERT(pFishMove.refCount() == 3); CHKVAL(pFishMove);
+    ASSERT(pFishCMove.refCount() == 3); CHKVAL(pFishCMove);
+}
+
+int main()
+{
+    testValuePIMPL();
+    testPointerPIMPL();
+
+    return 0;
+}