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

[Dean Michael Berris via llvm-commits]

Right -- I can go ahead and add those tests (I think we've also run into
the problem of Alexis not seeing the comments that are in email only).

On Tue, Jan 24, 2017 at 3:35 PM David Blaikie <dblaikie at gmail.com> wrote:

> Looks like this was missing test coverage for the default ctors.
>
> (also, think it's probably best to implement those op==/op!= as
> non-members - rather than the odd asymmetry of members with a ConstIterator
> parameter for the non-const Iterator operators, etc - you can still
> implement them in the class if that's more convenient by using an inline
> friend definition: friend bool operator==(const foo& a, const foo& b) {
> return a.x == b.x; } or similar)
>
> On Mon, Jan 23, 2017 at 8:22 PM Dean Michael Berris via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL292879: Allow DenseSet::iterators to be conveted to and
> compared with const_iterator (authored by dberris).
>
> Changed prior to commit:
>   https://reviews.llvm.org/D28999?vs=85323&id=85522#toc
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D28999
>
> Files:
>   llvm/trunk/include/llvm/ADT/DenseSet.h
>   llvm/trunk/unittests/ADT/DenseSetTest.cpp
>
>
> Index: llvm/trunk/unittests/ADT/DenseSetTest.cpp
> ===================================================================
> --- llvm/trunk/unittests/ADT/DenseSetTest.cpp
> +++ llvm/trunk/unittests/ADT/DenseSetTest.cpp
> @@ -73,6 +73,15 @@
>    EXPECT_EQ(0u, set.count(3));
>  }
>
> +TYPED_TEST(DenseSetTest, ConstIteratorComparison){
> +  TypeParam set({1});
> +  const TypeParam &cset = set;
> +  EXPECT_EQ(set.begin(), cset.begin());
> +  EXPECT_EQ(set.end(), cset.end());
> +  EXPECT_NE(set.end(), cset.begin());
> +  EXPECT_NE(set.begin(), cset.end());
> +}
> +
>  TYPED_TEST(DenseSetTest, EmptyInitializerList) {
>    TypeParam set({});
>    EXPECT_EQ(0u, set.size());
> Index: llvm/trunk/include/llvm/ADT/DenseSet.h
> ===================================================================
> --- llvm/trunk/include/llvm/ADT/DenseSet.h
> +++ llvm/trunk/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() = default;
>      Iterator(const typename MapTy::iterator &i) : I(i) {}
>
>      ValueT &operator*() { return I->getFirst(); }
> @@ -110,21 +114,26 @@
>
>      Iterator& operator++() { ++I; return *this; }
>      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; }
>    };
>
>    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() = default;
> +
>      ConstIterator(const typename MapTy::const_iterator &i) : I(i) {}
>
>      const ValueT &operator*() const { return I->getFirst(); }
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170124/28147b30/attachment.html>