From 405ad7165e4f71d86cd4178f7fe890cc3f5c6d70 Mon Sep 17 00:00:00 2001 From: Mathew Sutcliffe Date: Sat, 11 Jan 2014 16:53:21 +0000 Subject: [PATCH] Iterator classes: removed templated ctors and added static methods to replace them, for consistency with the container classes and to avoid any potential infinite recursion with MSVC2010. See also commit:6a9065b Also fixed a mistake in CSequence and CCollection, where Pimpl::erase was calling the wrong version of m_impl.erase reported by Roland. --- src/blackmisc/collection.h | 21 ++++++------ src/blackmisc/iterator.h | 69 +++++++++++++++++--------------------- src/blackmisc/sequence.h | 18 +++++----- 3 files changed, 50 insertions(+), 58 deletions(-) diff --git a/src/blackmisc/collection.h b/src/blackmisc/collection.h index 72ca06dec..a01319024 100644 --- a/src/blackmisc/collection.h +++ b/src/blackmisc/collection.h @@ -238,8 +238,8 @@ namespace BlackMisc struct base { void push_back(); }; template struct derived : public C, public base {}; static yes hasPushHelper(...); template static no hasPushHelper(D *, typecheck * = 0); template struct hasPush : public std::integral_constant*)0)) == sizeof(yes)> {}; - template static iterator insertImpl(typename std::enable_if< hasPush::value, C>::type &c, const T &value) { c.push_back(value); return c.end() - 1; } - template static iterator insertImpl(typename std::enable_if < !hasPush::value, C >::type &c, const T &value) { return c.insert(value); } + template static iterator insertImpl(typename std::enable_if< hasPush::value, C>::type &c, const T &value) { c.push_back(value); return iterator::fromImpl(c.end() - 1); } + template static iterator insertImpl(typename std::enable_if < !hasPush::value, C >::type &c, const T &value) { return iterator::fromImpl(c.insert(value)); } }; template class Pimpl : public PimplBase @@ -249,18 +249,19 @@ namespace BlackMisc Pimpl(C &&c) : m_impl(std::move(c)) {} PimplBase *clone() const { return new Pimpl(*this); } PimplBase *cloneEmpty() const { return new Pimpl(C()); } - iterator begin() { return m_impl.begin(); } - const_iterator begin() const { return m_impl.cbegin(); } - const_iterator cbegin() const { return m_impl.cbegin(); } - iterator end() { return m_impl.end(); } - const_iterator end() const { return m_impl.cend(); } - const_iterator cend() const { return m_impl.cend(); } + iterator begin() { return iterator::fromImpl(m_impl.begin()); } + const_iterator begin() const { return const_iterator::fromImpl(m_impl.cbegin()); } + const_iterator cbegin() const { return const_iterator::fromImpl(m_impl.cbegin()); } + iterator end() { return iterator::fromImpl(m_impl.end()); } + const_iterator end() const { return const_iterator::fromImpl(m_impl.cend()); } + const_iterator cend() const { return const_iterator::fromImpl(m_impl.cend()); } size_type size() const { return m_impl.size(); } bool empty() const { return m_impl.empty(); } void clear() { m_impl.clear(); } iterator insert(const T &value) { return PimplBase::insertImpl(m_impl, value); } - iterator erase(iterator pos) { return m_impl.erase(*static_cast(pos.getImpl())); } - iterator erase(iterator it1, iterator it2) { return m_impl.erase(*static_cast(it1.getImpl(), it2.getImpl())); } + iterator erase(iterator pos) { return iterator::fromImpl(m_impl.erase(*static_cast(pos.getImpl()))); } + //iterator erase(iterator it1, iterator it2) { return iterator::fromImpl(m_impl.erase(*static_cast(it1.getImpl()), *static_cast(it2.getImpl()))); } + iterator erase(iterator it1, iterator it2) { while (it1 != it2) { it1 = iterator::fromImpl(m_impl.erase(*static_cast(it1.getImpl()))); } return it1; } bool operator ==(const PimplBase &other) const { Pimpl copy = C(); for (auto i = other.cbegin(); i != other.cend(); ++i) copy.insert(*i); return m_impl == copy.m_impl; } private: C m_impl; diff --git a/src/blackmisc/iterator.h b/src/blackmisc/iterator.h index f238ef2e7..79396ba59 100644 --- a/src/blackmisc/iterator.h +++ b/src/blackmisc/iterator.h @@ -44,12 +44,6 @@ namespace BlackMisc //! \brief Default constructor. ConstForwardIterator() {} - /*! - * \brief Constructor. - * \param i - */ - template ConstForwardIterator(I i) : m_pimpl(new Pimpl(std::move(i))) {} - /*! * \brief Copy constructor. * \param other @@ -62,13 +56,6 @@ namespace BlackMisc */ ConstForwardIterator(ConstForwardIterator &&other) : m_pimpl(other.m_pimpl.take()) {} - /*! - * \brief Assignment. - * \param i - * \return - */ - template ConstForwardIterator &operator =(I i) { m_pimpl.reset(new Pimpl(std::move(i))); return *this; } - /*! * \brief Copy assignment. * \param other @@ -83,6 +70,14 @@ namespace BlackMisc */ ConstForwardIterator &operator =(ConstForwardIterator &&other) { m_pimpl.reset(other.m_pimpl.take()); return *this; } + /*! + * \brief Create a new iterator with a specific implementation type. + * \tparam C Becomes the iterator's implementation type. + * \param c Initial value for the iterator. The value is copied. + * \return + */ + template static ConstForwardIterator fromImpl(I i) { return ConstForwardIterator(new Pimpl(std::move(i))); } + /*! * \brief Returns a reference to the object pointed to. * \return @@ -183,6 +178,8 @@ namespace BlackMisc typedef QScopedPointer PimplPtr; PimplPtr m_pimpl; + explicit ConstForwardIterator(PimplBase *pimpl) : m_pimpl(pimpl) {} // private ctor used by fromImpl() + // using these methods to access m_pimpl.data() eases the cognitive burden of correctly forwarding const PimplBase *pimpl() { return m_pimpl.data(); } const PimplBase *pimpl() const { return m_pimpl.data(); } @@ -211,12 +208,6 @@ namespace BlackMisc //! \brief Default constructor. ConstBidirectionalIterator() {} - /*! - * \brief Constructor. - * \param i - */ - template ConstBidirectionalIterator(I i) : m_pimpl(new Pimpl(std::move(i))) {} - /*! * \brief Copy constructor. * \param other @@ -229,13 +220,6 @@ namespace BlackMisc */ ConstBidirectionalIterator(ConstBidirectionalIterator &&other) : m_pimpl(other.m_pimpl.take()) {} - /*! - * \brief Assignment. - * \param i - * \return - */ - template ConstBidirectionalIterator &operator =(I i) { m_pimpl.reset(new Pimpl(std::move(i))); return *this; } - /*! * \brief Copy assignment. * \param other @@ -250,6 +234,14 @@ namespace BlackMisc */ ConstBidirectionalIterator &operator =(ConstBidirectionalIterator &&other) { m_pimpl.reset(other.m_pimpl.take()); return *this; } + /*! + * \brief Create a new iterator with a specific implementation type. + * \tparam C Becomes the iterator's implementation type. + * \param c Initial value for the iterator. The value is copied. + * \return + */ + template static ConstBidirectionalIterator fromImpl(I i) { return ConstBidirectionalIterator(new Pimpl(std::move(i))); } + /*! * \brief Returns a reference to the object pointed to. * \return @@ -404,6 +396,8 @@ namespace BlackMisc typedef QScopedPointer PimplPtr; PimplPtr m_pimpl; + explicit ConstBidirectionalIterator(PimplBase *pimpl) : m_pimpl(pimpl) {} // private ctor used by fromImpl() + // using these methods to access m_pimpl.data() eases the cognitive burden of correctly forwarding const PimplBase *pimpl() { return m_pimpl.data(); } const PimplBase *pimpl() const { return m_pimpl.data(); } @@ -432,12 +426,6 @@ namespace BlackMisc //! \brief Default constructor. BidirectionalIterator() {} - /*! - * \brief Constructor. - * \param i - */ - template BidirectionalIterator(I i) : m_pimpl(new Pimpl(std::move(i))) {} - /*! * \brief Copy constructor. * \param other @@ -450,13 +438,6 @@ namespace BlackMisc */ BidirectionalIterator(BidirectionalIterator &&other) : m_pimpl(other.m_pimpl.take()) {} - /*! - * \brief Assignment. - * \param i - * \return - */ - template BidirectionalIterator &operator =(I i) { m_pimpl.reset(new Pimpl(std::move(i))); return *this; } - /*! * \brief Copy assignment. * \param other @@ -471,6 +452,14 @@ namespace BlackMisc */ BidirectionalIterator &operator =(BidirectionalIterator &&other) { m_pimpl.reset(other.m_pimpl.take()); return *this; } + /*! + * \brief Create a new iterator with a specific implementation type. + * \tparam C Becomes the iterator's implementation type. + * \param c Initial value for the iterator. The value is copied. + * \return + */ + template static BidirectionalIterator fromImpl(I i) { return BidirectionalIterator(new Pimpl(std::move(i))); } + /*! * \brief Returns a reference to the object pointed to. * \return @@ -641,6 +630,8 @@ namespace BlackMisc typedef QScopedPointer PimplPtr; PimplPtr m_pimpl; + explicit BidirectionalIterator(PimplBase *pimpl) : m_pimpl(pimpl) {} // private ctor used by fromImpl() + // using these methods to access m_pimpl.data() eases the cognitive burden of correctly forwarding const PimplBase *pimpl() { return m_pimpl.data(); } const PimplBase *pimpl() const { return m_pimpl.data(); } diff --git a/src/blackmisc/sequence.h b/src/blackmisc/sequence.h index 2fd7ca62d..af9de197a 100644 --- a/src/blackmisc/sequence.h +++ b/src/blackmisc/sequence.h @@ -474,12 +474,12 @@ namespace BlackMisc Pimpl(C &&c) : m_impl(std::move(c)) {} PimplBase *clone() const { return new Pimpl(*this); } PimplBase *cloneEmpty() const { return new Pimpl(C()); } - iterator begin() { return m_impl.begin(); } - const_iterator begin() const { return m_impl.cbegin(); } - const_iterator cbegin() const { return m_impl.cbegin(); } - iterator end() { return m_impl.end(); } - const_iterator end() const { return m_impl.cend(); } - const_iterator cend() const { return m_impl.cend(); } + iterator begin() { return iterator::fromImpl(m_impl.begin()); } + const_iterator begin() const { return const_iterator::fromImpl(m_impl.cbegin()); } + const_iterator cbegin() const { return const_iterator::fromImpl(m_impl.cbegin()); } + iterator end() { return iterator::fromImpl(m_impl.end()); } + const_iterator end() const { return const_iterator::fromImpl(m_impl.cend()); } + const_iterator cend() const { return const_iterator::fromImpl(m_impl.cend()); } reference operator [](size_type index) { return m_impl[index]; } const_reference operator [](size_type index) const { return m_impl[index]; } reference front() { return m_impl.front(); } @@ -489,11 +489,11 @@ namespace BlackMisc size_type size() const { return m_impl.size(); } bool empty() const { return m_impl.empty(); } void clear() { m_impl.clear(); } - iterator insert(iterator pos, const T &value) { return m_impl.insert(*static_cast(pos.getImpl()), value); } + iterator insert(iterator pos, const T &value) { return iterator::fromImpl(m_impl.insert(*static_cast(pos.getImpl()), value)); } void push_back(const T &value) { m_impl.push_back(value); } void pop_back() { m_impl.pop_back(); } - iterator erase(iterator pos) { return m_impl.erase(*static_cast(pos.getImpl())); } - iterator erase(iterator it1, iterator it2) { return m_impl.erase(*static_cast(it1.getImpl(), it2.getImpl())); } + iterator erase(iterator pos) { return iterator::fromImpl(m_impl.erase(*static_cast(pos.getImpl()))); } + iterator erase(iterator it1, iterator it2) { return iterator::fromImpl(m_impl.erase(*static_cast(it1.getImpl()), *static_cast(it2.getImpl()))); } bool operator ==(const PimplBase &other) const { Pimpl copy = C(); for (auto i = other.cbegin(); i != other.cend(); ++i) copy.push_back(*i); return m_impl == copy.m_impl; } private: C m_impl;