[llvm] [ADT] Simplify SmallPtrSetIterator with std::reverse_iterator (NFC) (PR #160643)

[Kazu Hirata via llvm-commits]

https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/160643

SmallPtrSetIterator has two tasks:

 - iterate the buckets in the requested direction
 - skip the empty and tombstone buckets

These tasks are intertwined in the current implementation. This patch
separates them.

A new private iterator type, BucketItTy, now handles the iteration
direction. This is an alias for a raw pointer for forward iteration
and std::reverse_iterator<pointer> for reverse iteration.

The user-facing iterator now focuses solely on advancing BucketItTy
while skipping invalid (empty or tombstone) buckets. AdvanceIfNotValid
now works transparently for both directions. operator++ on BucketItTy
does the right thing whether it's a raw pointer or a
std::reverse_iterator.

This simplification removes RetreatIfNotValid and the
reverse-iteration logic in operator*() and operator++().


>From 677cf7211b1c561359360021a71ffedd73de3e33 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Wed, 24 Sep 2025 14:03:07 -0700
Subject: [PATCH] [ADT] Simplify SmallPtrSetIterator with std::reverse_iterator
 (NFC)

SmallPtrSetIterator has two tasks:

 - iterate the buckets in the requested direction
 - skip the empty and tombstone buckets

These tasks are intertwined in the current implementation. This patch
separates them.

A new private iterator type, BucketItTy, now handles the iteration
direction. This is an alias for a raw pointer for forward iteration
and std::reverse_iterator<pointer> for reverse iteration.

The user-facing iterator now focuses solely on advancing BucketItTy
while skipping invalid (empty or tombstone) buckets. AdvanceIfNotValid
now works transparently for both directions. operator++ on BucketItTy
does the right thing whether it's a raw pointer or a
std::reverse_iterator.

This simplification removes RetreatIfNotValid and the
reverse-iteration logic in operator*() and operator++().
---
 llvm/include/llvm/ADT/SmallPtrSet.h | 30 +++++++----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/llvm/include/llvm/ADT/SmallPtrSet.h b/llvm/include/llvm/ADT/SmallPtrSet.h
index dca9f7995926d..d5332379fc542 100644
--- a/llvm/include/llvm/ADT/SmallPtrSet.h
+++ b/llvm/include/llvm/ADT/SmallPtrSet.h
@@ -281,16 +281,17 @@ class SmallPtrSetImplBase : public DebugEpochBase {
 /// instances of SmallPtrSetIterator.
 class SmallPtrSetIteratorImpl {
 protected:
-  const void *const *Bucket;
-  const void *const *End;
+  using BucketItTy =
+      std::conditional_t<shouldReverseIterate(),
+                         std::reverse_iterator<const void *const *>,
+                         const void *const *>;
+
+  BucketItTy Bucket;
+  BucketItTy End;
 
 public:
   explicit SmallPtrSetIteratorImpl(const void *const *BP, const void *const *E)
       : Bucket(BP), End(E) {
-    if (shouldReverseIterate()) {
-      RetreatIfNotValid();
-      return;
-    }
     AdvanceIfNotValid();
   }
 
@@ -312,14 +313,6 @@ class SmallPtrSetIteratorImpl {
             *Bucket == SmallPtrSetImplBase::getTombstoneMarker()))
       ++Bucket;
   }
-  void RetreatIfNotValid() {
-    assert(Bucket >= End);
-    while (Bucket != End &&
-           (Bucket[-1] == SmallPtrSetImplBase::getEmptyMarker() ||
-            Bucket[-1] == SmallPtrSetImplBase::getTombstoneMarker())) {
-      --Bucket;
-    }
-  }
 };
 
 /// SmallPtrSetIterator - This implements a const_iterator for SmallPtrSet.
@@ -344,21 +337,12 @@ class LLVM_DEBUGEPOCHBASE_HANDLEBASE_EMPTYBASE SmallPtrSetIterator
 
   const PtrTy operator*() const {
     assert(isHandleInSync() && "invalid iterator access!");
-    if (shouldReverseIterate()) {
-      assert(Bucket > End);
-      return PtrTraits::getFromVoidPointer(const_cast<void *>(Bucket[-1]));
-    }
     assert(Bucket < End);
     return PtrTraits::getFromVoidPointer(const_cast<void *>(*Bucket));
   }
 
   inline SmallPtrSetIterator &operator++() { // Preincrement
     assert(isHandleInSync() && "invalid iterator access!");
-    if (shouldReverseIterate()) {
-      --Bucket;
-      RetreatIfNotValid();
-      return *this;
-    }
     ++Bucket;
     AdvanceIfNotValid();
     return *this;