This is an archive of the discontinued LLVM Phabricator instance.

[ADT] Fix zip iterator interface.
ClosedPublic

Authored by bryant on Feb 22 2017, 1:33 AM.

Details

Summary

This patch:

  • Ensures that zip_first/zip_shortest inherit from the iterator_facade_base (which it should);
  • marks zippy::begin/::end const;
  • and ensures that zip plays well with make_filter_range.

Diff Detail

Repository
rL LLVM

Event Timeline

bryant created this revision.Feb 22 2017, 1:33 AM
dberlin edited edge metadata.Feb 22 2017, 5:38 AM

Thanks for doing this, i'll give it a shot and report back :)

include/llvm/ADT/STLExtras.h
363 ↗(On Diff #89336)

This is not correct, AFAIK:
The difference type is actually the difference type of the first iterator template argument:

typedef typename std::tuple_element<0, std::tuple<Iters...> >::type temp;
typedef typename std::iterator_traits<temp>::difference_type difference_type;

(see what boost does, which is similar.
It's possible to do this without the tuple trick, but it requires similar meta-programming)

dberlin accepted this revision.Feb 23 2017, 12:21 PM

If you change it to:
diff --git a/include/llvm/ADT/STLExtras.h b/include/llvm/ADT/STLExtras.h
index 44bf84d1d1e..a1e2b072528 100644

  • a/include/llvm/ADT/STLExtras.h

+++ b/include/llvm/ADT/STLExtras.h
@@ -360,7 +360,8 @@ template <typename ZipType, typename... Iters>
using zip_traits =

iterator_facade_base<ZipType, std::input_iterator_tag,
                     std::tuple<decltype(*std::declval<Iters>())...>,
  • std::ptrdiff_t,

+ typename std::iterator_traits<typename std::tuple_element<
+ 0, std::tuple<Iters...>>::type>::difference_type,

std::tuple<decltype(*std::declval<Iters>())...> *,
std::tuple<decltype(*std::declval<Iters>())...>>;

I think this would be ready to go in :)

This revision is now accepted and ready to land.Feb 23 2017, 12:21 PM
bryant updated this revision to Diff 89563.Feb 23 2017, 2:11 PM
  • Use boost's difference_type calculation, even though it's not morally correct.
  • Support reverse iteration, if all the inner iterators support it.
bryant marked an inline comment as done.Feb 23 2017, 2:16 PM
bryant updated this revision to Diff 89565.Feb 23 2017, 2:20 PM
  • Allow operator-- when inner iterators are random access.
bryant updated this revision to Diff 89570.Feb 23 2017, 2:45 PM

Test filter with reversed zip.

This revision was automatically updated to reflect the committed changes.