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

[Tim Shen via llvm-commits]

timshen added a comment.

In https://reviews.llvm.org/D22951#514042, @dblaikie wrote:

> In https://reviews.llvm.org/D22951#501451, @chandlerc wrote:
>
> > One high level question that I'd love others' thoughts on: should this be spelled "filter_iterator" and "make_filter_range" or "filtered_iterator" and "make_filtered_range". For a type, I personally prefer "filtered_iterator". For the factory function, I wonder if we want something more like "reverse", perhaps "filter"?
>
>
> I'll vote for "filter" here - at least the version that takes an existing range (& perhaps that's the only version we should have - we already have utilities for making ranges out of a pair of iterators, so users with 'legacy' iterator APIs can use that to construct a range to pass to filter).
>
> As for the name of the iterator - I'd be OK making that an unmentionable implementation detail. Most of the time its type is going to include a lambda type anyway, right - and thus be unmentionable.


I don't have strong opinion on this. Changed to filter_iterator and make_filter_range.


================
Comment at: unittests/Support/IteratorTest.cpp:161-163
@@ +160,5 @@
+      IsOdd);
+  ASSERT_EQ(3, std::distance(Range.begin(), Range.end()));
+  SmallVector<int, 3> Actual(Range.begin(), Range.end());
+  EXPECT_EQ((SmallVector<int, 3>{1, 3, 5}), Actual);
+}
----------------
dblaikie wrote:
> The distance tests probably aren't necessary if you're testing for exact equality anyway?
Right. Removed.


https://reviews.llvm.org/D22951