[llvm] r298440 - Revert "Improve StringMap iterator support."

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 21 14:23:57 PDT 2017


Author: zturner
Date: Tue Mar 21 16:23:57 2017
New Revision: 298440

URL: http://llvm.org/viewvc/llvm-project?rev=298440&view=rev
Log:
Revert "Improve StringMap iterator support."

This is causing crashes in clang, so reverting until the problem
is figured out.

Modified:
    llvm/trunk/include/llvm/ADT/STLExtras.h
    llvm/trunk/include/llvm/ADT/StringMap.h
    llvm/trunk/unittests/ADT/StringMapTest.cpp

Modified: llvm/trunk/include/llvm/ADT/STLExtras.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/STLExtras.h?rev=298440&r1=298439&r2=298440&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/STLExtras.h (original)
+++ llvm/trunk/include/llvm/ADT/STLExtras.h Tue Mar 21 16:23:57 2017
@@ -893,8 +893,7 @@ auto partition(R &&Range, UnaryPredicate
 /// SmallVector with elements of the vector.  This is useful, for example,
 /// when you want to iterate a range and then sort the results.
 template <unsigned Size, typename R>
-SmallVector<typename std::remove_const<detail::ValueOfRange<R>>::type, Size>
-to_vector(R &&Range) {
+SmallVector<detail::ValueOfRange<R>, Size> to_vector(R &&Range) {
   return {std::begin(Range), std::end(Range)};
 }
 

Modified: llvm/trunk/include/llvm/ADT/StringMap.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringMap.h?rev=298440&r1=298439&r2=298440&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/StringMap.h (original)
+++ llvm/trunk/include/llvm/ADT/StringMap.h Tue Mar 21 16:23:57 2017
@@ -15,13 +15,13 @@
 #define LLVM_ADT_STRINGMAP_H
 
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/iterator.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/PointerLikeTypeTraits.h"
 #include <cassert>
 #include <cstdint>
 #include <cstdlib>
 #include <cstring>
+#include <utility>
 #include <initializer_list>
 #include <new>
 #include <utility>
@@ -32,7 +32,6 @@ namespace llvm {
   class StringMapConstIterator;
   template<typename ValueT>
   class StringMapIterator;
-  template <typename ValueT> class StringMapKeyIterator;
   template<typename ValueTy>
   class StringMapEntry;
 
@@ -313,11 +312,6 @@ public:
     return const_iterator(TheTable+NumBuckets, true);
   }
 
-  llvm::iterator_range<StringMapKeyIterator<ValueTy>> keys() const {
-    return make_range(StringMapKeyIterator<ValueTy>(begin()),
-                      StringMapKeyIterator<ValueTy>(end()));
-  }
-
   iterator find(StringRef Key) {
     int Bucket = FindKey(Key);
     if (Bucket == -1) return end();
@@ -450,39 +444,42 @@ public:
   }
 };
 
-template <typename DerivedTy, typename ValueTy>
-class StringMapIterBase
-    : public iterator_facade_base<DerivedTy, std::forward_iterator_tag,
-                                  ValueTy> {
+template <typename ValueTy> class StringMapConstIterator {
 protected:
   StringMapEntryBase **Ptr = nullptr;
 
 public:
-  StringMapIterBase() = default;
+  typedef StringMapEntry<ValueTy> value_type;
 
-  explicit StringMapIterBase(StringMapEntryBase **Bucket,
-                             bool NoAdvance = false)
-      : Ptr(Bucket) {
+  StringMapConstIterator() = default;
+
+  explicit StringMapConstIterator(StringMapEntryBase **Bucket,
+                                  bool NoAdvance = false)
+  : Ptr(Bucket) {
     if (!NoAdvance) AdvancePastEmptyBuckets();
   }
 
-  DerivedTy &operator=(const DerivedTy &Other) {
-    Ptr = Other.Ptr;
-    return static_cast<DerivedTy &>(*this);
+  const value_type &operator*() const {
+    return *static_cast<StringMapEntry<ValueTy>*>(*Ptr);
+  }
+  const value_type *operator->() const {
+    return static_cast<StringMapEntry<ValueTy>*>(*Ptr);
   }
 
-  bool operator==(const DerivedTy &RHS) const { return Ptr == RHS.Ptr; }
+  bool operator==(const StringMapConstIterator &RHS) const {
+    return Ptr == RHS.Ptr;
+  }
+  bool operator!=(const StringMapConstIterator &RHS) const {
+    return Ptr != RHS.Ptr;
+  }
 
-  DerivedTy &operator++() { // Preincrement
+  inline StringMapConstIterator& operator++() {   // Preincrement
     ++Ptr;
     AdvancePastEmptyBuckets();
-    return static_cast<DerivedTy &>(*this);
+    return *this;
   }
-
-  DerivedTy operator++(int) { // Post-increment
-    DerivedTy Tmp(Ptr);
-    ++*this;
-    return Tmp;
+  StringMapConstIterator operator++(int) {        // Postincrement
+    StringMapConstIterator tmp = *this; ++*this; return tmp;
   }
 
 private:
@@ -492,67 +489,22 @@ private:
   }
 };
 
-template <typename ValueTy>
-class StringMapConstIterator
-    : public StringMapIterBase<StringMapConstIterator<ValueTy>,
-                               const StringMapEntry<ValueTy>> {
-  using base = StringMapIterBase<StringMapConstIterator<ValueTy>,
-                                 const StringMapEntry<ValueTy>>;
-
-public:
-  StringMapConstIterator() = default;
-  explicit StringMapConstIterator(StringMapEntryBase **Bucket,
-                                  bool NoAdvance = false)
-      : base(Bucket, NoAdvance) {}
-
-  const StringMapEntry<ValueTy> &operator*() const {
-    return *static_cast<const StringMapEntry<ValueTy> *>(*this->Ptr);
-  }
-};
-
-template <typename ValueTy>
-class StringMapIterator : public StringMapIterBase<StringMapIterator<ValueTy>,
-                                                   StringMapEntry<ValueTy>> {
-  using base =
-      StringMapIterBase<StringMapIterator<ValueTy>, StringMapEntry<ValueTy>>;
-
+template<typename ValueTy>
+class StringMapIterator : public StringMapConstIterator<ValueTy> {
 public:
   StringMapIterator() = default;
+
   explicit StringMapIterator(StringMapEntryBase **Bucket,
                              bool NoAdvance = false)
-      : base(Bucket, NoAdvance) {}
-
-  StringMapEntry<ValueTy> &operator*() const {
-    return *static_cast<StringMapEntry<ValueTy> *>(*this->Ptr);
+    : StringMapConstIterator<ValueTy>(Bucket, NoAdvance) {
   }
 
-  operator StringMapConstIterator<ValueTy>() const {
-    return StringMapConstIterator<ValueTy>(this->Ptr, false);
+  StringMapEntry<ValueTy> &operator*() const {
+    return *static_cast<StringMapEntry<ValueTy>*>(*this->Ptr);
   }
-};
-
-template <typename ValueTy>
-class StringMapKeyIterator
-    : public iterator_adaptor_base<StringMapKeyIterator<ValueTy>,
-                                   StringMapConstIterator<ValueTy>,
-                                   std::forward_iterator_tag, StringRef> {
-  using base = iterator_adaptor_base<StringMapKeyIterator<ValueTy>,
-                                     StringMapConstIterator<ValueTy>,
-                                     std::forward_iterator_tag, StringRef>;
-
-public:
-  StringMapKeyIterator() = default;
-
-  explicit StringMapKeyIterator(StringMapConstIterator<ValueTy> Iter)
-      : base(std::move(Iter)) {}
-
-  StringRef &operator*() {
-    Key = this->wrapped()->getKey();
-    return Key;
+  StringMapEntry<ValueTy> *operator->() const {
+    return static_cast<StringMapEntry<ValueTy>*>(*this->Ptr);
   }
-
-private:
-  StringRef Key;
 };
 
 } // end namespace llvm

Modified: llvm/trunk/unittests/ADT/StringMapTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringMapTest.cpp?rev=298440&r1=298439&r2=298440&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/StringMapTest.cpp (original)
+++ llvm/trunk/unittests/ADT/StringMapTest.cpp Tue Mar 21 16:23:57 2017
@@ -8,7 +8,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/DataTypes.h"
 #include "gtest/gtest.h"
@@ -270,34 +269,6 @@ TEST_F(StringMapTest, InsertRehashingPai
   EXPECT_EQ(42u, It->second);
 }
 
-TEST_F(StringMapTest, IterMapKeys) {
-  StringMap<int> Map;
-  Map["A"] = 1;
-  Map["B"] = 2;
-  Map["C"] = 3;
-  Map["D"] = 3;
-
-  auto Keys = to_vector<4>(Map.keys());
-  std::sort(Keys.begin(), Keys.end());
-
-  SmallVector<StringRef, 4> Expected = {"A", "B", "C", "D"};
-  EXPECT_EQ(Expected, Keys);
-}
-
-TEST_F(StringMapTest, IterSetKeys) {
-  StringSet<> Set;
-  Set.insert("A");
-  Set.insert("B");
-  Set.insert("C");
-  Set.insert("D");
-
-  auto Keys = to_vector<4>(Set.keys());
-  std::sort(Keys.begin(), Keys.end());
-
-  SmallVector<StringRef, 4> Expected = {"A", "B", "C", "D"};
-  EXPECT_EQ(Expected, Keys);
-}
-
 // Create a non-default constructable value
 struct StringMapTestStruct {
   StringMapTestStruct(int i) : i(i) {}




More information about the llvm-commits mailing list