[PATCH] D22951: [ADT] Add filter_iterator for filtering elements

Tim Shen via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 29 11:45:06 PDT 2016


timshen marked 5 inline comments as done.

================
Comment at: include/llvm/ADT/STLExtras.h:274-277
@@ +273,6 @@
+          std::input_iterator_tag, std::forward_iterator_tag>::type,
+      typename std::iterator_traits<WrappedIteratorT>::value_type,
+      typename std::iterator_traits<WrappedIteratorT>::difference_type,
+      typename std::iterator_traits<WrappedIteratorT>::pointer,
+      typename std::iterator_traits<WrappedIteratorT>::reference>;
+
----------------
chandlerc wrote:
> You don't need these four lines. The adaptor base will do this for you.
The adaptor will do PointerT = T* and ReferenceT = T& for me, but I want to forward WrappedIteratorT's pointer and reference type in filter_iterator (because the filtered elements have the same type as the original ones).

================
Comment at: unittests/Support/IteratorTest.cpp:104
@@ +103,3 @@
+  int a[] = {0, 1, 2, 3, 4, 5, 6};
+  const auto Range = make_filter_range(make_range(a, a + 7), IsOdd);
+  ASSERT_EQ(3, std::distance(Range.begin(), Range.end()));
----------------
chandlerc wrote:
> Please test other forms of callables. std::function, function pointer at least.
> 
> Also test an input iterator and combining filter with some of our other adaptors like a filtered pointee iterator.
During adding the tests I do find a iterator_adaptor_base constructor problem. Changed the constructor to take WrappedIteratorT only. All users seem to still work.


https://reviews.llvm.org/D22951





More information about the llvm-commits mailing list