SkTArray to move when moving.

include/core/SkTArray.h

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkTArray_DEFINED
 #define SkTArray_DEFINED
 
+#include "../private/SkTLogic.h"
 #include "../private/SkTemplates.h"
 #include "SkTypes.h"
 
 #include <new>
 #include <utility>
 
 template <typename T, bool MEM_COPY = false> class SkTArray;
-
-namespace SkTArrayExt {
-
-template<typename T>
-inline void copy(SkTArray<T, true>* self, int dst, int src) {
-    memcpy(&self->fItemArray[dst], &self->fItemArray[src], sizeof(T));
-}
-template<typename T>
-inline void copy(SkTArray<T, true>* self, const T* array) {
-    sk_careful_memcpy(self->fMemArray, array, self->fCount * sizeof(T));
-}
-template<typename T>
-inline void copyAndDelete(SkTArray<T, true>* self, char* newMemArray) {
-    sk_careful_memcpy(newMemArray, self->fMemArray, self->fCount * sizeof(T));
-}
-
-template<typename T>
-inline void copy(SkTArray<T, false>* self, int dst, int src) {
-    new (&self->fItemArray[dst]) T(self->fItemArray[src]);
-}
-template<typename T>
-inline void copy(SkTArray<T, false>* self, const T* array) {
-    for (int i = 0; i < self->fCount; ++i) {
-        new (self->fItemArray + i) T(array[i]);
-    }
-}
-template<typename T>
-inline void copyAndDelete(SkTArray<T, false>* self, char* newMemArray) {
-    for (int i = 0; i < self->fCount; ++i) {
-        new (newMemArray + sizeof(T) * i) T(self->fItemArray[i]);
-        self->fItemArray[i].~T();
-    }
-}
-
-}
-
 template <typename T, bool MEM_COPY> void* operator new(size_t, SkTArray<T, MEM_COPY>*, int);
 
 /** When MEM_COPY is true T will be bit copied when moved.
     When MEM_COPY is false, T will be copy constructed / destructed.
     In all cases T will be default-initialized on allocation,
     and its destructor will be called from this object's destructor.
 */
 template <typename T, bool MEM_COPY> class SkTArray {
 public:
     /**
@@ skipping 34 matching lines @@
     /**
      * assign copy of array to this
      */
     SkTArray& operator =(const SkTArray& array) {
         for (int i = 0; i < fCount; ++i) {
             fItemArray[i].~T();
         }
         fCount = 0;
         this->checkRealloc((int)array.count());
         fCount = array.count();
-        SkTArrayExt::copy(this, static_cast<const T*>(array.fMemArray));
+        this->copy(static_cast<const T*>(array.fMemArray));
         return *this;
     }
 
     ~SkTArray() {
         for (int i = 0; i < fCount; ++i) {
             fItemArray[i].~T();
         }
         if (fMemArray != fPreAllocMemArray) {
             sk_free(fMemArray);
         }
@@ skipping 24 matching lines @@
     /**
      * Resets to a copy of a C array.
      */
     void reset(const T* array, int count) {
         for (int i = 0; i < fCount; ++i) {
             fItemArray[i].~T();
         }
         int delta = count - fCount;
         this->checkRealloc(delta);
         fCount = count;
-        SkTArrayExt::copy(this, array);
+        this->copy(array);
     }
 
     void removeShuffle(int n) {
         SkASSERT(n < fCount);
         int newCount = fCount - 1;
         fCount = newCount;
         fItemArray[n].~T();
         if (n != newCount) {
-            SkTArrayExt::copy(this, n, newCount);
-            fItemArray[newCount].~T();
+            this->move(n, newCount);
         }
     }
 
     /**
      * Number of elements in the array.
      */
     int count() const { return fCount; }
 
     /**
      * Is the array empty.
@@ skipping 241 matching lines @@
         fPreAllocMemArray   = preAllocStorage;
         if (fReserveCount >= fCount &&
             preAllocStorage) {
             fAllocCount = fReserveCount;
             fMemArray = preAllocStorage;
         } else {
             fAllocCount = SkMax32(fCount, fReserveCount);
             fMemArray = sk_malloc_throw(fAllocCount * sizeof(T));
         }
 
-        SkTArrayExt::copy(this, array);
+        this->copy(array);
     }
 
 private:
+    /** In the following move and copy methods, 'dst' is assumed to be uninitialized raw storage.
+     *  When moving (and MEM_COPY is false), the moved from element will be destroyed.

[mtklein, 2016/02/07 22:21:23]

Shouldn't we just move assign/copy it and let the type of the moved-from element
handle its own business?

[bungeman-skia, 2016/02/08 03:17:03]

How would it? Everything here is placement newed and destructed, it's not like
we ever call delete on these. Even moved-from objects need to be destroyed.
Also, this is just documenting what was already happening, but it wasn't clear
before what the real intent was (this makes the two 'move' methods more
obviously parallel).

Perhaps instead of calling these methods 'move' they could be called
'moveAndDestroy' to avoid confusion with std::move(InputIt, InputIt, OutputIt).

[bungeman-skia, 2016/02/08 03:36:24]

Perhaps it would be clearer to replace this second sentence with:

In the following move methods, 'src' is destroyed leaving behind uninitialized
raw storage.

[mtklein, 2016/02/08 16:33:07]

sgtm

+     */
+    template <bool E = MEM_COPY> SK_WHEN(E, void) copy(const T* src) {
+        sk_careful_memcpy(fMemArray, src, fCount * sizeof(T));
+    }
+    template <bool E = MEM_COPY> SK_WHEN(E, void) move(int dst, int src) {
+        memcpy(&fItemArray[dst], &fItemArray[src], sizeof(T));
+    }
+    template <bool E = MEM_COPY> SK_WHEN(E, void) move(char* dst) {
+        sk_careful_memcpy(dst, fMemArray, fCount * sizeof(T));
+    }
+
+    template <bool E = MEM_COPY> SK_WHEN(!E, void) copy(const T* src) {
+        for (int i = 0; i < fCount; ++i) {
+            new (fItemArray + i) T(src[i]);
+        }
+    }
+    template <bool E = MEM_COPY> SK_WHEN(!E, void) move(int dst, int src) {
+        new (&fItemArray[dst]) T(std::move(fItemArray[src]));
+        fItemArray[src].~T();
+    }
+    template <bool E = MEM_COPY> SK_WHEN(!E, void) move(char* dst) {
+        for (int i = 0; i < fCount; ++i) {
+            new (dst + sizeof(T) * i) T(std::move(fItemArray[i]));
+            fItemArray[i].~T();
+        }
+    }
 
     static const int gMIN_ALLOC_COUNT = 8;
 
     // Helper function that makes space for n objects, adjusts the count, but does not initialize
     // the new objects.
     void* push_back_raw(int n) {
         this->checkRealloc(n);
         void* ptr = fItemArray + fCount;
         fCount += n;
         return ptr;
@@ skipping 17 matching lines @@
 
             fAllocCount = newAllocCount;
             char* newMemArray;
 
             if (fAllocCount == fReserveCount && fPreAllocMemArray) {
                 newMemArray = (char*) fPreAllocMemArray;
             } else {
                 newMemArray = (char*) sk_malloc_throw(fAllocCount*sizeof(T));
             }
 
-            SkTArrayExt::copyAndDelete<T>(this, newMemArray);
+            this->move(newMemArray);
 
             if (fMemArray != fPreAllocMemArray) {
                 sk_free(fMemArray);
             }
             fMemArray = newMemArray;
         }
     }
 
     friend void* operator new<T>(size_t, SkTArray*, int);
 
-    template<typename X> friend void SkTArrayExt::copy(SkTArray<X, true>* that, int dst, int src);
-    template<typename X> friend void SkTArrayExt::copy(SkTArray<X, true>* that, const X*);
-    template<typename X> friend void SkTArrayExt::copyAndDelete(SkTArray<X, true>* that, char*);
-
-    template<typename X> friend void SkTArrayExt::copy(SkTArray<X, false>* that, int dst, int src);
-    template<typename X> friend void SkTArrayExt::copy(SkTArray<X, false>* that, const X*);
-    template<typename X> friend void SkTArrayExt::copyAndDelete(SkTArray<X, false>* that, char*);
-
     int     fReserveCount;
     int     fCount;
     int     fAllocCount;
     void*   fPreAllocMemArray;
     union {
         T*       fItemArray;
         void*    fMemArray;
     };
 };
 
@@ skipping 55 matching lines @@
     SkSTArray& operator= (const INHERITED& array) {
         INHERITED::operator=(array);
         return *this;
     }
 
 private:
     SkAlignedSTStorage<N,T> fStorage;
 };
 
 #endif