Stop using setSharable in the Java-style mutable iterators

First and foremost, the STL-style iterators don't do this. Those don't
provide a guarantee that the container won't get shared again while the
iterator is active.

Second, there's no protection against a second mutable iterator being
created and resetting the sharable flag back to true.

[ChangeLog][Important behavior changes] The mutable Java-style iterators
like QListMutableIterator and QHashMutableIterator no longer set the
parent container to unsharable mode. If you create a copy of the
container being iterated on after the iterator, any changes done with
the iterator might affect the copy too.

Discussed-on: http://lists.qt-project.org/pipermail/development/2014-February/015724.html
Change-Id: Iccfe411d5558c85ae459cff944215614c392388e
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@digia.com>
This commit is contained in:
Thiago Macieira 2014-04-03 15:44:59 -07:00 committed by The Qt Project
parent 1aede2d7fc
commit bf3e80023a
4 changed files with 7 additions and 125 deletions

View File

@ -87,12 +87,9 @@ class QMutable##C##Iterator \
public: \
inline QMutable##C##Iterator(Q##C<T> &container) \
: c(&container) \
{ c->setSharable(false); i = c->begin(); n = c->end(); } \
inline ~QMutable##C##Iterator() \
{ c->setSharable(true); } \
{ i = c->begin(); n = c->end(); } \
inline QMutable##C##Iterator &operator=(Q##C<T> &container) \
{ c->setSharable(true); c = &container; c->setSharable(false); \
i = c->begin(); n = c->end(); return *this; } \
{ c = &container; i = c->begin(); n = c->end(); return *this; } \
inline void toFront() { i = c->begin(); n = c->end(); } \
inline void toBack() { i = c->end(); n = i; } \
inline bool hasNext() const { return c->constEnd() != const_iterator(i); } \
@ -160,11 +157,9 @@ class QMutable##C##Iterator \
public: \
inline QMutable##C##Iterator(Q##C<Key,T> &container) \
: c(&container) \
{ c->setSharable(false); i = c->begin(); n = c->end(); } \
inline ~QMutable##C##Iterator() \
{ c->setSharable(true); } \
{ i = c->begin(); n = c->end(); } \
inline QMutable##C##Iterator &operator=(Q##C<Key,T> &container) \
{ c->setSharable(true); c = &container; c->setSharable(false); i = c->begin(); n = c->end(); return *this; } \
{ c = &container; i = c->begin(); n = c->end(); return *this; } \
inline void toFront() { i = c->begin(); n = c->end(); } \
inline void toBack() { i = c->end(); n = c->end(); } \
inline bool hasNext() const { return const_iterator(i) != c->constEnd(); } \

View File

@ -501,17 +501,6 @@
\sa operator=()
*/
/*!
\fn QMutableListIterator::~QMutableListIterator()
\fn QMutableLinkedListIterator::~QMutableLinkedListIterator()
\fn QMutableVectorIterator::~QMutableVectorIterator()
\fn QMutableSetIterator::~QMutableSetIterator()
Destroys the iterator.
\sa operator=()
*/
/*! \fn QMutableListIterator &QMutableListIterator::operator=(QList<T> &list)
\fn QMutableLinkedListIterator &QMutableLinkedListIterator::operator=(QLinkedList<T> &list)
\fn QListIterator &QListIterator::operator=(const QList<T> &list)
@ -1121,15 +1110,6 @@
\sa operator=()
*/
/*!
\fn QMutableMapIterator::~QMutableMapIterator()
\fn QMutableHashIterator::~QMutableHashIterator()
Destroys the iterator.
\sa operator=()
*/
/*! \fn QMapIterator &QMapIterator::operator=(const QMap<Key, T> &map)
\fn QMutableMapIterator &QMutableMapIterator::operator=(QMap<Key, T> &map)

View File

@ -356,12 +356,9 @@ class QMutableSetIterator
public:
inline QMutableSetIterator(QSet<T> &container)
: c(&container)
{ c->setSharable(false); i = c->begin(); n = c->end(); }
inline ~QMutableSetIterator()
{ c->setSharable(true); }
{ i = c->begin(); n = c->end(); }
inline QMutableSetIterator &operator=(QSet<T> &container)
{ c->setSharable(true); c = &container; c->setSharable(false);
i = c->begin(); n = c->end(); return *this; }
{ c = &container; i = c->begin(); n = c->end(); return *this; }
inline void toFront() { i = c->begin(); n = c->end(); }
inline void toBack() { i = c->end(); n = i; }
inline bool hasNext() const { return c->constEnd() != i; }

View File

@ -2437,8 +2437,7 @@ void testContainer()
}
/*
Verify that the 'sharable' flag is true while no mutable
iterator is active.
Verify that the 'sharable' flag is true in populated containers.
*/
{
Container c1;
@ -2453,95 +2452,6 @@ void testContainer()
QVERIFY(!c2.isDetached());
}
/*
Verify that the 'sharable' flag is set to false by the
mutable iterator.
*/
{
Container c1;
populate(c1);
QVERIFY(c1.size() == 4);
QVERIFY(c1.isDetached());
ContainerMutableIterator i(c1);
i.next();
Container c2 = c1;
QVERIFY(c1.size() == 4);
QVERIFY(c2.size() == 4);
QVERIFY(c1.isDetached());
QVERIFY(c2.isDetached());
i.remove();
QVERIFY(c1.size() == 3);
QVERIFY(c2.size() == 4);
}
/*
Verify that the 'sharable' flag is reset to true by the
mutable iterator's destructor.
*/
{
Container c1;
populate(c1);
QVERIFY(c1.size() == 4);
QVERIFY(c1.isDetached());
{
ContainerMutableIterator i(c1);
i.next();
}
Container c2 = c1;
QVERIFY(c1.size() == 4);
QVERIFY(c2.size() == 4);
QVERIFY(!c1.isDetached());
QVERIFY(!c2.isDetached());
}
/*
Verify that the 'sharable' flag only affects the original
object, not the copies.
*/
{
Container c1;
populate(c1);
QVERIFY(c1.size() == 4);
QVERIFY(c1.isDetached());
Container c2 = c1;
QVERIFY(isSharable(c2));
ContainerMutableIterator i(c1);
QVERIFY(!isSharable(c1));
QVERIFY(isSharable(c2));
Container c3 = c1;
QVERIFY(!isSharable(c1));
QVERIFY(isSharable(c2));
QVERIFY(isSharable(c3));
QVERIFY(c1.isDetached());
QVERIFY(c2.isDetached());
QVERIFY(c3.isDetached());
Container c4;
c4 = c1;
QVERIFY(!isSharable(c1));
QVERIFY(isSharable(c2));
QVERIFY(isSharable(c4));
QVERIFY(c1.isDetached());
QVERIFY(c2.isDetached());
QVERIFY(c4.isDetached());
c3 = c2;
QVERIFY(!isSharable(c1));
QVERIFY(isSharable(c2));
QVERIFY(isSharable(c3));
QVERIFY(c1.isDetached());
QVERIFY(!c2.isDetached());
QVERIFY(!c3.isDetached());
}
/* test that the move operators work properly */
{
Container c1 = Container(newInstance<Container>());