[llvm] [ADT] Simplify SmallSetIterator with std::variant (NFC) (PR #157229)

via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 5 22:45:14 PDT 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

<details>
<summary>Changes</summary>

SmallSet supports two underlying data structures -- SmallVector and
std::set.  SmallSetIterator supports them by maintaining a flag
IsSmall and a union of the two underlying iterators.

This patch essentially packages the flag and the union into
std::variant, eliminating a lot of code involving IsSmall.

With this change, we drop from the Rule of Five all the way down to
Rule of Zero.


---
Full diff: https://github.com/llvm/llvm-project/pull/157229.diff


1 Files Affected:

- (modified) llvm/include/llvm/ADT/SmallSet.h (+10-76) 


``````````diff
diff --git a/llvm/include/llvm/ADT/SmallSet.h b/llvm/include/llvm/ADT/SmallSet.h
index 0e90293352630..11d1d25ccb932 100644
--- a/llvm/include/llvm/ADT/SmallSet.h
+++ b/llvm/include/llvm/ADT/SmallSet.h
@@ -24,6 +24,7 @@
 #include <initializer_list>
 #include <set>
 #include <utility>
+#include <variant>
 
 namespace llvm {
 
@@ -37,92 +38,25 @@ class SmallSetIterator
   using SetIterTy = typename std::set<T, C>::const_iterator;
   using VecIterTy = typename SmallVector<T, N>::const_iterator;
 
-  /// Iterators to the parts of the SmallSet containing the data. They are set
-  /// depending on isSmall.
-  union {
-    SetIterTy SetIter;
-    VecIterTy VecIter;
-  };
-
-  bool IsSmall;
+  // A variant over the two possible iterators.
+  std::variant<VecIterTy, SetIterTy> Iter;
 
 public:
-  SmallSetIterator(SetIterTy SetIter) : SetIter(SetIter), IsSmall(false) {}
-
-  SmallSetIterator(VecIterTy VecIter) : VecIter(VecIter), IsSmall(true) {}
-
-  // Spell out destructor, copy/move constructor and assignment operators for
-  // MSVC STL, where set<T>::const_iterator is not trivially copy constructible.
-  ~SmallSetIterator() {
-    if (IsSmall)
-      VecIter.~VecIterTy();
-    else
-      SetIter.~SetIterTy();
-  }
-
-  SmallSetIterator(const SmallSetIterator &Other) : IsSmall(Other.IsSmall) {
-    if (IsSmall)
-      VecIter = Other.VecIter;
-    else
-      // Use placement new, to make sure SetIter is properly constructed, even
-      // if it is not trivially copy-able (e.g. in MSVC).
-      new (&SetIter) SetIterTy(Other.SetIter);
-  }
-
-  SmallSetIterator(SmallSetIterator &&Other) : IsSmall(Other.IsSmall) {
-    if (IsSmall)
-      VecIter = std::move(Other.VecIter);
-    else
-      // Use placement new, to make sure SetIter is properly constructed, even
-      // if it is not trivially copy-able (e.g. in MSVC).
-      new (&SetIter) SetIterTy(std::move(Other.SetIter));
-  }
-
-  SmallSetIterator& operator=(const SmallSetIterator& Other) {
-    // Call destructor for SetIter, so it gets properly destroyed if it is
-    // not trivially destructible in case we are setting VecIter.
-    if (!IsSmall)
-      SetIter.~SetIterTy();
-
-    IsSmall = Other.IsSmall;
-    if (IsSmall)
-      VecIter = Other.VecIter;
-    else
-      new (&SetIter) SetIterTy(Other.SetIter);
-    return *this;
-  }
-
-  SmallSetIterator& operator=(SmallSetIterator&& Other) {
-    // Call destructor for SetIter, so it gets properly destroyed if it is
-    // not trivially destructible in case we are setting VecIter.
-    if (!IsSmall)
-      SetIter.~SetIterTy();
-
-    IsSmall = Other.IsSmall;
-    if (IsSmall)
-      VecIter = std::move(Other.VecIter);
-    else
-      new (&SetIter) SetIterTy(std::move(Other.SetIter));
-    return *this;
-  }
+  SmallSetIterator(VecIterTy VecIter) : Iter(VecIter) {}
+  SmallSetIterator(SetIterTy SetIter) : Iter(SetIter) {}
 
   bool operator==(const SmallSetIterator &RHS) const {
-    if (IsSmall != RHS.IsSmall)
-      return false;
-    if (IsSmall)
-      return VecIter == RHS.VecIter;
-    return SetIter == RHS.SetIter;
+    return Iter == RHS.Iter;
   }
 
   SmallSetIterator &operator++() { // Preincrement
-    if (IsSmall)
-      ++VecIter;
-    else
-      ++SetIter;
+    std::visit([](auto &It) { ++It; }, Iter);
     return *this;
   }
 
-  const T &operator*() const { return IsSmall ? *VecIter : *SetIter; }
+  const T &operator*() const {
+    return std::visit([](const auto &It) -> const T & { return *It; }, Iter);
+  }
 };
 
 /// SmallSet - This maintains a set of unique values, optimizing for the case

``````````

</details>


https://github.com/llvm/llvm-project/pull/157229


More information about the llvm-commits mailing list