[PATCH] D31171: Improve StringMap iterator support.

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Some optional bits/pieces. (also - could include test coverage for op->. Or perhaps better/alternatively: put a static_assert in iterator_adapter_base to ensure that the return type of op* is a reference type?)



================
Comment at: llvm/unittests/ADT/StringMapTest.cpp:287-300
+TEST_F(StringMapTest, IterSetKeys) {
+  StringSet<> Set;
+  Set.insert("A");
+  Set.insert("B");
+  Set.insert("C");
+  Set.insert("D");
+
----------------
Not sure this test is necessary (I mean, depending on your perspective) - since it's testing the same API in StringMap that's inherited by StringSet - there's no new/interesting code under test here, compared to the previous test.

(should StringSet's iterators actually be key iterators, rather than a 'keys()' operation on a set, which doesn't quite make sense - it only has keys, no values, so why would there be two different ranges to iterate over?)


https://reviews.llvm.org/D31171