-
Notifications
You must be signed in to change notification settings - Fork 0
Foundation Classes - Implement move semantics for math_Matrix and math_Vector #61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ | |
| #include <Standard_Boolean.hxx> | ||
|
|
||
| #include <array> | ||
| #include <utility> | ||
|
|
||
| class math_DoubleTab | ||
| { | ||
|
|
@@ -77,9 +78,34 @@ public: | |
| { | ||
| } | ||
|
|
||
| //! Move constructor | ||
| math_DoubleTab(math_DoubleTab&& theOther) noexcept | ||
| : myBuffer{}, | ||
| myArray(theOther.myArray.IsDeletable() | ||
| ? std::move(theOther.myArray) | ||
| : (theOther.NbRows() * theOther.NbColumns() <= THE_BUFFER_SIZE | ||
| ? NCollection_Array2<Standard_Real>(*myBuffer.data(), | ||
| theOther.LowerRow(), | ||
| theOther.UpperRow(), | ||
| theOther.LowerCol(), | ||
| theOther.UpperCol()) | ||
| : NCollection_Array2<Standard_Real>(theOther.LowerRow(), | ||
| theOther.UpperRow(), | ||
| theOther.LowerCol(), | ||
| theOther.UpperCol()))) | ||
| { | ||
| if (!theOther.myArray.IsEmpty()) | ||
| { | ||
| myArray.Assign(theOther.myArray); | ||
| } | ||
| } | ||
|
|
||
| //! Copy data to theOther | ||
| void Copy(math_DoubleTab& theOther) const { theOther.myArray.Assign(myArray); } | ||
|
|
||
| //! Returns true if the internal array is deletable (heap-allocated) | ||
| Standard_Boolean IsDeletable() const { return myArray.IsDeletable(); } | ||
|
|
||
| //! Set lower row index | ||
| void SetLowerRow(const Standard_Integer theLowerRow) { myArray.UpdateLowerRow(theLowerRow); } | ||
|
|
||
|
|
@@ -130,6 +156,39 @@ public: | |
| return Value(theRowIndex, theColIndex); | ||
| } | ||
|
|
||
| //! Assignment operator | ||
| math_DoubleTab& operator=(const math_DoubleTab& theOther) | ||
| { | ||
| if (this != &theOther) | ||
| { | ||
| myArray = theOther.myArray; | ||
| } | ||
| return *this; | ||
| } | ||
|
|
||
| //! Move assignment operator | ||
| math_DoubleTab& operator=(math_DoubleTab&& theOther) noexcept | ||
| { | ||
| if (this == &theOther) | ||
| { | ||
| return *this; | ||
| } | ||
|
|
||
| if (myArray.IsDeletable() && theOther.myArray.IsDeletable() | ||
| && myArray.NbRows() == theOther.myArray.NbRows() | ||
| && myArray.NbColumns() == theOther.myArray.NbColumns() | ||
| && myArray.LowerRow() == theOther.myArray.LowerRow() | ||
| && myArray.LowerCol() == theOther.myArray.LowerCol()) | ||
| { | ||
| myArray.Move(theOther.myArray); | ||
| } | ||
| else | ||
| { | ||
| myArray = theOther.myArray; | ||
|
||
| } | ||
| return *this; | ||
| } | ||
|
Comment on lines
+169
to
+190
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove The move assignment operator is marked Additionally, line 187 uses copy assignment instead of move assignment. Since Apply this diff to remove the unsafe - math_DoubleTab& operator=(math_DoubleTab&& theOther) noexcept
+ math_DoubleTab& operator=(math_DoubleTab&& theOther)
{
if (this == &theOther)Optionally, consider this diff for the performance optimization: else
{
- myArray = theOther.myArray;
+ myArray = std::move(theOther.myArray);
}🤖 Prompt for AI Agents |
||
|
|
||
| //! Destructor | ||
| ~math_DoubleTab() = default; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -701,12 +701,23 @@ inline math_Matrix& math_Matrix::Initialized(const math_Matrix& Other) | |||||||||
|
|
||||||||||
| //================================================================================================== | ||||||||||
|
|
||||||||||
| inline math_Matrix& math_Matrix::operator=(math_Matrix&& Other) noexcept | ||||||||||
| inline math_Matrix& math_Matrix::operator=(math_Matrix&& Other) | ||||||||||
| { | ||||||||||
| if (this != &Other) | ||||||||||
| if (this == &Other) | ||||||||||
| { | ||||||||||
| return *this; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (Array.IsDeletable() && Other.Array.IsDeletable() && RowNumber() == Other.RowNumber() | ||||||||||
| && ColNumber() == Other.ColNumber() && LowerRow() == Other.LowerRow() | ||||||||||
| && LowerCol() == Other.LowerCol()) | ||||||||||
| { | ||||||||||
| Array = std::move(Other.Array); | ||||||||||
| } | ||||||||||
| else | ||||||||||
| { | ||||||||||
| Initialized(Other); | ||||||||||
|
||||||||||
| Initialized(Other); | |
| // Copy Other before any move operations to avoid copying from a moved-from object | |
| math_Matrix tempOther(Other); | |
| Initialized(tempOther); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -76,6 +76,23 @@ math_VectorBase<TheItemType>::math_VectorBase(const math_VectorBase<TheItemType> | |||||
| { | ||||||
| } | ||||||
|
|
||||||
| template <typename TheItemType> | ||||||
| math_VectorBase<TheItemType>::math_VectorBase(math_VectorBase<TheItemType>&& theOther) noexcept | ||||||
| : myBuffer{}, | ||||||
| Array(theOther.Array.IsDeletable() | ||||||
| ? std::move(theOther.Array) | ||||||
| : (theOther.Length() <= math_VectorBase::THE_BUFFER_SIZE | ||||||
| ? NCollection_Array1<TheItemType>(*myBuffer.data(), | ||||||
| theOther.Lower(), | ||||||
| theOther.Upper()) | ||||||
| : NCollection_Array1<TheItemType>(theOther.Lower(), theOther.Upper()))) | ||||||
| { | ||||||
| if (!theOther.Array.IsEmpty()) | ||||||
| { | ||||||
| Array.Assign(theOther.Array); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| template <typename TheItemType> | ||||||
| void math_VectorBase<TheItemType>::SetLower(const Standard_Integer theLower) | ||||||
| { | ||||||
|
|
@@ -543,6 +560,27 @@ math_VectorBase<TheItemType>& math_VectorBase<TheItemType>::Initialized( | |||||
| return *this; | ||||||
| } | ||||||
|
|
||||||
| template <typename TheItemType> | ||||||
| math_VectorBase<TheItemType>& math_VectorBase<TheItemType>::operator=( | ||||||
| math_VectorBase<TheItemType>&& theOther) | ||||||
| { | ||||||
| if (this == &theOther) | ||||||
| { | ||||||
| return *this; | ||||||
| } | ||||||
|
|
||||||
| if (Array.IsDeletable() && theOther.Array.IsDeletable() && Lower() == theOther.Lower() | ||||||
| && Length() == theOther.Length()) | ||||||
| { | ||||||
| Array.Move(theOther.Array); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| Initialized(theOther); | ||||||
|
||||||
| Initialized(theOther); | |
| Initialized(static_cast<const math_VectorBase<TheItemType>&>(theOther)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
noexcept—move constructor can throw when allocating for large external arrays.The move constructor is marked
noexcept, but line 92–95 allocates a heap-backedNCollection_Array2when the source is a large non-deletable array (exceedsTHE_BUFFER_SIZE). Heap allocation can throwbad_alloc, violating thenoexceptguarantee and causingstd::terminateif an exception escapes.Apply this diff to remove the unsafe
noexcept:🤖 Prompt for AI Agents