[ADT] Adding bidirectional iterator functionality + unit tests (#160726)

This feature is needed for #160415 , kuhar suggested that I split that
PR into 2 so that the ADT work is checking in first.

---------

Co-authored-by: Jakub Kuderski <[email protected]>

Author: FranciscoThiesen

llvm/include/llvm/ADT/BitVector.h

@@ -40,12 +40,20 @@ template <typename BitVectorT> class const_set_bits_iterator_impl {
     Current = Parent.find_next(Current);
   }
 
+  void retreat() {
+    if (Current == -1) {
+      Current = Parent.find_last();
+    } else {
+      Current = Parent.find_prev(Current);
+    }
+  }
+
 public:
-  using iterator_category = std::forward_iterator_tag;
-  using difference_type   = std::ptrdiff_t;
-  using value_type        = int;
-  using pointer           = value_type*;
-  using reference         = value_type&;
+  using iterator_category = std::bidirectional_iterator_tag;
+  using difference_type = std::ptrdiff_t;
+  using value_type = unsigned;
+  using pointer = const value_type *;
+  using reference = value_type;
 
   const_set_bits_iterator_impl(const BitVectorT &Parent, int Current)
       : Parent(Parent), Current(Current) {}
@@ -64,6 +72,17 @@ template <typename BitVectorT> class const_set_bits_iterator_impl {
     return *this;
   }
 
+  const_set_bits_iterator_impl operator--(int) {
+    auto Prev = *this;
+    retreat();
+    return Prev;
+  }
+
+  const_set_bits_iterator_impl &operator--() {
+    retreat();
+    return *this;
+  }
+
   unsigned operator*() const { return Current; }
 
   bool operator==(const const_set_bits_iterator_impl &Other) const {

llvm/unittests/ADT/BitVectorTest.cpp

@@ -8,6 +8,7 @@
 
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "gtest/gtest.h"
 
@@ -1177,6 +1178,98 @@ TYPED_TEST(BitVectorTest, Iterators) {
     EXPECT_EQ(List[i++], Bit);
 }
 
+TYPED_TEST(BitVectorTest, BidirectionalIterator) {
+  // Test decrement operators.
+  TypeParam Vec(100, false);
+  Vec.set(10);
+  Vec.set(20);
+  Vec.set(30);
+  Vec.set(40);
+
+  // Test that we can decrement from end().
+  auto EndIt = Vec.set_bits_end();
+  auto LastIt = EndIt;
+  --LastIt;
+  EXPECT_EQ(*LastIt, 40U);
+
+  // Test post-decrement.
+  auto It = Vec.set_bits_end();
+  auto PrevIt = It--;
+  EXPECT_EQ(PrevIt, Vec.set_bits_end());
+  EXPECT_EQ(*It, 40U);
+
+  // Test pre-decrement.
+  --It;
+  EXPECT_EQ(*It, 30U);
+
+  // Test full backward iteration.
+  std::vector<unsigned> BackwardBits;
+  for (auto RIt = Vec.set_bits_end(); RIt != Vec.set_bits_begin();) {
+    --RIt;
+    BackwardBits.push_back(*RIt);
+  }
+  EXPECT_EQ(BackwardBits.size(), 4U);
+  EXPECT_EQ(BackwardBits[0], 40U);
+  EXPECT_EQ(BackwardBits[1], 30U);
+  EXPECT_EQ(BackwardBits[2], 20U);
+  EXPECT_EQ(BackwardBits[3], 10U);
+}
+
+TYPED_TEST(BitVectorTest, ReverseIteration) {
+  // Test using llvm::reverse.
+  TypeParam Vec(100, false);
+  Vec.set(5);
+  Vec.set(15);
+  Vec.set(25);
+  Vec.set(35);
+  Vec.set(45);
+
+  std::vector<unsigned> ReversedBits;
+  for (unsigned Bit : llvm::reverse(Vec.set_bits())) {
+    ReversedBits.push_back(Bit);
+  }
+
+  EXPECT_EQ(ReversedBits.size(), 5U);
+  EXPECT_EQ(ReversedBits[0], 45U);
+  EXPECT_EQ(ReversedBits[1], 35U);
+  EXPECT_EQ(ReversedBits[2], 25U);
+  EXPECT_EQ(ReversedBits[3], 15U);
+  EXPECT_EQ(ReversedBits[4], 5U);
+}
+
+TYPED_TEST(BitVectorTest, BidirectionalIteratorEdgeCases) {
+  // Test empty BitVector.
+  TypeParam Empty;
+  EXPECT_EQ(Empty.set_bits_begin(), Empty.set_bits_end());
+
+  // Decrementing end() on empty should give -1 (no bits set).
+  auto EmptyEndIt = Empty.set_bits_end();
+  --EmptyEndIt;
+  // After decrement on empty, iterator should still be at "no bit" position.
+  EXPECT_EQ(*EmptyEndIt, static_cast<unsigned>(-1));
+
+  // Test single bit.
+  TypeParam Single(10, false);
+  Single.set(5);
+
+  auto SingleIt = Single.set_bits_end();
+  --SingleIt;
+  EXPECT_EQ(*SingleIt, 5U);
+  // After decrementing past the first element, the iterator is in an
+  // undefined state (before begin), so we don't test this case.
+
+  // Test all bits set.
+  TypeParam AllSet(10, true);
+  std::vector<unsigned> AllBitsReverse;
+  for (unsigned Bit : llvm::reverse(AllSet.set_bits())) {
+    AllBitsReverse.push_back(Bit);
+  }
+  EXPECT_EQ(AllBitsReverse.size(), 10U);
+  for (unsigned i = 0; i < 10; ++i) {
+    EXPECT_EQ(AllBitsReverse[i], 9 - i);
+  }
+}
+
 TYPED_TEST(BitVectorTest, PushBack) {
   TypeParam Vec(10, false);
   EXPECT_EQ(-1, Vec.find_first());