[PATCH] D131549: [ADT] Make SmallSet().insert(const T &) returns const_iterator

[Piggy via Phabricator via llvm-commits]

piggynl created this revision.
Herald added a project: All.
piggynl requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This will exactly match std::set's behavior and make deduplicating items with SmallSet easier.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131549

Files:
  llvm/include/llvm/ADT/SmallSet.h
  llvm/unittests/ADT/SmallSetTest.cpp


Index: llvm/unittests/ADT/SmallSetTest.cpp
===================================================================
--- llvm/unittests/ADT/SmallSetTest.cpp
+++ llvm/unittests/ADT/SmallSetTest.cpp
@@ -21,11 +21,17 @@
 
   SmallSet<int, 4> s1;
 
-  for (int i = 0; i < 4; i++)
-    s1.insert(i);
+  for (int i = 0; i < 4; i++) {
+    auto InsertResult = s1.insert(i);
+    EXPECT_EQ(*InsertResult.first, i);
+    EXPECT_EQ(InsertResult.second, true);
+  }
 
-  for (int i = 0; i < 4; i++)
-    s1.insert(i);
+  for (int i = 0; i < 4; i++) {
+    auto InsertResult = s1.insert(i);
+    EXPECT_EQ(*InsertResult.first, i);
+    EXPECT_EQ(InsertResult.second, false);
+  }
 
   EXPECT_EQ(4u, s1.size());
 
@@ -38,8 +44,17 @@
 TEST(SmallSetTest, Grow) {
   SmallSet<int, 4> s1;
 
-  for (int i = 0; i < 8; i++)
-    s1.insert(i);
+  for (int i = 0; i < 8; i++) {
+    auto InsertResult = s1.insert(i);
+    EXPECT_EQ(*InsertResult.first, i);
+    EXPECT_EQ(InsertResult.second, true);
+  }
+
+  for (int i = 0; i < 8; i++) {
+    auto InsertResult = s1.insert(i);
+    EXPECT_EQ(*InsertResult.first, i);
+    EXPECT_EQ(InsertResult.second, false);
+  }
 
   EXPECT_EQ(8u, s1.size());
 
Index: llvm/include/llvm/ADT/SmallSet.h
===================================================================
--- llvm/include/llvm/ADT/SmallSet.h
+++ llvm/include/llvm/ADT/SmallSet.h
@@ -141,6 +141,7 @@
   std::set<T, C> Set;
 
   using VIterator = typename SmallVector<T, N>::const_iterator;
+  using SIterator = typename std::set<T, C>::const_iterator;
   using mutable_iterator = typename SmallVector<T, N>::iterator;
 
   // In small mode SmallPtrSet uses linear search for the elements, so it is
@@ -177,18 +178,20 @@
   /// The first value of the returned pair is unused and provided for
   /// partial compatibility with the standard library self-associative container
   /// concept.
-  // FIXME: Add iterators that abstract over the small and large form, and then
-  // return those here.
-  std::pair<NoneType, bool> insert(const T &V) {
-    if (!isSmall())
-      return std::make_pair(None, Set.insert(V).second);
+  std::pair<const_iterator, bool> insert(const T &V) {
+    if (!isSmall()) {
+      SIterator SIt;
+      bool Inserted;
+      std::tie(SIt, Inserted) = Set.insert(V);
+      return std::make_pair(const_iterator(SIt), Inserted);
+    }
 
-    VIterator I = vfind(V);
-    if (I != Vector.end())    // Don't reinsert if it already exists.
-      return std::make_pair(None, false);
+    VIterator VIt = vfind(V);
+    if (VIt != Vector.end()) // Don't reinsert if it already exists.
+      return std::make_pair(const_iterator(VIt), false);
     if (Vector.size() < N) {
       Vector.push_back(V);
-      return std::make_pair(None, true);
+      return std::make_pair(const_iterator(std::prev(Vector.end())), true);
     }
 
     // Otherwise, grow from vector to set.
@@ -196,8 +199,7 @@
       Set.insert(Vector.back());
       Vector.pop_back();
     }
-    Set.insert(V);
-    return std::make_pair(None, true);
+    return std::make_pair(const_iterator(Set.insert(V).first), true);
   }
 
   template <typename IterT>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D131549.451375.patch
Type: text/x-patch
Size: 3127 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220810/20c66403/attachment.bin>