[PATCH] D28999: Allow DenseSet::iterators to be conveted to and compared with const_iterator

[David Blaikie via llvm-commits]

On Sun, Jan 22, 2017 at 7:23 PM Alexis Shaw via Phabricator <
reviews at reviews.llvm.org> wrote:

> varno created this revision.
>
> This seemed to be an oversight seeing as DenseMap has these conversions.
>
>
> https://reviews.llvm.org/D28999
>
> Files:
>   include/llvm/ADT/DenseSet.h
>
>
> Index: include/llvm/ADT/DenseSet.h
> ===================================================================
> --- include/llvm/ADT/DenseSet.h
> +++ include/llvm/ADT/DenseSet.h
> @@ -90,17 +90,21 @@
>
>    // Iterators.
>
> +  class ConstIterator;
> +
>    class Iterator {
>      typename MapTy::iterator I;
>      friend class DenseSetImpl;
> +    friend class ConstIterator;
>
>    public:
>      typedef typename MapTy::iterator::difference_type difference_type;
>      typedef ValueT value_type;
>      typedef value_type *pointer;
>      typedef value_type &reference;
>      typedef std::forward_iterator_tag iterator_category;
>
> +    Iterator() {};
>

What necessitated this ^ change?

Also, use = default, rather than {}. (but if you had kept the {}, there's
an unnecessary semicolon after this ctor and the one in ConstIterator that
would need to be removed)


>      Iterator(const typename MapTy::iterator &i) : I(i) {}
>
>      ValueT &operator*() { return I->getFirst(); }
> @@ -112,19 +116,27 @@
>      Iterator operator++(int) { auto T = *this; ++I; return T; }
>      bool operator==(const Iterator& X) const { return I == X.I; }
>      bool operator!=(const Iterator& X) const { return I != X.I; }
> +
> +    bool operator==(const ConstIterator& X) const { return I == X.I; }
> +    bool operator!=(const ConstIterator& X) const { return I == X.I; }
>

Do you need these? The op==/!= for ConstIterator should be non-members,
then you probably won't need these (because both operands can be converted
to ConstIterator, and compared there).

This is why it's generally advisable to make any op overload that can be a
non-member should be a non-member.


>    };
>
>    class ConstIterator {
>      typename MapTy::const_iterator I;
>      friend class DenseSet;
> +    friend class Iterator;
>
>    public:
>      typedef typename MapTy::const_iterator::difference_type
> difference_type;
>      typedef ValueT value_type;
>      typedef value_type *pointer;
>      typedef value_type &reference;
>      typedef std::forward_iterator_tag iterator_category;
>
> +    ConstIterator(const Iterator &B) : I(B.I) {}
> +
> +    ConstIterator() {};
> +
>      ConstIterator(const typename MapTy::const_iterator &i) : I(i) {}
>
>      const ValueT &operator*() const { return I->getFirst(); }
> @@ -134,6 +146,9 @@
>      ConstIterator operator++(int) { auto T = *this; ++I; return T; }
>      bool operator==(const ConstIterator& X) const { return I == X.I; }
>      bool operator!=(const ConstIterator& X) const { return I != X.I; }
> +
> +    bool operator==(const Iterator& X) const { return I == X.I; }
> +    bool operator!=(const Iterator& X) const { return I != X.I; }
>    };
>
>    typedef Iterator      iterator;
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170123/e2cf8429/attachment.html>