[ADT] Refactor StringMap iterators (NFC) (#156137)

StringMap has four iterator classes:

- StringMapIterBase
- StringMapIterator
- StringMapConstIterator
- StringMapKeyIterator

This patch consolidates the first three into one class, namely
StringMapIterBase, adds a boolean template parameter to indicate
desired constness, and then use "using" directives to specialize the
common class:

  using const_iterator = StringMapIterBase<ValueTy, true>;
  using iterator = StringMapIterBase<ValueTy, false>;

just like how we simplified DenseMapIterator.

Remarks:

- This patch drops CRTP and iterator_facade_base for simplicity.  For
  fairly simple forward iterators, iterator_facade_base doesn't buy us
  much.  We just have to write a few "using" directives and operator!=
  manually.

- StringMapIterBase has a SFINAE-based constructor to construct a
  const iterator from a non-const one just like DenseMapIterator.

- We now rely on compiler-generated copy and assignment operators.

1 files changed, 54 insertions, 0 deletions

diff --git a/llvm/unittests/ADT/StringMapTest.cpp b/llvm/unittests/ADT/StringMapTest.cpp
index 35135cd..92ae364 100644
--- a/llvm/unittests/ADT/StringMapTest.cpp
+++ b/llvm/unittests/ADT/StringMapTest.cpp
@@ -693,4 +693,58 @@ TEST(StringMapCustomTest, StringMapEntrySize) {
   EXPECT_EQ(LargeValue, Key.size());
 }
 
+TEST(StringMapCustomTest, NonConstIterator) {
+  StringMap<int> Map;
+  Map["key"] = 1;
+
+  // Check that Map.begin() returns a non-const iterator.
+  static_assert(
+      std::is_same_v<decltype(Map.begin()), StringMap<int>::iterator>);
+
+  // Check that the value_type of a non-const iterator is not a const type.
+  static_assert(
+      !std::is_const_v<StringMap<int>::iterator::value_type>,
+      "The value_type of a non-const iterator should not be a const type.");
+
+  // Check that pointer and reference types are not const.
+  static_assert(std::is_same_v<StringMap<int>::iterator::pointer,
+                               StringMap<int>::iterator::value_type *>);
+  static_assert(std::is_same_v<StringMap<int>::iterator::reference,
+                               StringMap<int>::iterator::value_type &>);
+
+  // Check that we can construct a const_iterator from an iterator.
+  static_assert(std::is_constructible_v<StringMap<int>::const_iterator,
+                                        StringMap<int>::iterator>);
+
+  // Double check that we can actually construct a const_iterator.
+  StringMap<int>::const_iterator const_it = Map.begin();
+  (void)const_it;
+}
+
+TEST(StringMapCustomTest, ConstIterator) {
+  StringMap<int> Map;
+  const StringMap<int> &ConstMap = Map;
+
+  // Check that ConstMap.begin() returns a const_iterator.
+  static_assert(std::is_same_v<decltype(ConstMap.begin()),
+                               StringMap<int>::const_iterator>);
+
+  // Check that the value_type of a const iterator is not a const type.
+  static_assert(
+      !std::is_const_v<StringMap<int>::const_iterator::value_type>,
+      "The value_type of a const iterator should not be a const type.");
+
+  // Check that pointer and reference types are const.
+  static_assert(
+      std::is_same_v<StringMap<int>::const_iterator::pointer,
+                     const StringMap<int>::const_iterator::value_type *>);
+  static_assert(
+      std::is_same_v<StringMap<int>::const_iterator::reference,
+                     const StringMap<int>::const_iterator::value_type &>);
+
+  // Check that we cannot construct an iterator from a const_iterator.
+  static_assert(!std::is_constructible_v<StringMap<int>::iterator,
+                                         StringMap<int>::const_iterator>);
+}
+
 } // end anonymous namespace