fixed wxVector<T> to work with non-POD types again; added optimization to keep using...

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@51330 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

diff --git a/include/wx/meta/if.h b/include/wx/meta/if.h
new file mode 100644 (file)
index 0000000..64532e6
--- /dev/null
+++ b/include/wx/meta/if.h
@@ -0,0 +1,58 @@
+/////////////////////////////////////////////////////////////////////////////
+// Name:        wx/meta/if.h
+// Purpose:     declares wxIf<> metaprogramming construct
+// Author:      Vaclav Slavik
+// Created:     2008-01-22
+// RCS-ID:      $Id$
+// Copyright:   (c) 2008 Vaclav Slavik
+// Licence:     wxWindows licence
+/////////////////////////////////////////////////////////////////////////////
+
+#ifndef _WX_META_IF_H_
+#define _WX_META_IF_H_
+
+// NB: This code is intentionally written without partial templates
+//     specialization, because some older compilers (notably VC6) don't
+//     support it.
+
+namespace wxPrivate
+{
+
+template<bool Cond> struct wxIfImpl {};
+
+// specialization for true:
+template<>
+struct wxIfImpl<true>
+{
+    template<typename TTrue, typename TFalse> struct Result
+    {
+        typedef TTrue value;
+    };
+};
+
+// specialization for false:
+template<>
+struct wxIfImpl<false>
+{
+    template<typename TTrue, typename TFalse> struct Result
+    {
+        typedef TFalse value;
+    };
+};
+
+} // namespace wxPrivate
+
+// wxIf<> template defines nested type "value" which is the same as 
+// TTrue if the condition Cond (boolean compile-time constant) was met and
+// TFalse if it wasn't.
+//
+// See wxVector<T> in vector.h for usage example
+template<bool Cond, typename TTrue, typename TFalse>
+struct wxIf
+{
+    typedef typename wxPrivate::wxIfImpl<Cond>
+                         ::template Result<TTrue,TFalse>::value
+            value;
+};
+
+#endif // _WX_META_IF_H_

diff --git a/include/wx/meta/movable.h b/include/wx/meta/movable.h
new file mode 100644 (file)
index 0000000..ada5354
--- /dev/null
+++ b/include/wx/meta/movable.h
@@ -0,0 +1,78 @@
+/////////////////////////////////////////////////////////////////////////////
+// Name:        wx/meta/movable.h
+// Purpose:     Test if a type is movable using memmove() etc.
+// Author:      Vaclav Slavik
+// Created:     2008-01-21
+// RCS-ID:      $Id$
+// Copyright:   (c) 2008 Vaclav Slavik
+// Licence:     wxWindows licence
+/////////////////////////////////////////////////////////////////////////////
+
+#ifndef _WX_META_MOVABLE_H_
+#define _WX_META_MOVABLE_H_
+
+#include "wx/defs.h"
+#include "wx/string.h" // for wxIsMovable<wxString> specialization
+
+// Helper to decide if an object of type T is "movable", i.e. if it can be
+// copied to another memory location using memmove() or realloc() C functions.
+// C++ only gurantees that POD types (including primitive types) are
+// movable.
+template<typename T>
+struct wxIsMovable
+{
+    // NB: enum, because VC6 can't handle "static const bool value = true;"
+    enum { value = false };
+};
+
+// Macro to add wxIsMovable<T> specialization for given type that marks it
+// as movable:
+#define WX_DECLARE_TYPE_MOVABLE(type)                       \
+    template<> struct wxIsMovable<type>                     \
+    {                                                       \
+        enum { value = true };                              \
+    };
+
+WX_DECLARE_TYPE_MOVABLE(bool)
+WX_DECLARE_TYPE_MOVABLE(unsigned char)
+WX_DECLARE_TYPE_MOVABLE(signed char)
+WX_DECLARE_TYPE_MOVABLE(unsigned int)
+WX_DECLARE_TYPE_MOVABLE(signed int)
+WX_DECLARE_TYPE_MOVABLE(unsigned short int)
+WX_DECLARE_TYPE_MOVABLE(signed short int)
+WX_DECLARE_TYPE_MOVABLE(signed long int)
+WX_DECLARE_TYPE_MOVABLE(unsigned long int)
+WX_DECLARE_TYPE_MOVABLE(float)
+WX_DECLARE_TYPE_MOVABLE(double)
+WX_DECLARE_TYPE_MOVABLE(long double)
+#if wxWCHAR_T_IS_REAL_TYPE
+WX_DECLARE_TYPE_MOVABLE(wchar_t)
+#endif
+#ifdef wxLongLong_t
+WX_DECLARE_TYPE_MOVABLE(wxLongLong_t)
+WX_DECLARE_TYPE_MOVABLE(wxULongLong_t)
+#endif
+
+// pointers are movable:
+template<typename T>
+struct wxIsMovable<T*>
+{
+    enum { value = true };
+};
+template<typename T>
+struct wxIsMovable<const T*>
+{
+    enum { value = true };
+};
+
+// Our implementation of wxString is written in such way that it's safe to move
+// it around. OTOH, we don't know anything about std::string.
+// (NB: we don't put this into string.h and choose to include wx/string.h from
+// here instead so that rarely-used wxIsMovable<T> code isn't included by
+// everything)
+#if !wxUSE_STL
+WX_DECLARE_TYPE_MOVABLE(wxString)
+#endif
+
+
+#endif // _WX_META_MOVABLE_H_

diff --git a/include/wx/vector.h b/include/wx/vector.h
index 9de3d724672cd0537cc2d5e8d210e01f31d32f7b..f7aac0b20bb859a097bce2ac09d1e33fe2c1f85f 100644 (file)
--- a/include/wx/vector.h
+++ b/include/wx/vector.h
@@ -22,13 +22,91 @@
 #else // !wxUSE_STL
 
 #include "wx/utils.h"
 #else // !wxUSE_STL
 
 #include "wx/utils.h"

+#include "wx/meta/movable.h"
+#include "wx/meta/if.h"

 
 #include "wx/beforestd.h"
 #include <new> // for placement new
 #include "wx/afterstd.h"
 
 
 #include "wx/beforestd.h"
 #include <new> // for placement new
 #include "wx/afterstd.h"
 

+namespace wxPrivate
+{
+
+// These templates encapsulate memory operations for use by wxVector; there are
+// two implementations, both in generic way for any C++ types and as an
+// optimized version for "movable" types that uses realloc() and memmove().
+
+// version for movable types:
+template<typename T>
+struct wxVectorMemOpsMovable
+{
+    static void Free(T* array)
+        { free(array); }
+
+    static T* Realloc(T* old, size_t newCapacity, size_t WXUNUSED(occupiedSize))
+        { return (T*)realloc(old, newCapacity * sizeof(T)); }
+
+    static void MemmoveBackward(T* dest, T* source, size_t count)
+        { memmove(dest, source, count * sizeof(T)); }
+
+    static void MemmoveForward(T* dest, T* source, size_t count)
+        { memmove(dest, source, count * sizeof(T)); }
+};
+
+// generic version for non-movable types:
+template<typename T>
+struct wxVectorMemOpsGeneric
+{
+    static void Free(T* array)
+        { ::operator delete(array); }
+
+    static T* Realloc(T* old, size_t newCapacity, size_t occupiedSize)
+    {
+        T *mem = (T*)::operator new(newCapacity * sizeof(T));
+        for ( size_t i = 0; i < occupiedSize; i++ )
+        {
+            new(mem + i) T(old[i]);
+            old[i].~T();
+        }
+        ::operator delete(old);
+        return mem;
+    }
+
+    static void MemmoveBackward(T* dest, T* source, size_t count)
+    {
+        wxASSERT( dest < source );
+        T* destptr = dest;
+        T* sourceptr = source;
+        for ( size_t i = count; i > 0; --i, ++destptr, ++sourceptr )
+        {
+            new(destptr) T(*sourceptr);
+            sourceptr->~T();
+        }
+    }
+
+    static void MemmoveForward(T* dest, T* source, size_t count)
+    {
+        wxASSERT( dest > source );
+        T* destptr = dest + count - 1;
+        T* sourceptr = source + count - 1;
+        for ( size_t i = count; i > 0; --i, --destptr, --sourceptr )
+        {
+            new(destptr) T(*sourceptr);
+            sourceptr->~T();
+        }
+    }
+};
+
+
+} // namespace wxPrivate
+

 template<typename T>
 class wxVector
 template<typename T>
 class wxVector

+    // this cryptic expression means "derive from wxVectorMemOpsMovable if
+    // type T is movable type, otherwise derive from wxVectorMemOpsGeneric
+    : private wxIf< wxIsMovable<T>::value,
+                    wxPrivate::wxVectorMemOpsMovable<T>,
+                    wxPrivate::wxVectorMemOpsGeneric<T> >::value

 {
 public:
     typedef size_t size_type;
 {
 public:
     typedef size_t size_type;


@@ -57,7 +135,7 @@ public:
             m_values[i].~T();
         }
 
             m_values[i].~T();
         }
 

-        free(m_values);
+        Free(m_values);

         m_values = NULL;
         m_size = m_capacity = 0;
     }
         m_values = NULL;
         m_size = m_capacity = 0;
     }


@@ -78,8 +156,7 @@ public:
         if ( m_capacity + increment > n )
             n = m_capacity + increment;
 
         if ( m_capacity + increment > n )
             n = m_capacity + increment;
 

-        m_values = (value_type*)realloc(m_values, n * sizeof(value_type));
-
+        m_values = Realloc(m_values, n * sizeof(value_type), m_size);

         m_capacity = n;
     }
 
         m_capacity = n;
     }
 


@@ -162,9 +239,7 @@ public:
         // the way:
         if ( after > 0 )
         {
         // the way:
         if ( after > 0 )
         {

-            memmove(m_values + idx + 1,
-                    m_values + idx,
-                    after * sizeof(value_type));
+            MemmoveForward(m_values + idx + 1, m_values + idx, after);

         }
 
 #if wxUSE_EXCEPTIONS
         }
 
 #if wxUSE_EXCEPTIONS


@@ -183,9 +258,7 @@ public:
             // back to their original positions in m_values
             if ( after > 0 )
             {
             // back to their original positions in m_values
             if ( after > 0 )
             {

-                memmove(m_values + idx,
-                        m_values + idx + 1,
-                        after * sizeof(value_type));
+                MemmoveBackward(m_values + idx, m_values + idx + 1, after);

             }
 
             throw; // rethrow the exception
             }
 
             throw; // rethrow the exception


@@ -217,14 +290,12 @@ public:
 
         // erase elements by calling their destructors:
         for ( iterator i = first; i < last; ++i )
 
         // erase elements by calling their destructors:
         for ( iterator i = first; i < last; ++i )

-            i->~value_type();
+            i->~T();

 
         // once that's done, move following elements over to the freed space:
         if ( after > 0 )
         {
 
         // once that's done, move following elements over to the freed space:
         if ( after > 0 )
         {

-            memmove(m_values + idx,
-                    m_values + idx + count,
-                    after * sizeof(value_type));
+            MemmoveBackward(m_values + idx, m_values + idx + count, after);

         }
 
         m_size -= count;
         }
 
         m_size -= count;

diff --git a/tests/vectors/vectors.cpp b/tests/vectors/vectors.cpp
index ad49527a94ee27f239ecd782376ed3e8a43d2d36..7f0128c80d7171b0ea017639535ac2399c2d67b1 100644 (file)
--- a/tests/vectors/vectors.cpp
+++ b/tests/vectors/vectors.cpp
@@ -23,9 +23,47 @@
 
 #include "wx/vector.h"
 
 
 #include "wx/vector.h"
 

-// --------------------------------------------------------------------------
+// ----------------------------------------------------------------------------
+// simple class capable of detecting leaks of its objects
+// ----------------------------------------------------------------------------
+
+class CountedObject
+{
+public:
+    CountedObject(int n = 0) : m_n(n) { ms_count++; }
+    CountedObject(const CountedObject& co) : m_n(co.m_n) { ms_count++; }
+    ~CountedObject() { ms_count--; }
+
+    int GetValue() const { return m_n; }
+
+    static int GetCount() { return ms_count; }
+
+private:
+    static int ms_count;
+
+    int m_n;
+};
+
+int CountedObject::ms_count = 0;
+
+// ----------------------------------------------------------------------------
+// simple class capable of checking it's this pointer validity
+// ----------------------------------------------------------------------------
+
+class SelfPointingObject
+{
+public:
+    SelfPointingObject() { m_self = this; }
+    SelfPointingObject(const SelfPointingObject&) { m_self = this; }
+    ~SelfPointingObject() { CPPUNIT_ASSERT( this == m_self ); }
+
+private:
+    SelfPointingObject *m_self;
+};
+
+// ----------------------------------------------------------------------------

 // test class
 // test class

-// --------------------------------------------------------------------------
+// ----------------------------------------------------------------------------

 
 class VectorsTestCase : public CppUnit::TestCase
 {
 
 class VectorsTestCase : public CppUnit::TestCase
 {


@@ -38,12 +76,16 @@ private:
         CPPUNIT_TEST( Insert );
         CPPUNIT_TEST( Erase );
         CPPUNIT_TEST( Iterators );
         CPPUNIT_TEST( Insert );
         CPPUNIT_TEST( Erase );
         CPPUNIT_TEST( Iterators );

+        CPPUNIT_TEST( Objects );
+        CPPUNIT_TEST( NonPODs );

     CPPUNIT_TEST_SUITE_END();
 
     void PushPopTest();
     void Insert();
     void Erase();
     void Iterators();
     CPPUNIT_TEST_SUITE_END();
 
     void PushPopTest();
     void Insert();
     void Erase();
     void Iterators();

+    void Objects();
+    void NonPODs();

 
     DECLARE_NO_COPY_CLASS(VectorsTestCase)
 };
 
     DECLARE_NO_COPY_CLASS(VectorsTestCase)
 };


@@ -155,3 +197,41 @@ void VectorsTestCase::Iterators()
         CPPUNIT_ASSERT_EQUAL( value, *i );
     }
 }
         CPPUNIT_ASSERT_EQUAL( value, *i );
     }
 }

+
+void VectorsTestCase::Objects()
+{
+    wxVector<CountedObject> v;
+    v.push_back(CountedObject(1));
+    v.push_back(CountedObject(2));
+    v.push_back(CountedObject(3));
+
+    v.erase(v.begin());
+    WX_ASSERT_SIZET_EQUAL( 2, v.size() );
+    CPPUNIT_ASSERT_EQUAL( 2, CountedObject::GetCount() );
+
+    v.clear();
+    CPPUNIT_ASSERT_EQUAL( 0, CountedObject::GetCount() );
+}
+
+void VectorsTestCase::NonPODs()
+{
+    wxVector<SelfPointingObject> v;
+    v.push_back(SelfPointingObject());
+    v.push_back(SelfPointingObject());
+    v.push_back(SelfPointingObject());
+
+    v.erase(v.begin());
+    v.clear();
+
+    // try the same with wxString, which is not POD, but is implemented in
+    // a movable way (this won't assert, but would crash or show some memory
+    // problems under Valgrind if wxString couldn't be safely moved with
+    // memmove()):
+    wxVector<wxString> vs;
+    vs.push_back("one");
+    vs.push_back("two");
+    vs.push_back("three");
+
+    vs.erase(vs.begin());
+    vs.clear();
+}