Make value_type a non-const StringMapEntry<ValueTy>.

Author: kazutakahirata

llvm/include/llvm/ADT/StringMap.h

@@ -432,17 +432,15 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
 };
 
 template <typename ValueTy, bool IsConst> class StringMapIterBase {
-  using EntryTy = std::conditional_t<IsConst, const StringMapEntry<ValueTy>,
-                                     StringMapEntry<ValueTy>>;
-
   StringMapEntryBase **Ptr = nullptr;
 
 public:
   using iterator_category = std::forward_iterator_tag;
-  using value_type = EntryTy;
+  using value_type = StringMapEntry<ValueTy>;
   using difference_type = std::ptrdiff_t;
-  using pointer = value_type *;
-  using reference = value_type &;
+  using pointer = std::conditional_t<IsConst, const value_type *, value_type *>;
+  using reference =
+      std::conditional_t<IsConst, const value_type &, value_type &>;
 
   StringMapIterBase() = default;
 
@@ -453,8 +451,8 @@ template <typename ValueTy, bool IsConst> class StringMapIterBase {
       AdvancePastEmptyBuckets();
   }
 
-  reference operator*() const { return *static_cast<EntryTy *>(*Ptr); }
-  pointer operator->() const { return &operator*(); }
+  reference operator*() const { return *static_cast<value_type *>(*Ptr); }
+  pointer operator->() const { return static_cast<value_type *>(*Ptr); }
 
   StringMapIterBase &operator++() { // Preincrement
     ++Ptr;

llvm/unittests/ADT/StringMapTest.cpp

@@ -701,6 +701,19 @@ TEST(StringMapCustomTest, NonConstIterator) {
   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>);
@@ -718,6 +731,19 @@ TEST(StringMapCustomTest, ConstIterator) {
   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>);