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

[David Blaikie via llvm-commits]

dblaikie added a comment.

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.


================
Comment at: include/llvm/ADT/STLExtras.h:305
@@ +304,3 @@
+  friend iterator_range<filtered_iterator<WT, PT>>
+      make_filter_range(iterator_range<WT>, PT);
+};
----------------
If you like you can just define the friend inline at the friend declaration:

  friend void foo() {
    ...
  }

================
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);
+}
----------------
The distance tests probably aren't necessary if you're testing for exact equality anyway?


https://reviews.llvm.org/D22951