From 52bb4675a9f9c381f290c6831c7df99d7f7da5f1 Mon Sep 17 00:00:00 2001 From: ski Date: Wed, 28 Sep 2016 17:13:47 +0300 Subject: [PATCH] 0027571: Bad implementation of NCollection_Sequence::Iterator Reverse iterator after creation points to the first iterated item now (last item of sequence). Test case was created. --- src/NCollection/NCollection_BaseSequence.hxx | 16 ++- src/NCollection/NCollection_Sequence.hxx | 30 ++++- src/NCollection/NCollection_StlIterator.hxx | 4 +- src/QABugs/QABugs_20.cxx | 71 +++++++++++ tests/bugs/fclasses/bug27571 | 118 +++++++++++++++++++ 5 files changed, 229 insertions(+), 10 deletions(-) create mode 100644 tests/bugs/fclasses/bug27571 diff --git a/src/NCollection/NCollection_BaseSequence.hxx b/src/NCollection/NCollection_BaseSequence.hxx index 42f4197b0e..1be1c86c2c 100644 --- a/src/NCollection/NCollection_BaseSequence.hxx +++ b/src/NCollection/NCollection_BaseSequence.hxx @@ -58,7 +58,7 @@ public: { public: //! Empty constructor - Iterator (void) : myCurrent (NULL), myPrevious(NULL) {} + Iterator (void) : myCurrent (NULL), myPrevious(NULL), isReversed(Standard_False) {} //! Constructor with initialisation Iterator (const NCollection_BaseSequence& theSeq, @@ -71,8 +71,9 @@ public: void Init (const NCollection_BaseSequence& theSeq, const Standard_Boolean isStart = Standard_True) { - myCurrent = (isStart ? theSeq.myFirstItem : NULL); - myPrevious = (isStart ? NULL : theSeq.myLastItem); + myCurrent = (isStart ? theSeq.myFirstItem : theSeq.myLastItem); + myPrevious = NULL; + isReversed = !isStart; } //! Assignment @@ -86,13 +87,18 @@ public: void Previous() { myCurrent = myPrevious; - if (myCurrent) - myPrevious = myCurrent->Previous(); + if (myCurrent) { + if (!isReversed) + myPrevious = myCurrent->Previous(); + else + myPrevious = myCurrent->Next(); + } } protected: NCollection_SeqNode* myCurrent; //!< Pointer to the current node NCollection_SeqNode* myPrevious; //!< Pointer to the previous node + Standard_Boolean isReversed; friend class NCollection_BaseSequence; }; diff --git a/src/NCollection/NCollection_Sequence.hxx b/src/NCollection/NCollection_Sequence.hxx index f50896cf02..d8abac79c2 100644 --- a/src/NCollection/NCollection_Sequence.hxx +++ b/src/NCollection/NCollection_Sequence.hxx @@ -71,7 +71,10 @@ public: if (myCurrent) { myPrevious = myCurrent; - myCurrent = myCurrent->Next(); + if (isReversed) + myCurrent = myCurrent->Previous(); + else + myCurrent = myCurrent->Next(); } } //! Constant value access @@ -93,19 +96,38 @@ public: //! Shorthand for a constant iterator type. typedef NCollection_StlIterator const_iterator; + //! Shorthand for a reverse iterator type. + typedef NCollection_StlIterator, Iterator, TheItemType, false> reverse_iterator; + + //! Shorthand for a constant reverse iterator type. + typedef NCollection_StlIterator, Iterator, TheItemType, true> const_reverse_iterator; + //! Returns an iterator pointing to the first element in the sequence. iterator begin() const { return Iterator (*this, true); } //! Returns an iterator referring to the past-the-end element in the sequence. - iterator end() const { Iterator anIter (*this, false); anIter.Next(); return anIter; } + iterator end() const { Iterator anIter (*this, false); anIter.Previous(); return anIter; } //! Returns a const iterator pointing to the first element in the sequence. const_iterator cbegin() const { return Iterator (*this, true); } //! Returns a const iterator referring to the past-the-end element in the sequence. - const_iterator cend() const { Iterator anIter (*this, false); anIter.Next(); return anIter; } + const_iterator cend() const { Iterator anIter(*this, false); anIter.Previous(); return anIter; } - public: + //! Returns a reverse iterator pointing to the last element in the sequence. + reverse_iterator rbegin() const { return Iterator(*this, false); } + + //! Returns a reverse iterator referring to the past-the-end element in the reversed sequence. + reverse_iterator rend() const { Iterator anIter(*this, true); anIter.Previous(); return anIter; } + + //! Returns a const reverse iterator pointing to the last element in the sequence. + const_reverse_iterator crbegin() const { return Iterator(*this, false); } + + //! Returns a const reverse iterator referring to the past-the-end element in the reversed sequence. + const_reverse_iterator crend() const { Iterator anIter(*this, true); anIter.Previous(); return anIter; } + + +public: // ---------- PUBLIC METHODS ------------ //! Constructor diff --git a/src/NCollection/NCollection_StlIterator.hxx b/src/NCollection/NCollection_StlIterator.hxx index d666270ecd..93e0ed2edd 100644 --- a/src/NCollection/NCollection_StlIterator.hxx +++ b/src/NCollection/NCollection_StlIterator.hxx @@ -166,7 +166,9 @@ public: //! @name methods related to bidirectional STL iterator NCollection_StlIterator& operator--() { Standard_STATIC_ASSERT((opencascade::is_same::value || - opencascade::is_same::value)); + opencascade::is_same::value || + opencascade::is_same>, Category>::value || + opencascade::is_same>, Category>::value)); myIterator.Previous(); return *this; } diff --git a/src/QABugs/QABugs_20.cxx b/src/QABugs/QABugs_20.cxx index eae027dba5..56c7c6900c 100644 --- a/src/QABugs/QABugs_20.cxx +++ b/src/QABugs/QABugs_20.cxx @@ -2114,6 +2114,76 @@ static Standard_Integer OCC27552(Draw_Interpretor&, return 0; } + +#include +static Standard_Integer OCC27571(Draw_Interpretor& di, Standard_Integer /*nb*/, const char ** /*a*/) +{ + // Create NCollection_Sequence + NCollection_Sequence aSequence; + aSequence.Append("1"); + aSequence.Append("2"); + aSequence.Append("3"); + aSequence.Append("4"); + aSequence.Append("5"); + + // Create iterator + NCollection_Sequence::Iterator anIterator(aSequence); + + // Create reverse iterator + NCollection_Sequence::Iterator aReverseIterator(aSequence, Standard_False); + + // Display elements using iterator (with More/Next) + di << "Iterator (More/Next):"; + for (; anIterator.More(); anIterator.Next()) + di << " " << anIterator.Value(); + anIterator.Previous(); + di << "\nCurrent (last) element - Iterator: " << anIterator.Value(); + anIterator.Previous(); + di << "\nPrevious element - Iterator: " << anIterator.Value(); + + // Display elements using reverse iterator (with More/Next) + di << "\nReverse iterator (More/Next):"; + for (; aReverseIterator.More(); aReverseIterator.Next()) + di << " " << aReverseIterator.Value(); + aReverseIterator.Previous(); + di << "\nCurrent (last) element - Reverse iterator: " << aReverseIterator.Value(); + aReverseIterator.Previous(); + di << "\nPrevious element - Reverse iterator: " << aReverseIterator.Value(); + + // Display elements using iterator (with begin/end) + di << "\nIterator (begin/end):"; + NCollection_Sequence::iterator anIteratorBeginEnd = aSequence.begin(); + for (; anIteratorBeginEnd != aSequence.end(); ++anIteratorBeginEnd) + di << " " << *anIteratorBeginEnd; + --anIteratorBeginEnd; + di << "\nCurrent (last) element - Iterator (begin/end): " << *anIteratorBeginEnd; + + // Display elements using const_iterator (with cbegin/cend) + di << "\nIterator (cbegin/cend) Const:"; + NCollection_Sequence::const_iterator anIteratorBeginEndConst = aSequence.cbegin(); + for (; anIteratorBeginEndConst != aSequence.cend(); ++anIteratorBeginEndConst) + di << " " << *anIteratorBeginEndConst; + --anIteratorBeginEndConst; + di << "\nCurrent (last) element - Iterator (cbegin/cend) Const: " << *anIteratorBeginEndConst; + + // Display elements using reverse_iterator (with rbegin/rend) + di << "\nReverse iterator (rbegin/rend):"; + NCollection_Sequence::reverse_iterator aReverseIteratorBeginEnd = aSequence.rbegin(); + for (; aReverseIteratorBeginEnd != aSequence.rend(); ++aReverseIteratorBeginEnd) + di << " " << *aReverseIteratorBeginEnd; + --aReverseIteratorBeginEnd; + di << "\nCurrent (last) element - Reverse iterator (rbegin/rend): " << *aReverseIteratorBeginEnd; + + // Display elements using const_reverse_iterator (with crbegin/crend) + di << "\nReverse iterator (crbegin/crend) Const:"; + NCollection_Sequence::const_reverse_iterator aReverseIteratorBeginEndConst = aSequence.crbegin(); + for (; aReverseIteratorBeginEndConst != aSequence.crend(); ++aReverseIteratorBeginEndConst) + di << " " << *aReverseIteratorBeginEndConst; + --aReverseIteratorBeginEndConst; + di << "\nCurrent (last) element - Reverse iterator (crbegin/crend) Const: " << *aReverseIteratorBeginEndConst; + return 0; +} + void QABugs::Commands_20(Draw_Interpretor& theCommands) { const char *group = "QABugs"; @@ -2132,5 +2202,6 @@ void QABugs::Commands_20(Draw_Interpretor& theCommands) { theCommands.Add ("OCC27357", "OCC27357", __FILE__, OCC27357, group); theCommands.Add("OCC26270", "OCC26270 shape result", __FILE__, OCC26270, group); theCommands.Add ("OCC27552", "OCC27552", __FILE__, OCC27552, group); + theCommands.Add("OCC27571", "OCC27571", __FILE__, OCC27571, group); return; } diff --git a/tests/bugs/fclasses/bug27571 b/tests/bugs/fclasses/bug27571 new file mode 100644 index 0000000000..52f0798777 --- /dev/null +++ b/tests/bugs/fclasses/bug27571 @@ -0,0 +1,118 @@ +puts "========" +puts "OCC27571" +puts "========" +puts "" +######################################################## +# Bad implementation of NCollection_Sequence::Iterator +######################################################## + +pload QAcommands + +set info [OCC27571] + +puts "" + +set Sequence "1 2 3 4 5" +set Sequence_Last "5" +set Sequence_Preult "4" +set ReversedSequence "5 4 3 2 1" +set ReversedSequence_Last "1" +set ReversedSequence_Preult "2" + +# Iterator using Init(aSequence, Standard_True) method. +if { [regexp "Iterator \\(More/Next\\): ${Sequence}" $info] } { + puts "OK : Iterator (More/Next)" +} else { + puts "Error : Iterator (More/Next)" +} + +# Method Previous() called after full iteration, current value of Iterator is equal to last element of sequence +if { [regexp "Current \\(last\\) element - Iterator: ${Sequence_Last}" $info] } { + puts "OK : Current (last) element - Iterator" +} else { + puts "Error : Current (last) element - Iterator" +} + +# Method Previous() called once more again, current value of Iterator is equal to penult element of sequence +if { [regexp "Previous element - Iterator: ${Sequence_Preult}" $info] } { + puts "OK : Previous element - Iterator" +} else { + puts "Error : Previous element - Iterator" +} + +# Reverse iterator using Init(aSequence, Standard_False) method. +if { [regexp "Reverse iterator \\(More/Next\\): ${ReversedSequence}" $info] } { + puts "OK : Reverse iterator (More/Next)" +} else { + puts "Error : Reverse iterator (More/Next)" +} + +# Method Previous() called after full iteration, current value of Iterator is equal to last element of sequence +if { [regexp "Current \\(last\\) element - Reverse iterator: ${ReversedSequence_Last}" $info] } { + puts "OK : Current (last) element - Reverse iterator" +} else { + puts "Error : Current (last) element - Reverse iterator" +} + +# Method Previous() called once more again, current value of Iterator is equal to penult element of sequence +if { [regexp "Previous element - Reverse iterator: ${ReversedSequence_Preult}" $info] } { + puts "OK : Previous element - Reverse iterator" +} else { + puts "Error : Previous element - Reverse iterator" +} + +# Iterator using method "begin". +if { [regexp "Iterator \\(begin/end\\): ${Sequence}" $info] } { + puts "OK : Iterator (begin/end)" +} else { + puts "Error : Iterator (begin/end)" +} + +# Check previous element after full iteration, current value of Iterator is equal to last element of sequence +if { [regexp "Current \\(last\\) element - Iterator \\(begin/end\\): ${Sequence_Last}" $info] } { + puts "OK : Current (last) element - Iterator (begin/end)" +} else { + puts "Error : Current (last) element - Iterator (begin/end)" +} + +# Const iterator using method "begin". +if { [regexp "Iterator \\(cbegin/cend\\) Const: ${Sequence}" $info] } { + puts "OK : Iterator (cbegin/cend) Const" +} else { + puts "Error : Iterator (cbegin/cend) Const" +} + +# Check previous element after full iteration, current value of Iterator is equal to last element of sequence +if { [regexp "Current \\(last\\) element - Iterator \\(cbegin/cend\\) Const: ${Sequence_Last}" $info] } { + puts "OK : Current (last) element - Iterator (cbegin/cend) Const" +} else { + puts "Error : Current (last) element - Iterator (cbegin/cend) Const" +} + +# Reverse iterator using method "rbegin". +if { [regexp "Reverse iterator \\(rbegin/rend\\): ${ReversedSequence}" $info] } { + puts "OK : Reverse iterator (rbegin/rend)" +} else { + puts "Error : Reverse iterator (rbegin/rend)" +} + +# Check previous element after full iteration, current value of Iterator is equal to last element of sequence +if { [regexp "Current \\(last\\) element - Reverse iterator \\(rbegin/rend\\): ${ReversedSequence_Last}" $info] } { + puts "OK : Current (last) element - Iterator (rbegin/rend)" +} else { + puts "Error : Current (last) element - Iterator (rbegin/rend)" +} + +# Const reverse iterator using method "crbegin". +if { [regexp "Reverse iterator \\(crbegin/crend\\) Const: ${ReversedSequence}" $info] } { + puts "OK : Reverse iterator (crbegin/crend) Const" +} else { + puts "Error : Reverse iterator (crbegin/crend) Const" +} + +# Check previous element after full iteration, current value of Iterator is equal to last element of sequence +if { [regexp "Current \\(last\\) element - Reverse iterator \\(crbegin/crend\\) Const: ${ReversedSequence_Last}" $info] } { + puts "OK : Current (last) element - Iterator (rbegin/rend) Const" +} else { + puts "Error : Current (last) element - Iterator (rbegin/rend) Const" +} -- 2.39.5