[PATCH] D47942: [SmallSet] Add SmallSetIterator.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: include/llvm/ADT/SmallSet.h:32
+template <typename T, unsigned N, typename C>
+class SmallSetIterator
+    : public iterator_facade_base<SmallSetIterator<T, N, C>,
----------------
I imagine this type needs user-defined special members (copy/move ctors and copy/move assignment operators) - the usual "rule of 5" (the user-defined dtor being an indicator here)


================
Comment at: unittests/ADT/SmallSetTest.cpp:77-78
+  for (int i = 0; i < 3; i++)
+    s1.insert(i);
+  {
+    std::vector<int> V(s1.begin(), s1.end());
----------------
I'd probably put a blank line between these two lines - can look a bit confusing otherwise (similarly below).

And/or maybe skip the explicit scopes here - could move the std::vector declaration up to the s1 declaration, and replace the iter+iter ctor with v.assign(iter, iter) instead.


https://reviews.llvm.org/D47942