0029258: Foundation Classes - provide move constructors for string classes
authorkgv <kgv@opencascade.com>
Sun, 15 Oct 2017 13:08:01 +0000 (16:08 +0300)
committerbugmaster <bugmaster@opencascade.com>
Thu, 9 Nov 2017 15:08:07 +0000 (18:08 +0300)
New macro OCCT_NO_RVALUE_REFERENCE is introduced to disable methods using move semantics on obsolete compilers that do not support rvalue references.

TCollection_AsciiString, TCollection_ExtendedString, NCollection_UtfString - added method Swap(), move constructor, and move assignment operator.

Draw command QATestArrayMove is added to test for memory corruption if NCollection_Array1<> bound to local C buffer is returned from function by value.

src/NCollection/NCollection_Array1.hxx
src/NCollection/NCollection_UtfString.hxx
src/NCollection/NCollection_UtfString.lxx
src/QANCollection/QANCollection_Test.cxx
src/Standard/Standard_Macro.hxx
src/TCollection/TCollection_AsciiString.cxx
src/TCollection/TCollection_AsciiString.hxx
src/TCollection/TCollection_ExtendedString.cxx
src/TCollection/TCollection_ExtendedString.hxx

index 85d2c43..91e2da2 100644 (file)
@@ -155,6 +155,7 @@ public:
   // ---------- PUBLIC METHODS ------------
 
   //! Empty constructor; should be used with caution.
+  //! @sa methods Resize() and Move().
   NCollection_Array1()
   : myLowerBound (1),
     myUpperBound (0),
@@ -188,25 +189,36 @@ public:
     Standard_OutOfMemory_Raise_if (!pBegin, "NCollection_Array1 : Allocation failed");
     myData = pBegin - myLowerBound;
 
-    *this = theOther;
+    Assign (theOther);
   }
 
+#ifndef OCCT_NO_RVALUE_REFERENCE
   //! Move constructor
-#if(defined(_MSC_VER) && (_MSC_VER < 1600))
-#else
   NCollection_Array1 (NCollection_Array1&& theOther)
   : myLowerBound (theOther.myLowerBound),
     myUpperBound (theOther.myUpperBound),
     myDeletable  (theOther.myDeletable),
     myData       (theOther.myData)
   {
-    theOther.myUpperBound = theOther.myLowerBound - 1;
     theOther.myDeletable  = false;
-    theOther.myData       = NULL;
   }
 #endif
 
-  //! C array-based constructor
+  //! C array-based constructor.
+  //!
+  //! Makes this array to use the buffer pointed by theBegin
+  //! instead of allocating it dynamically.
+  //! Argument theBegin should be a reference to the first element
+  //! of the pre-allocated buffer (usually local C array buffer),
+  //! with size at least theUpper - theLower + 1 items.
+  //!
+  //! Warning: returning array object created using this constructor
+  //! from function by value will result in undefined behavior
+  //! if compiler performs return value optimization (this is likely
+  //! to be true for all modern compilers in release mode).
+  //! The same happens if array is copied using Move() function
+  //! or move constructor and target object's lifespan is longer
+  //! than that of the buffer.
   NCollection_Array1 (const TheItemType& theBegin,
                       const Standard_Integer theLower,
                       const Standard_Integer theUpper) :
@@ -262,7 +274,9 @@ public:
   Standard_Boolean IsAllocated (void) const
   { return myDeletable; }
 
-  //! Assignment
+  //! Copies data of theOther array to this.
+  //! This array should be pre-allocated and have the same length as theOther;
+  //! otherwise exception Standard_DimensionMismatch is thrown.
   NCollection_Array1& Assign (const NCollection_Array1& theOther)
   {
     if (&theOther == this)
@@ -281,7 +295,10 @@ public:
     return *this;
   }
 
-  //! Move assignment
+  //! Move assignment.
+  //! This array will borrow all the data from theOther.
+  //! The moved object will keep pointer to the memory buffer and
+  //! range, but it will not free the buffer on destruction.
   NCollection_Array1& Move (NCollection_Array1& theOther)
   {
     if (&theOther == this)
@@ -293,26 +310,25 @@ public:
     {
       delete[] &myData[myLowerBound];
     }
+
     myLowerBound = theOther.myLowerBound;
     myUpperBound = theOther.myUpperBound;
     myDeletable  = theOther.myDeletable;
     myData       = theOther.myData;
 
-    theOther.myUpperBound = theOther.myLowerBound - 1;
-    theOther.myDeletable  = Standard_False;
-    theOther.myData       = NULL;
+    theOther.myDeletable = Standard_False;
+
     return *this;
   }
 
-  //! Assignment operator
+  //! Assignment operator; @sa Assign()
   NCollection_Array1& operator= (const NCollection_Array1& theOther)
   { 
     return Assign (theOther);
   }
 
-  //! Move assignment operator.
-#if(defined(_MSC_VER) && (_MSC_VER < 1600))
-#else
+#ifndef OCCT_NO_RVALUE_REFERENCE
+  //! Move assignment operator; @sa Move()
   NCollection_Array1& operator= (NCollection_Array1&& theOther)
   {
     return Move (theOther);
@@ -424,7 +440,10 @@ public:
 
   //! Destructor - releases the memory
   ~NCollection_Array1 (void)
-  { if (myDeletable) delete [] &(myData[myLowerBound]); }
+  { 
+    if (myDeletable) 
+      delete [] &(myData[myLowerBound]);
+  }
 
  protected:
   // ---------- PROTECTED FIELDS -----------
index 2cacbbb..210ac96 100755 (executable)
@@ -80,6 +80,11 @@ public:
   //! @param theCopy string to copy.
   NCollection_UtfString (const NCollection_UtfString& theCopy);
 
+#ifndef OCCT_NO_RVALUE_REFERENCE
+  //! Move constructor
+  NCollection_UtfString (NCollection_UtfString&& theOther);
+#endif
+
   //! Copy constructor from UTF-8 string.
   //! @param theCopyUtf8 UTF-8 string to copy
   //! @param theLength   optional length limit in Unicode symbols (NOT bytes!)
@@ -196,7 +201,18 @@ public:
 public: //! @name assign operators
 
   //! Copy from another string.
-  const NCollection_UtfString& operator= (const NCollection_UtfString& theOther);
+  const NCollection_UtfString& Assign (const NCollection_UtfString& theOther);
+
+  //! Exchange the data of two strings (without reallocating memory).
+  void Swap (NCollection_UtfString& theOther);
+
+  //! Copy from another string.
+  const NCollection_UtfString& operator= (const NCollection_UtfString& theOther) { return Assign (theOther); }
+
+#ifndef OCCT_NO_RVALUE_REFERENCE
+  //! Move assignment operator.
+  NCollection_UtfString& operator= (NCollection_UtfString&& theOther) { Swap (theOther); return *this; }
+#endif
 
   //! Copy from UTF-8 NULL-terminated string.
   const NCollection_UtfString& operator= (const char* theStringUtf8);
index e367203..e53cfb3 100755 (executable)
@@ -90,6 +90,23 @@ NCollection_UtfString<Type>::NCollection_UtfString (const NCollection_UtfString&
   strCopy ((Standard_Byte* )myString, (const Standard_Byte* )theCopy.myString, mySize);
 }
 
+#ifndef OCCT_NO_RVALUE_REFERENCE
+// =======================================================================
+// function : NCollection_UtfString
+// purpose  :
+// =======================================================================
+template<typename Type> inline
+NCollection_UtfString<Type>::NCollection_UtfString (NCollection_UtfString&& theOther)
+: myString(theOther.myString),
+  mySize  (theOther.mySize),
+  myLength(theOther.myLength)
+{
+  theOther.myString = NULL;
+  theOther.mySize   = 0;
+  theOther.myLength = 0;
+}
+#endif
+
 // =======================================================================
 // function : NCollection_UtfString
 // purpose  :
@@ -159,11 +176,11 @@ NCollection_UtfString<Type>::~NCollection_UtfString()
 }
 
 // =======================================================================
-// function : operator=
+// function : Assign
 // purpose  :
 // =======================================================================
 template<typename Type> inline
-const NCollection_UtfString<Type>& NCollection_UtfString<Type>::operator= (const NCollection_UtfString<Type>& theOther)
+const NCollection_UtfString<Type>& NCollection_UtfString<Type>::Assign (const NCollection_UtfString<Type>& theOther)
 {
   if (this == &theOther)
   {
@@ -178,6 +195,26 @@ const NCollection_UtfString<Type>& NCollection_UtfString<Type>::operator= (const
   return (*this);
 }
 
+// =======================================================================
+// function : Swap
+// purpose  :
+// =======================================================================
+template<typename Type> inline
+void NCollection_UtfString<Type>::Swap (NCollection_UtfString<Type>& theOther)
+{
+  // Note: we could use std::swap() here, but prefer to not
+  // have dependency on <algorithm> header at that level
+  Type* aString = myString;
+  const Standard_Integer aSize   = mySize;
+  const Standard_Integer aLength = myLength;
+  myString = theOther.myString;
+  mySize   = theOther.mySize;
+  myLength = theOther.myLength;
+  theOther.myString = aString;
+  theOther.mySize   = aSize;
+  theOther.myLength = aLength;
+}
+
 #if !defined(__ANDROID__)
 //! Auxiliary convertion tool.
 class NCollection_UtfStringTool
index f06d11d..493a177 100644 (file)
@@ -815,6 +815,54 @@ static Standard_Integer QANColTestSequence(Draw_Interpretor& di, Standard_Intege
   return 0;
 }
 
+//=======================================================================
+//function : QANColTestMove
+//purpose  : 
+//=======================================================================
+
+// Return array based on local C array buffer by value.
+// Note that this is expected to cause errors due
+// to the fact that returned copy will keep reference to the
+// buffer allocated in the stack of this function.
+// Unfortunately, this cannot be prevented due to the fact that
+// modern compilers use return value optimization in release mode
+// (object that is returned is constructed once at its target 
+// place and never copied).
+static NCollection_Array1<double> GetArrayByValue()
+{
+  const int aLen = 1024;
+  double aCArray[aLen];
+  NCollection_Array1<double> anArray (aCArray[0], 1, aLen);
+  for (int i = 1; i <= aLen; i++)
+    anArray.SetValue(i, i + 113.);
+  return anArray;
+}
+
+// check array for possible corruption
+static bool CheckArrayByValue(NCollection_Array1<double> theArray)
+{
+  for (int i = 1; i <= theArray.Length(); i++)
+  {
+    if (theArray.Value(i) != i + 113.)
+    {
+      std::cout << "Error at item " << i << ": value = " << theArray.Value(i) << ", expected " << i + 113. << std::endl;
+      return false;
+    }
+  }
+  return true;
+}
+
+static Standard_Integer QANColTestArrayMove (Draw_Interpretor& di, Standard_Integer argc, const char ** argv)
+{
+  if ( argc != 1) {
+    di << "Usage : " << argv[0] << "\n";
+    return 1;
+  }
+  NCollection_Array1<double> anArray = GetArrayByValue();
+  di << (CheckArrayByValue(anArray) ? "Error: memory corruption is not detected" : "Expected behavior: memory is corrupted");
+  return 0;
+}
+
 void QANCollection::CommandsTest(Draw_Interpretor& theCommands) {
   const char *group = "QANCollection";
 
@@ -829,4 +877,5 @@ void QANCollection::CommandsTest(Draw_Interpretor& theCommands) {
   theCommands.Add("QANColTestList",           "QANColTestList",           __FILE__, QANColTestList,           group);  
   theCommands.Add("QANColTestSequence",       "QANColTestSequence",       __FILE__, QANColTestSequence,       group);  
   theCommands.Add("QANColTestVector",         "QANColTestVector",         __FILE__, QANColTestVector,         group);  
+  theCommands.Add("QANColTestArrayMove",      "QANColTestArrayMove (is expected to give error)",      __FILE__, QANColTestArrayMove,      group);  
 }
index b1038fa..e2b6680 100644 (file)
   #define Standard_ENABLE_DEPRECATION_WARNINGS
 #endif
 
+//! @def OCCT_NO_RVALUE_REFERENCE
+//! Disables methods and constructors that use rvalue references
+//! (C++11 move semantics) not supported by obsolete compilers.
+#if (defined(_MSC_VER) && (_MSC_VER < 1600))
+  #define OCCT_NO_RVALUE_REFERENCE
+#endif
+
 # ifdef _WIN32
 
 // We must be careful including windows.h: it is really poisonous stuff!
index 027b975..03dbed0 100644 (file)
 #include <TCollection_ExtendedString.hxx>
 #include <TCollection_HAsciiString.hxx>
 
+#include <algorithm>
 #include <cctype>
 #include <cstdio>
 #include <cstring>
+
 // Shortcuts to standard allocate and reallocate functions
 static inline Standard_PCharacter Allocate(const Standard_Size aLength)
 {
@@ -417,6 +419,15 @@ void TCollection_AsciiString::Copy(const TCollection_AsciiString& fromwhere)
 }
 
 // ----------------------------------------------------------------------------
+// Swap
+// ----------------------------------------------------------------------------
+void TCollection_AsciiString::Swap (TCollection_AsciiString& theOther)
+{
+  std::swap (mystring, theOther.mystring);
+  std::swap (mylength, theOther.mylength);
+}
+
+// ----------------------------------------------------------------------------
 // Destroy
 // ----------------------------------------------------------------------------
 TCollection_AsciiString::~TCollection_AsciiString()
index cac5525..e402b71 100644 (file)
@@ -80,6 +80,17 @@ public:
   
   //! Initializes a AsciiString with another AsciiString.
   Standard_EXPORT TCollection_AsciiString(const TCollection_AsciiString& astring);
+
+#ifndef OCCT_NO_RVALUE_REFERENCE
+  //! Move constructor
+  Standard_EXPORT TCollection_AsciiString (TCollection_AsciiString&& theOther)
+  : mystring (theOther.mystring),
+    mylength (theOther.mylength)
+  {
+    theOther.mystring = NULL;
+    theOther.mylength = 0;
+  }
+#endif
   
   //! Initializes a AsciiString with copy of another AsciiString
   //! concatenated with the message character.
@@ -270,7 +281,15 @@ void operator = (const TCollection_AsciiString& fromwhere)
 {
   Copy(fromwhere);
 }
-  
+
+  //! Exchange the data of two strings (without reallocating memory).
+  Standard_EXPORT void Swap (TCollection_AsciiString& theOther);
+
+#ifndef OCCT_NO_RVALUE_REFERENCE
+  //! Move assignment operator
+  TCollection_AsciiString& operator= (TCollection_AsciiString&& theOther) { Swap (theOther); return *this; }
+#endif
+
   //! Frees memory allocated by AsciiString.
   Standard_EXPORT ~TCollection_AsciiString();
   
index c46da4d..5e0aef7 100644 (file)
@@ -23,6 +23,7 @@
 #include <Standard_OutOfRange.hxx>
 #include <TCollection_AsciiString.hxx>
 
+#include <algorithm>
 #include <cctype>
 #include <cstdio>
 
@@ -374,6 +375,15 @@ void TCollection_ExtendedString::Copy (const TCollection_ExtendedString& fromwhe
 }
 
 // ----------------------------------------------------------------------------
+// Swap
+// ----------------------------------------------------------------------------
+void TCollection_ExtendedString::Swap (TCollection_ExtendedString& theOther)
+{
+  std::swap (mystring, theOther.mystring);
+  std::swap (mylength, theOther.mylength);
+}
+
+// ----------------------------------------------------------------------------
 // Destroy
 // ----------------------------------------------------------------------------
 TCollection_ExtendedString::~TCollection_ExtendedString()
index 5f10fec..324dda4 100644 (file)
@@ -100,7 +100,18 @@ public:
   
   //! Initializes a ExtendedString with another ExtendedString.
   Standard_EXPORT TCollection_ExtendedString(const TCollection_ExtendedString& astring);
-  
+
+#ifndef OCCT_NO_RVALUE_REFERENCE
+  //! Move constructor
+  Standard_EXPORT TCollection_ExtendedString (TCollection_ExtendedString&& theOther)
+  : mystring (theOther.mystring),
+    mylength (theOther.mylength)
+  {
+    theOther.mystring = NULL;
+    theOther.mylength = 0;
+  }
+#endif
+
   //! Creation by converting an Ascii string to an extended
   //! string. The string is treated as having UTF-8 coding.
   //! If it is not a UTF-8 then each character is copied to ExtCharacter.
@@ -140,7 +151,15 @@ void operator = (const TCollection_ExtendedString& fromwhere)
 {
   Copy(fromwhere);
 }
-  
+
+  //! Exchange the data of two strings (without reallocating memory).
+  Standard_EXPORT void Swap (TCollection_ExtendedString& theOther);
+
+#ifndef OCCT_NO_RVALUE_REFERENCE
+  //! Move assignment operator
+  TCollection_ExtendedString& operator= (TCollection_ExtendedString&& theOther) { Swap (theOther); return *this; }
+#endif
+
   //! Frees memory allocated by ExtendedString.
   Standard_EXPORT ~TCollection_ExtendedString();