[PATCH] D48598: [ADT] drop_begin: use std namespace. NFC.

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 26 10:26:35 PDT 2018


I think we now have adl_begin/adl_end helpers in the llvm namespace that
should do the right thigh (which is that the call should be made like
"using std::begin; ...begin(X)..." The same way swap is meant to be called
(this allows a default implementation in the std namespace and allows for
types to implement their own custom implementation in their own namespace
to be found via ADL))

On Tue., 26 Jun. 2018, 10:19 am Michael Kruse via Phabricator, <
reviews at reviews.llvm.org> wrote:

> Meinersbur created this revision.
> Meinersbur added reviewers: dblaikie, grosbach, aaron.ballman, ruiu.
>
> The instantiation of the drop_begin function template usually fails
> because the functions begin() and end() do not exist. Only when using on a
> container from the std namespace (or `llvm::iterator_range`s of something
> derived from `std::iterator`), they are matched to std::begin() and
> std::end() due to Koenig-lookup.
>
> Explicitly use std::begin and std::end to make drop_begin applicable to
> anything iterable (including C-style arrays).
>
> A solution for general `llvm::iterator_range`s was already tried in
> r244620, but got reverted in r244621 due to MSVC not liking it.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D48598
>
> Files:
>   include/llvm/ADT/iterator_range.h
>   unittests/ADT/IteratorTest.cpp
>
>
> Index: unittests/ADT/IteratorTest.cpp
> ===================================================================
> --- unittests/ADT/IteratorTest.cpp
> +++ unittests/ADT/IteratorTest.cpp
> @@ -373,4 +373,17 @@
>    EXPECT_EQ(std::distance(v2.begin(), v2.end()), size(v2));
>  }
>
> +TEST(IteratorRangeTest, DropBegin) {
> +  SmallVector<int, 5> vec{0, 1, 2, 3, 4};
> +
> +  for (int n = 0; n < 5; ++n) {
> +    int i = n;
> +    for (auto &v : drop_begin(vec, n)) {
> +      EXPECT_EQ(v, i);
> +      i += 1;
> +    }
> +    EXPECT_EQ(i, 5);
> +  }
> +}
> +
>  } // anonymous namespace
> Index: include/llvm/ADT/iterator_range.h
> ===================================================================
> --- include/llvm/ADT/iterator_range.h
> +++ include/llvm/ADT/iterator_range.h
> @@ -59,9 +59,10 @@
>    return iterator_range<T>(std::move(p.first), std::move(p.second));
>  }
>
> -template<typename T>
> -iterator_range<decltype(begin(std::declval<T>()))> drop_begin(T &&t, int
> n) {
> -  return make_range(std::next(begin(t), n), end(t));
> +template <typename T>
> +iterator_range<decltype(std::begin(std::declval<T>()))> drop_begin(T &&t,
> +                                                                   int n)
> {
> +  return make_range(std::next(std::begin(t), n), std::end(t));
>  }
>  }
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180626/43bd9ae7/attachment.html>


More information about the llvm-commits mailing list