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

[David Blaikie via llvm-commits]

On Mon, Jan 23, 2017 at 8:37 PM Dean Michael Berris <dberris at google.com>
wrote:

> Right -- I can go ahead and add those tests
>

Thanks


> (I think we've also run into the problem of Alexis not seeing the comments
> that are in email only)
>

Ah - right. Should get that sorted - lots of stuff only happens in email &
Phab's not perfect at reflecting email discussions. (maybe it can cope with
simple top-posts? I'm not rightly sure)


> .
>
> 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/6b94a682/attachment.html>